Update attribute can only appear in static items

Resolves dotnet#1618

src/XMakeBuildEngine/Construction/ProjectItemElement.cs

@@ -434,6 +434,8 @@ internal override void VerifyThrowInvalidOperationAcceptableLocation(ProjectElem
         {
             ErrorUtilities.VerifyThrowInvalidOperation(parent.Parent is ProjectTargetElement || (Include.Length > 0 || Update.Length > 0 || Remove.Length > 0), "OM_ItemsOutsideTargetMustHaveIncludeOrUpdateOrRemove");
             ErrorUtilities.VerifyThrowInvalidOperation(parent.Parent is ProjectRootElement || parent.Parent is ProjectTargetElement || parent.Parent is ProjectWhenElement || parent.Parent is ProjectOtherwiseElement, "OM_CannotAcceptParent");
+
+            ErrorUtilities.VerifyThrowInvalidOperation(Update.Length == 0 || parent.Parent is ProjectRootElement, "OM_UpdateOnlyUnderProject");
         }
 
         /// <summary>

src/XMakeBuildEngine/Evaluation/ProjectParser.cs

@@ -312,6 +312,7 @@ private ProjectItemGroupElement ParseProjectItemGroupElement(XmlElementWithLocat
         private ProjectItemElement ParseProjectItemElement(XmlElementWithLocation element, ProjectItemGroupElement parent)
         {
             bool belowTarget = parent.Parent is ProjectTargetElement;
+            bool belowProject = parent.Parent is ProjectRootElement;
 
             string itemType = element.Name;
             string include = element.GetAttribute(XMakeAttributes.include);
@@ -350,6 +351,9 @@ private ProjectItemElement ParseProjectItemElement(XmlElementWithLocation elemen
             // Exclude must be missing, unless Include exists
             ProjectXmlUtilities.VerifyThrowProjectInvalidAttribute(exclude.Length == 0 || include.Length > 0, (XmlAttributeWithLocation)element.Attributes[XMakeAttributes.exclude]);
 
+            // Update must be missing, unless inside a target and Include is missing
+            ProjectXmlUtilities.VerifyThrowProjectInvalidAttribute(update.Length == 0 || belowProject, (XmlAttributeWithLocation)element.Attributes[XMakeAttributes.update]);
+
             // If we have an Include attribute at all, it must have non-zero length
             ProjectErrorUtilities.VerifyThrowInvalidProject(include.Length > 0 || element.Attributes[XMakeAttributes.include] == null, element.Location, "MissingRequiredAttribute", XMakeAttributes.include, itemType);
 

src/XMakeBuildEngine/Resources/Strings.resx

@@ -1360,8 +1360,9 @@ Utilization:          {0} Average Utilization: {1:###.0}</value>
   <data name="OM_OneOfAttributeButNotMore">
     <value>The &lt;{0}&gt; element must have either the "{1}" attribute, the "{2}" attribute, or the "{3}" attribute, but not more than one.</value>
   </data>
-  <data name="OM_NoRemoveOutsideTargets">
-    <value>The "Remove" attribute is not permitted on an item outside of a &lt;Target&gt;.</value>
+  <data name="OM_UpdateOnlyUnderProject">
+    <value>The "Update" attribute is only permitted on items whose ItemGroup is under &lt;Project&gt;.</value>
+    <comment>Don't translate: Update, items, ItemGroup, Project</comment>
   </data>
   <data name="OM_NoKeepMetadataOutsideTargets">
     <value>The "KeepMetadata" attribute is not permitted on an item outside of a &lt;Target&gt;.</value>

src/XMakeBuildEngine/UnitTestsPublicOM/Construction/ConstructionEditing_Tests.cs

@@ -677,6 +677,28 @@ public void InvalidAttemptToAddItemToTarget()
            );
         }
         /// <summary>
+        /// Attempt to insert an update operation inside a target itemgroup
+        /// </summary>
+        [Fact]
+        public void InvalidAttemptToAddItemWithUpdateToTarget()
+        {
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                ProjectRootElement project = ProjectRootElement.Create();
+                ProjectTargetElement target = project.CreateTargetElement("t");
+                ProjectItemGroupElement itemgroup = project.CreateItemGroupElement();
+                ProjectItemElement item = project.CreateItemElement("i");
+
+                item.Update = "a";
+
+                project.AppendChild(target);
+                target.AppendChild(itemgroup);
+
+                itemgroup.AppendChild(item);
+            }
+           );
+        }
+        /// <summary>
         /// Attempt to insert item without include in itemgroup in project
         /// </summary>
         [Fact]

src/XMakeBuildEngine/UnitTestsPublicOM/Construction/ProjectItemElement_Tests.cs

@@ -72,9 +72,11 @@ public class ProjectItemElement_Tests
                 ";
         private const string UpdateInTarget = @"
                     <Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
-                            <ItemGroup>
-                                <i Update='i'/>
-                            </ItemGroup>
+                            <Target Name='t'>
+                                <ItemGroup>
+                                    <i Update='i'/>
+                                </ItemGroup>
+                            </Target>
                     </Project>
                 ";
 
@@ -597,17 +599,28 @@ public void ReadValidRemove(string project)
         }
 
         /// <summary>
-        /// Read item with Remove inside of Target
+        /// Read item with Update outside of Target
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void ReadValidUpdate(string project)
+        [Fact]
+        public void ReadValidUpdate()
         {
-            var item = GetItemFromContent(project);
+            var item = GetItemFromContent(UpdateOutsideTarget);
 
             Assert.Equal("i", item.Update);
         }
+
+        /// <summary>
+        /// Update can't appear inside a target
+        /// </summary>
+        [Fact]
+        public void ReadInvalidUpdateInTarget()
+        {
+            Assert.Throws<InvalidProjectFileException>(
+                () =>
+                {
+                    GetItemFromContent(UpdateInTarget);
+                });
+        }
 
         /// <summary>
         /// Read item with Exclude without Include, inside of Target
@@ -857,16 +870,13 @@ public void SetInvalidUpdateWithRemove(string project)
            );
         }
 
-        /// 
         /// <summary>
         /// Set the Update on an item
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void SetUpdate(string project)
+        [Fact]
+        public void SetUpdate()
         {
-            ProjectItemElement item = GetItemFromContent(project);
+            ProjectItemElement item = GetItemFromContent(UpdateOutsideTarget);
 
             item.Update = "ib";
 
@@ -876,12 +886,10 @@ public void SetUpdate(string project)
         /// <summary>
         /// Set empty Update: this removes it
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void SetEmptyUpdate(string project)
+        [Fact]
+        public void SetEmptyUpdate()
         {
-            ProjectItemElement item = GetItemFromContent(project);
+            ProjectItemElement item = GetItemFromContent(UpdateOutsideTarget);
 
             item.Update = String.Empty;
 
@@ -891,12 +899,10 @@ public void SetEmptyUpdate(string project)
         /// <summary>
         /// Set null Update: this removes it
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void SetNullUpdate(string project)
+        [Fact]
+        public void SetNullUpdate()
         {
-            ProjectItemElement item = GetItemFromContent(project);
+            ProjectItemElement item = GetItemFromContent(UpdateOutsideTarget);
 
             item.Update = null;
 
@@ -906,14 +912,12 @@ public void SetNullUpdate(string project)
         /// <summary>
         /// Set Include when Update is present
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void SetInvalidIncludeWithUpdate(string project)
+        [Fact]
+        public void SetInvalidIncludeWithUpdate()
         {
             Assert.Throws<InvalidOperationException>(() =>
             {
-                ProjectItemElement item = GetItemFromContent(project);
+                ProjectItemElement item = GetItemFromContent(UpdateOutsideTarget);
 
                 item.Include = "i1";
             }
@@ -924,7 +928,6 @@ public void SetInvalidIncludeWithUpdate(string project)
         /// Set Exclude when Update is present
         /// </summary>
         [Theory]
-        [InlineData(UpdateInTarget)]
         [InlineData(UpdateOutsideTarget)]
         public void SetInvalidExcludeWithUpdate(string project)
         {
@@ -940,12 +943,10 @@ public void SetInvalidExcludeWithUpdate(string project)
         /// <summary>
         /// Set the condition on an item
         /// </summary>
-        [Theory]
-        [InlineData(UpdateInTarget)]
-        [InlineData(UpdateOutsideTarget)]
-        public void SetCondition(string project)
+        [Fact]
+        public void SetCondition()
         {
-            ProjectItemElement item = GetItemFromContent(project);
+            ProjectItemElement item = GetItemFromContent(UpdateOutsideTarget);
 
             item.Condition = "c";