Merge branch 'main' into dev/adamint/fix-define-constants-validation

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Menus.vsct

@@ -227,7 +227,7 @@
 
   <Symbols>
     <GuidSymbol name="guidSolutionExplorerToolWindow" value="{3AE79031-E1BC-11D0-8F78-00A0C9110057}" />
-    
+
     <!-- The .NET Project System package GUID. -->
     <GuidSymbol name="PackageGuidString" value="{860A27C0-B665-47F3-BC12-637E16A1050A}" />
 
@@ -270,8 +270,6 @@
       <IDSymbol name="IDM_VS_CTXT_SHAREDPROJECTREFERENCE" value="0x04A8" />
       <IDSymbol name="IDM_VS_CTXT_TRANSITIVE_ASSEMBLY_REFERENCE" value="0x04B1" />
     </GuidSymbol>
-
-    <GuidSymbol name="SlnExplorerGuid" value="{3AE79031-E1BC-11D0-8F78-00A0C9110057}" />
   </Symbols>
 
 </CommandTable>

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs

@@ -36,7 +36,10 @@ public LanguageServiceErrorListProvider(
 
         _isLspPullDiagnosticsEnabled = new AsyncLazy<bool>(
             async () => await projectSystemOptions.IsLspPullDiagnosticsEnabledAsync(CancellationToken.None),
-            joinableTaskContext.Factory);
+            joinableTaskContext.Factory)
+        {
+            SuppressRecursiveFactoryDetection = true
+        };
     }
 
     public void SuspendRefresh()

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs

@@ -54,25 +54,28 @@ private sealed class DesignTimeTelemetryLogger(ITelemetryService telemetryServic
         /// <remarks>
         /// The initial capacity here is based on an empty .NET Console App's DTB, which has
         /// around 120 targets, and aims to avoid resizing the dictionary during the build.
+        ///
+        /// Targets can run concurrently, so use of this collection must be protected by a lock.
         /// </remarks>
         private readonly Dictionary<int, TargetRecord> _targetRecordById = new(capacity: 140);
 
         /// <summary>
-        /// The names of any targets that reported errors. Names may be hashed.
-        /// May be empty even when errors exist, as not all errors come from a target.
+        /// Data about any errors that occurred during the build. Note that design-time build
+        /// targets are supposed to be authored to respect ContinueOnError, so errors might
+        /// not fail the build. However it's still useful to know which targets reported errors.
+        /// 
+        /// Similarly, this collection may be empty even when the build fails.
         /// </summary>
-        private List<string>? _errorTargets;
+        /// <remarks>
+        /// Targets can run concurrently, so use of this collection must be protected by a lock.
+        /// </remarks>
+        private readonly List<ErrorData> _errors = [];
 
         /// <summary>
         /// Whether the build succeeded.
         /// </summary>
         private bool? _succeeded;
 
-        /// <summary>
-        /// The number of errors reported during the build.
-        /// </summary>
-        private int _errorCount;
-
         LoggerVerbosity ILogger.Verbosity { get; set; }
 
         string? ILogger.Parameters { get; set; }
@@ -93,7 +96,10 @@ void OnTargetStarted(object sender, TargetStartedEventArgs e)
                         e.TargetFile,
                         file => ProjectFileClassifier.IsShippedByMicrosoft(e.TargetFile));
 
-                    _targetRecordById[id] = new TargetRecord(e.TargetName, IsMicrosoft: isMicrosoft, e.Timestamp);
+                    lock (_targetRecordById)
+                    {
+                        _targetRecordById[id] = new TargetRecord(e.TargetName, IsMicrosoft: isMicrosoft, e.Timestamp);
+                    }
                 }
             }
 
@@ -107,12 +113,16 @@ void OnTargetFinished(object sender, TargetFinishedEventArgs e)
 
             void OnErrorRaised(object sender, BuildErrorEventArgs e)
             {
-                _errorCount++;
+                string? targetName = null;
 
                 if (TryGetTargetRecord(e.BuildEventContext, out TargetRecord? record))
                 {
-                    _errorTargets ??= [];
-                    _errorTargets.Add(GetHashedTargetName(record));
+                    targetName = GetHashedTargetName(record);
+                }
+
+                lock (_errors)
+                {
+                    _errors.Add(new(targetName, e.Code));
                 }
             }
 
@@ -123,9 +133,12 @@ void OnBuildFinished(object sender, BuildFinishedEventArgs e)
 
             bool TryGetTargetRecord(BuildEventContext? context, [NotNullWhen(returnValue: true)] out TargetRecord? record)
             {
-                if (context is { TargetId: int id } && _targetRecordById.TryGetValue(id, out record))
+                lock (_targetRecordById)
                 {
-                    return true;
+                    if (context is { TargetId: int id } && _targetRecordById.TryGetValue(id, out record))
+                    {
+                        return true;
+                    }
                 }
 
                 record = null;
@@ -141,29 +154,40 @@ void ILogger.Shutdown()
 
             void SendTelemetry()
             {
-                // Filter out very fast targets (to reduce the cost of ordering) then take the top ten by elapsed time.
-                // Note that targets can run multiple times, so the same target may appear more than once in the results.
-                object[][] targetDurations = _targetRecordById.Values
-                    .Where(static record => record.Elapsed > new TimeSpan(ticks: 5 * TimeSpan.TicksPerMillisecond))
-                    .OrderByDescending(record => record.Elapsed)
-                    .Take(10)
-                    .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds })
-                    .ToArray();
+                object[][] targetDurations;
+                ErrorData[] errors;
+
+                lock (_targetRecordById)
+                {
+                    // Filter out very fast targets (to reduce the cost of ordering) then take the top ten by elapsed time.
+                    // Note that targets can run multiple times, so the same target may appear more than once in the results.
+                    targetDurations = _targetRecordById.Values
+                        .Where(static record => record.Elapsed > new TimeSpan(ticks: 5 * TimeSpan.TicksPerMillisecond))
+                        .OrderByDescending(record => record.Elapsed)
+                        .Take(10)
+                        .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds })
+                        .ToArray();
+
+                    errors = _errors.ToArray();
+                }
 
                 telemetryService.PostProperties(
                     TelemetryEventName.DesignTimeBuildComplete,
                     [
                         (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, _succeeded),
                         (TelemetryPropertyName.DesignTimeBuildComplete.Targets, new ComplexPropertyValue(targetDurations)),
-                        (TelemetryPropertyName.DesignTimeBuildComplete.ErrorCount, _errorCount),
-                        (TelemetryPropertyName.DesignTimeBuildComplete.ErrorTargets, _errorTargets),
+                        (TelemetryPropertyName.DesignTimeBuildComplete.ErrorCount, errors.Length),
+                        (TelemetryPropertyName.DesignTimeBuildComplete.Errors, new ComplexPropertyValue(errors)),
                     ]);
             }
 
             void ReportBuildErrors()
             {
-                if (_errorCount is 0)
+                if (_succeeded is not false)
                 {
+                    // Only report a failure if the build failed. Specific targets can have
+                    // errors, yet if ContinueOnError is set accordingly they won't fail the
+                    // build. We don't want to report those to the user.
                     return;
                 }
 
@@ -208,5 +232,12 @@ private sealed record class TargetRecord(string TargetName, bool IsMicrosoft, Da
 
             public TimeSpan Elapsed => Ended - Started;
         }
+
+        /// <summary>
+        /// Data about errors reported during design-time builds.
+        /// </summary>
+        /// <param name="TargetName">Names of the targets that reported the error. May be hashed. <see langword="null"/> if the target name could not be identified.</param>
+        /// <param name="ErrorCode">An error code that identifies the type of error.</param>
+        private sealed record class ErrorData(string? TargetName, string ErrorCode);
     }
 }

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs

@@ -36,6 +36,11 @@ namespace Microsoft.VisualStudio.ProjectSystem.LanguageServices;
 [AppliesTo(ProjectCapability.DotNetLanguageService)]
 internal sealed class LanguageServiceHost : OnceInitializedOnceDisposedAsync, IProjectDynamicLoadComponent, IWorkspaceWriter
 {
+    /// <summary>
+    /// Singleton instance across all projects, initialized once.
+    /// </summary>
+    private static AsyncLazy<bool>? s_isEnabled;
+
     private readonly TaskCompletionSource _firstPrimaryWorkspaceSet = new();
 
     private readonly UnconfiguredProject _unconfiguredProject;
@@ -45,9 +50,6 @@ internal sealed class LanguageServiceHost : OnceInitializedOnceDisposedAsync, IP
     private readonly IUnconfiguredProjectTasksService _tasksService;
     private readonly ISafeProjectGuidService _projectGuidService;
     private readonly IProjectFaultHandlerService _projectFaultHandler;
-    private readonly JoinableTaskCollection _joinableTaskCollection;
-    private readonly JoinableTaskFactory _joinableTaskFactory;
-    private readonly AsyncLazy<bool> _isEnabled;
 
     private DisposableBag? _disposables;
 
@@ -78,7 +80,9 @@ public LanguageServiceHost(
         _projectGuidService = projectGuidService;
         _projectFaultHandler = projectFaultHandler;
 
-        _isEnabled = new(
+        // We initialize this once across all instances. Note that we don't need any synchronization here.
+        // If more than one thread initializes this, it's not a big deal.
+        s_isEnabled ??= new(
             async () =>
             {
                 // If VS is running in command line mode (e.g. "devenv.exe /build my.sln"),
@@ -87,11 +91,10 @@ public LanguageServiceHost(
                 return !await vsShell.IsCommandLineModeAsync()
                     || await vsShell.IsPopulateSolutionCacheModeAsync();
             },
-            threadingService.JoinableTaskFactory);
-
-        _joinableTaskCollection = threadingService.JoinableTaskContext.CreateCollection();
-        _joinableTaskCollection.DisplayName = "LanguageServiceHostTasks";
-        _joinableTaskFactory = new JoinableTaskFactory(_joinableTaskCollection);
+            threadingService.JoinableTaskFactory)
+        {
+            SuppressRecursiveFactoryDetection = true
+        };
     }
 
     public Task LoadAsync()
@@ -175,7 +178,7 @@ protected override async Task InitializeCoreAsync(CancellationToken cancellation
                 linkOptions: DataflowOption.PropagateCompletion,
                 cancellationToken: cancellationToken),
 
-            ProjectDataSources.JoinUpstreamDataSources(_joinableTaskFactory, _projectFaultHandler, _activeConfiguredProjectProvider, _activeConfigurationGroupSubscriptionService),
+            ProjectDataSources.JoinUpstreamDataSources(JoinableFactory, _projectFaultHandler, _activeConfiguredProjectProvider, _activeConfigurationGroupSubscriptionService),
 
             new DisposableDelegate(() =>
             {
@@ -211,7 +214,7 @@ async Task OnSlicesChangedAsync(IProjectVersionedValue<(ConfiguredProject Active
                     Guid projectGuid = await _projectGuidService.GetProjectGuidAsync(cancellationToken);
 
                     // New slice. Create a workspace for it.
-                    workspace = _workspaceFactory.Create(source, slice, _joinableTaskCollection, _joinableTaskFactory, projectGuid, cancellationToken);
+                    workspace = _workspaceFactory.Create(source, slice, JoinableCollection, JoinableFactory, projectGuid, cancellationToken);
 
                     if (workspace is null)
                     {
@@ -274,15 +277,17 @@ async Task OnSlicesChangedAsync(IProjectVersionedValue<(ConfiguredProject Active
 
     public Task<bool> IsEnabledAsync(CancellationToken cancellationToken)
     {
+        Assumes.NotNull(s_isEnabled);
+
         // Defer to the host environment to determine if we're enabled.
-        return _isEnabled.GetValueAsync(cancellationToken);
+        return s_isEnabled.GetValueAsync(cancellationToken);
     }
 
     public async Task WhenInitialized(CancellationToken token)
     {
         await ValidateEnabledAsync(token);
 
-        using (_joinableTaskCollection.Join())
+        using (JoinableCollection.Join())
         {
             await _firstPrimaryWorkspaceSet.Task.WithCancellation(token);
         }
@@ -350,7 +355,7 @@ public async Task AfterLoadInitialConfigurationAsync()
         // Ensure the project is not considered loaded until our first publication.
         Task result = _tasksService.PrioritizedProjectLoadedInHostAsync(async () =>
         {
-            using (_joinableTaskCollection.Join())
+            using (JoinableCollection.Join())
             {
                 await WhenInitialized(_tasksService.UnloadCancellationToken);
             }

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/AbstractProjectState.cs

@@ -9,8 +9,8 @@ internal abstract class AbstractProjectState : IProjectState
 {
     protected readonly UnconfiguredProject Project;
 
-    private readonly Dictionary<ProjectConfiguration, IPropertyPagesCatalog?> _catalogCache;
-    private readonly Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?> _ruleCache;
+    private readonly Dictionary<ProjectConfiguration, IPropertyPagesCatalog?> _catalogCache = [];
+    private readonly Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?> _ruleCache = [];
 
     private readonly AsyncLazy<IImmutableSet<ProjectConfiguration>?> _knownProjectConfigurations;
     private readonly AsyncLazy<ProjectConfiguration?> _defaultProjectConfiguration;
@@ -20,10 +20,34 @@ protected AbstractProjectState(UnconfiguredProject project)
         Project = project;
         JoinableTaskFactory joinableTaskFactory = project.Services.ThreadingPolicy.JoinableTaskFactory;
 
-        _knownProjectConfigurations = new AsyncLazy<IImmutableSet<ProjectConfiguration>?>(CreateKnownConfigurationsAsync, joinableTaskFactory);
-        _defaultProjectConfiguration = new AsyncLazy<ProjectConfiguration?>(CreateDefaultConfigurationAsync, joinableTaskFactory);
-        _catalogCache = new Dictionary<ProjectConfiguration, IPropertyPagesCatalog?>();
-        _ruleCache = new Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?>();
+        _knownProjectConfigurations = new AsyncLazy<IImmutableSet<ProjectConfiguration>?>(CreateKnownConfigurationsAsync, joinableTaskFactory)
+        {
+            SuppressRecursiveFactoryDetection = true
+        };
+
+        _defaultProjectConfiguration = new AsyncLazy<ProjectConfiguration?>(CreateDefaultConfigurationAsync, joinableTaskFactory)
+        {
+            SuppressRecursiveFactoryDetection = true
+        };
+
+        async Task<IImmutableSet<ProjectConfiguration>?> CreateKnownConfigurationsAsync()
+        {
+            return Project.Services.ProjectConfigurationsService switch
+            {
+                IProjectConfigurationsService configurationsService => await configurationsService.GetKnownProjectConfigurationsAsync(),
+                _ => null
+            };
+        }
+
+        async Task<ProjectConfiguration?> CreateDefaultConfigurationAsync()
+        {
+            return Project.Services.ProjectConfigurationsService switch
+            {
+                IProjectConfigurationsService2 configurationsService2 => await configurationsService2.GetSuggestedProjectConfigurationAsync(),
+                IProjectConfigurationsService configurationsService => configurationsService.SuggestedProjectConfiguration,
+                _ => null
+            };
+        }
     }
 
     /// <summary>
@@ -60,32 +84,6 @@ protected AbstractProjectState(UnconfiguredProject project)
     /// </summary>
     public Task<ProjectConfiguration?> GetSuggestedConfigurationAsync() => _defaultProjectConfiguration.GetValueAsync();
 
-    private async Task<ProjectConfiguration?> CreateDefaultConfigurationAsync()
-    {
-        if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService2 configurationsService2)
-        {
-            return await configurationsService2.GetSuggestedProjectConfigurationAsync();
-        }
-        else if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService configurationsService)
-        {
-            return configurationsService.SuggestedProjectConfiguration;
-        }
-        else
-        {
-            return null;
-        }
-    }
-
-    private async Task<IImmutableSet<ProjectConfiguration>?> CreateKnownConfigurationsAsync()
-    {
-        if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService configurationsService)
-        {
-            return await configurationsService.GetKnownProjectConfigurationsAsync();
-        }
-
-        return null;
-    }
-
     /// <summary>
     /// Retrieves the set of property pages that apply to the project level for the given <paramref
     /// name="projectConfiguration"/>.

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/ContextAndRuleProviderState.cs

@@ -10,7 +10,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query;
 /// to pass the state their child producers will need, but allows the actual binding
 /// of the <see cref="Rule"/> to be delayed until needed.
 /// </summary>
-internal sealed class ContextAndRuleProviderState
+public sealed class ContextAndRuleProviderState
 {
     public ContextAndRuleProviderState(IProjectState projectState, QueryProjectPropertiesContext propertiesContext, Rule rule)
     {

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IProjectState.cs

@@ -32,7 +32,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query;
 /// significantly reduce the amount of work we need to do.
 /// </para>
 /// </remarks>
-internal interface IProjectState
+public interface IProjectState
 {
     /// <summary>
     /// Binds the specified schema to a particular context within the given project configuration.

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/QueryProjectPropertiesContext.cs

@@ -16,7 +16,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query;
 /// manner that can be passed from one provider to another and is also suitable as a
 /// key into a cache (such as the <see cref="IProjectState"/>).
 /// </remarks>
-internal sealed class QueryProjectPropertiesContext : IProjectPropertiesContext, IEquatable<QueryProjectPropertiesContext>
+public sealed class QueryProjectPropertiesContext : IProjectPropertiesContext, IEquatable<QueryProjectPropertiesContext>
 {
     /// <summary>
     /// A well-known context representing the project file as a whole.
@@ -84,7 +84,7 @@ public override int GetHashCode()
     /// Creates a <see cref="QueryProjectPropertiesContext"/> from a Project Query API
     /// <see cref="EntityIdentity"/>.
     /// </summary>
-    public static bool TryCreateFromEntityId(EntityIdentity id, [NotNullWhen(true)] out QueryProjectPropertiesContext? propertiesContext)
+    internal static bool TryCreateFromEntityId(EntityIdentity id, [NotNullWhen(true)] out QueryProjectPropertiesContext? propertiesContext)
     {
         if (id.TryGetValue(ProjectModelIdentityKeys.ProjectPath, out string? projectPath))
         {