Idempotent change won't mark dependency unresolved

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/Filters/AddDependencyContext.cs

@@ -59,9 +59,8 @@ public bool TryGetDependency(DependencyId dependencyId, out IDependency dependen
         /// </remarks>
         public void AddOrUpdate(IDependency dependency)
         {
-            DependencyId key = dependency.GetDependencyId();
-            _dependencyById.Remove(key);
-            _dependencyById.Add(key, dependency);
+            _dependencyById[dependency.GetDependencyId()] = dependency;
+
             Changed = true;
         }
 

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/DependenciesChangesBuilder.cs

@@ -8,24 +8,45 @@ namespace Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies.Subscriptions
 {
     internal sealed class DependenciesChangesBuilder
     {
+        /// <summary>
+        /// Shared running set of resolved dependencies.
+        /// </summary>
+        /// <remarks>
+        /// Used in <see cref="HasResolvedItem"/>, where the caller wants to ensure we don't regress a resolved item to
+        /// unresolved state when we receive an evaluation-only update.
+        /// </remarks>
+        private readonly HashSet<(TargetFramework TargetFramework, string ProviderType, string DependencyId)>? _resolvedItems;
+
         private HashSet<IDependencyModel>? _added;
         private HashSet<IDependencyModel>? _removed;
 
-        public void Added(IDependencyModel model)
+        public DependenciesChangesBuilder(HashSet<(TargetFramework TargetFramework, string ProviderType, string DependencyId)>? resolvedItems = null)
+        {
+            _resolvedItems = resolvedItems;
+        }
+
+        public void Added(TargetFramework targetFramework, IDependencyModel model)
         {
             _added ??= new HashSet<IDependencyModel>(IDependencyModelEqualityComparer.Instance);
 
             _added.Remove(model);
             _added.Add(model);
+
+            if (model.Resolved)
+            {
+                _resolvedItems?.Add((targetFramework, model.ProviderType, model.Id));
+            }
         }
 
-        public void Removed(string providerType, string dependencyId)
+        public void Removed(TargetFramework targetFramework, string providerType, string dependencyId)
         {
             _removed ??= new HashSet<IDependencyModel>(IDependencyModelEqualityComparer.Instance);
 
             var identity = new RemovedDependencyModel(providerType, dependencyId);
             _removed.Remove(identity);
             _removed.Add(identity);
+
+            _resolvedItems?.Remove((targetFramework, providerType, dependencyId));
         }
 
         public IDependenciesChanges? TryBuildChanges()
@@ -40,6 +61,11 @@ public void Removed(string providerType, string dependencyId)
                 _removed == null ? (IImmutableList<IDependencyModel>)ImmutableList<IDependencyModel>.Empty : ImmutableArray.CreateRange(_removed));
         }
 
+        public bool HasResolvedItem(TargetFramework targetFramework, string providerType, string dependencyId)
+        {
+            return _resolvedItems?.Contains((targetFramework, providerType, dependencyId)) == true;
+        }
+
         public override string ToString() => ToString(_added, _removed);
 
         private sealed class DependenciesChanges : IDependenciesChanges

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/DependencyRulesSubscriber.cs

@@ -27,6 +27,8 @@ internal sealed class DependencyRulesSubscriber : DependencyRulesSubscriberBase<
         private readonly Lazy<string[]> _watchedEvaluationRules;
         private readonly Lazy<string[]> _watchedJointRules;
 
+        private readonly HashSet<(TargetFramework TargetFramework, string ProviderType, string DependencyId)> _resolvedItems = new();
+
         [ImportingConstructor]
         public DependencyRulesSubscriber(
             IUnconfiguredProjectCommonServices commonServices,
@@ -117,15 +119,15 @@ protected override void Handle(
             }
 
             // Create an object to track dependency changes.
-            var changesBuilder = new DependenciesChangesBuilder();
+            var changesBuilder = new DependenciesChangesBuilder(_resolvedItems);
 
             // Give each handler a chance to register dependency changes.
             foreach (Lazy<IDependenciesRuleHandler, IOrderPrecedenceMetadataView> handler in _handlers)
             {
-                IProjectChangeDescription evaluation = projectUpdate.ProjectChanges[handler.Value.EvaluatedRuleName];
-                IProjectChangeDescription? build = projectUpdate.ProjectChanges.GetValueOrDefault(handler.Value.ResolvedRuleName);
+                IProjectChangeDescription evaluationProjectChange = projectUpdate.ProjectChanges[handler.Value.EvaluatedRuleName];
+                IProjectChangeDescription? buildProjectChange = projectUpdate.ProjectChanges.GetValueOrDefault(handler.Value.ResolvedRuleName);
 
-                handler.Value.Handle(projectFullPath, evaluation, build, targetFrameworkToUpdate, changesBuilder);
+                handler.Value.Handle(projectFullPath, evaluationProjectChange, buildProjectChange, targetFrameworkToUpdate, changesBuilder);
             }
 
             IDependenciesChanges? changes = changesBuilder.TryBuildChanges();

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/DependencySharedProjectsSubscriber.cs

@@ -106,7 +106,7 @@ private void ProcessSharedProjectsUpdates(
                     isResolved: true,
                     isImplicit: false,
                     properties: ImmutableStringDictionary<string>.EmptyOrdinal);
-                changesBuilder.Added(added);
+                changesBuilder.Added(targetFramework, added);
             }
 
             // process removed nodes
@@ -117,6 +117,7 @@ private void ProcessSharedProjectsUpdates(
                 if (exists)
                 {
                     changesBuilder.Removed(
+                        targetFramework,
                         ProjectRuleHandler.ProviderTypeString,
                         dependencyId: removedSharedImportPath);
                 }

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/RuleHandlers/AnalyzerRuleHandler.cs

@@ -44,14 +44,14 @@ public AnalyzerRuleHandler()
         protected override IDependencyModel CreateDependencyModel(
             string path,
             string originalItemSpec,
-            bool resolved,
+            bool isResolved,
             bool isImplicit,
             IImmutableDictionary<string, string> properties)
         {
             return new AnalyzerDependencyModel(
                 path,
                 originalItemSpec,
-                resolved,
+                isResolved,
                 isImplicit,
                 properties);
         }

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/RuleHandlers/AssemblyRuleHandler.cs

@@ -42,14 +42,14 @@ public AssemblyRuleHandler()
         protected override IDependencyModel CreateDependencyModel(
             string path,
             string originalItemSpec,
-            bool resolved,
+            bool isResolved,
             bool isImplicit,
             IImmutableDictionary<string, string> properties)
         {
             return new AssemblyDependencyModel(
                 path,
                 originalItemSpec,
-                resolved,
+                isResolved,
                 isImplicit,
                 properties);
         }

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/RuleHandlers/ComRuleHandler.cs

@@ -42,14 +42,14 @@ public ComRuleHandler()
         protected override IDependencyModel CreateDependencyModel(
             string path,
             string originalItemSpec,
-            bool resolved,
+            bool isResolved,
             bool isImplicit,
             IImmutableDictionary<string, string> properties)
         {
             return new ComDependencyModel(
                 path,
                 originalItemSpec,
-                resolved,
+                isResolved,
                 isImplicit,
                 properties);
         }

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/RuleHandlers/DependenciesRuleHandlerBase.cs

@@ -54,43 +54,37 @@ public void Handle(
             TargetFramework targetFramework,
             DependenciesChangesBuilder changesBuilder)
         {
-            HandleChangesForRule(
-                resolved: false,
-                evaluationProjectChange,
-                null,
-                isEvaluatedItemSpec: null);
+            bool hasResolvedData = buildProjectChange is not null;
+
+            HandleChangesForRule(evaluationProjectChange);
 
             // We only have resolved data if the update came via the JointRule data source.
-            if (buildProjectChange != null)
+            if (hasResolvedData)
             {
                 Func<string, bool>? isEvaluatedItemSpec = ResolvedItemRequiresEvaluatedItem ? evaluationProjectChange.After.Items.ContainsKey : (Func<string, bool>?)null;
 
-                HandleChangesForRule(
-                    resolved: true,
-                    evaluationProjectChange,
-                    buildProjectChange,
-                    isEvaluatedItemSpec);
+                HandleChangesForRule(evaluationProjectChange, buildProjectChange, isEvaluatedItemSpec);
             }
 
             return;
 
-            void HandleChangesForRule(bool resolved, IProjectChangeDescription evaluationProjectChange, IProjectChangeDescription? buildProjectChange, Func<string, bool>? isEvaluatedItemSpec)
+            void HandleChangesForRule(IProjectChangeDescription evaluationProjectChange, IProjectChangeDescription? buildProjectChange = null, Func<string, bool>? isEvaluatedItemSpec = null)
             {
-                IProjectChangeDescription projectChange = resolved ? buildProjectChange! : evaluationProjectChange;
+                IProjectChangeDescription projectChange = buildProjectChange ?? evaluationProjectChange;
 
                 foreach (string removedItem in projectChange.Difference.RemovedItems)
                 {
-                    HandleRemovedItem(projectFullPath, removedItem, resolved, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
+                    HandleRemovedItem(projectFullPath, removedItem, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
                 }
 
                 foreach (string changedItem in projectChange.Difference.ChangedItems)
                 {
-                    HandleChangedItem(projectFullPath, changedItem, resolved, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
+                    HandleAddedItem(projectFullPath, changedItem, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
                 }
 
                 foreach (string addedItem in projectChange.Difference.AddedItems)
                 {
-                    HandleAddedItem(projectFullPath, addedItem, resolved, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
+                    HandleAddedItem(projectFullPath, addedItem, projectChange, evaluationProjectChange.After, changesBuilder, targetFramework, isEvaluatedItemSpec);
                 }
 
                 System.Diagnostics.Debug.Assert(projectChange.Difference.RenamedItems.Count == 0, "Project rule diff should not contain renamed items");
@@ -100,63 +94,54 @@ void HandleChangesForRule(bool resolved, IProjectChangeDescription evaluationPro
         protected virtual void HandleAddedItem(
             string projectFullPath,
             string addedItem,
-            bool resolved,
             IProjectChangeDescription projectChange,
             IProjectRuleSnapshot evaluationRuleSnapshot,
             DependenciesChangesBuilder changesBuilder,
             TargetFramework targetFramework,
             Func<string, bool>? isEvaluatedItemSpec)
         {
-            IDependencyModel? model = CreateDependencyModelForRule(addedItem, evaluationRuleSnapshot, projectChange.After, resolved, projectFullPath);
+            bool isResolved = isEvaluatedItemSpec is not null;
+
+            IDependencyModel? model = CreateDependencyModelForRule(addedItem, evaluationRuleSnapshot, projectChange.After, isResolved, changesBuilder, targetFramework, projectFullPath);
 
             if (model != null && (isEvaluatedItemSpec == null || isEvaluatedItemSpec(model.Id)))
             {
-                changesBuilder.Added(model);
+                changesBuilder.Added(targetFramework, model);
             }
         }
 
         protected virtual void HandleRemovedItem(
             string projectFullPath,
             string removedItem,
-            bool resolved,
             IProjectChangeDescription projectChange,
             IProjectRuleSnapshot evaluationRuleSnapshot,
             DependenciesChangesBuilder changesBuilder,
             TargetFramework targetFramework,
             Func<string, bool>? isEvaluatedItemSpec)
         {
-            string dependencyId = resolved
-                ? projectChange.Before.GetProjectItemProperties(removedItem)!.GetStringProperty(ProjectItemMetadata.OriginalItemSpec) ?? removedItem
-                : removedItem;
-
-            if (isEvaluatedItemSpec == null || isEvaluatedItemSpec(dependencyId))
+            if (isEvaluatedItemSpec is not null)
             {
-                changesBuilder.Removed(ProviderType, removedItem);
+                // The item is resolved
+                string dependencyId = projectChange.Before.GetProjectItemProperties(removedItem)!.GetStringProperty(ProjectItemMetadata.OriginalItemSpec) ?? removedItem;
+
+                if (!isEvaluatedItemSpec(dependencyId))
+                {
+                    // Skip any items returned by build that were not present in evaluation
+                    return;
+                }
             }
+
+            changesBuilder.Removed(targetFramework, ProviderType, removedItem);
         }
 
-        protected virtual void HandleChangedItem(
-            string projectFullPath,
-            string changedItem,
-            bool resolved,
-            IProjectChangeDescription projectChange,
+        private IDependencyModel? CreateDependencyModelForRule(
+            string itemSpec,
             IProjectRuleSnapshot evaluationRuleSnapshot,
+            IProjectRuleSnapshot updatedRuleSnapshot,
+            bool isResolved,
             DependenciesChangesBuilder changesBuilder,
             TargetFramework targetFramework,
-            Func<string, bool>? isEvaluatedItemSpec)
-        {
-            IDependencyModel? model = CreateDependencyModelForRule(changedItem, evaluationRuleSnapshot, projectChange.After, resolved, projectFullPath);
-
-            if (model != null && (isEvaluatedItemSpec == null || isEvaluatedItemSpec(model.Id)))
-            {
-                // For changes we try to add new dependency. If it is a resolved dependency, it would just override
-                // old one with new properties. If it is unresolved dependency, it would be added only when there is
-                // no resolved version in the snapshot (due to UnresolvedDependenciesSnapshotFilter).
-                changesBuilder.Added(model);
-            }
-        }
-
-        private IDependencyModel? CreateDependencyModelForRule(string itemSpec, IProjectRuleSnapshot evaluationRuleSnapshot, IProjectRuleSnapshot updatedRuleSnapshot, bool isResolved, string projectFullPath)
+            string projectFullPath)
         {
             IImmutableDictionary<string, string>? properties = updatedRuleSnapshot.GetProjectItemProperties(itemSpec);
 
@@ -181,6 +166,9 @@ protected virtual void HandleChangedItem(
 
             bool isImplicit = IsImplicit(projectFullPath, evaluationProperties);
 
+            // When we only have evaluation data, mark the dependency as resolved if we currently have a corresponding resolved item
+            isResolved = isResolved || changesBuilder.HasResolvedItem(targetFramework, ProviderType, originalItemSpec);
+
             return CreateDependencyModel(
                 itemSpec,
                 originalItemSpec,
@@ -218,7 +206,7 @@ protected static bool IsImplicit(
         protected virtual IDependencyModel CreateDependencyModel(
             string path,
             string originalItemSpec,
-            bool resolved,
+            bool isResolved,
             bool isImplicit,
             IImmutableDictionary<string, string> properties)
         {

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Subscriptions/RuleHandlers/FrameworkRuleHandler.cs

@@ -42,14 +42,14 @@ public FrameworkRuleHandler()
         protected override IDependencyModel CreateDependencyModel(
             string path,
             string originalItemSpec,
-            bool resolved,
+            bool isResolved,
             bool isImplicit,
             IImmutableDictionary<string, string> properties)
         {
             return new FrameworkDependencyModel(
                 path,
                 originalItemSpec,
-                resolved,
+                isResolved,
                 isImplicit,
                 properties);
         }