From f905ef9750033a69d4c0693904e57295204c4d21 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 30 Sep 2021 12:30:18 -0700 Subject: [PATCH 01/21] Add info to logs so we can determine issue serializing options --- .../Portable/Options/SerializableOptionSet.cs | 62 +++++++++++++++++++ .../Remote/ServiceHub/Host/TestUtils.cs | 27 ++++++++ 2 files changed, 89 insertions(+) diff --git a/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs b/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs index 0644c73d84a4c..7e6df4e0ea8a7 100644 --- a/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs +++ b/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs @@ -11,6 +11,7 @@ using System.Threading; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Roslyn.Utilities; @@ -222,6 +223,66 @@ internal override IEnumerable GetChangedOptions(OptionSet? optionSet) return (languages, valuesBuilder); } + public string GetDebugString() + { + // NOTE: keep this in sync with Serialize below. + + using var _ = PooledStringBuilder.GetInstance(out var sb); + + var (languages, values) = this.GetLanguagesAndValuesToSerialize(includeValues: true); + + sb.AppendLine($"languages count: {languages.Count}"); + foreach (var language in languages.Order()) + { + Debug.Assert(RemoteSupportedLanguages.IsSupported(language)); + sb.AppendLine(language); + } + + sb.AppendLine(); + sb.AppendLine($"values count: {values.Count}"); + foreach (var (optionKey, (kind, value)) in values) + { + SerializeOptionKey(optionKey); + + sb.Append($"{kind}: "); + if (kind == OptionValueKind.Enum) + { + RoslynDebug.Assert(value != null); + sb.AppendLine(value.ToString()); + } + else if (kind is OptionValueKind.CodeStyleOption) + { + var codeStyleOption = (ICodeStyleOption)value; + sb.AppendLine(codeStyleOption.ToXElement().ToString()); + } + else if (kind is OptionValueKind.NamingStylePreferences) + { + var namingStylePreferences = (NamingStylePreferences)value; + sb.AppendLine(namingStylePreferences.CreateXElement().ToString()); + } + else + { + sb.AppendLine($"{value}"); + } + + sb.AppendLine(); + } + + sb.AppendLine(); + sb.AppendLine($"changed options count: {_changedOptionKeysSerializable.Count}"); + foreach (var changedKey in _changedOptionKeysSerializable.OrderBy(OptionKeyComparer.Instance)) + SerializeOptionKey(changedKey); + + return sb.ToString(); + + void SerializeOptionKey(OptionKey optionKey) + { + Debug.Assert(ShouldSerialize(optionKey)); + + sb.AppendLine($"{optionKey.Option.Name} {optionKey.Option.Feature} {optionKey.Option.IsPerLanguage} {optionKey.Language}"); + } + } + public void Serialize(ObjectWriter writer, CancellationToken cancellationToken) { // We serialize the following contents from this option set: @@ -230,6 +291,7 @@ public void Serialize(ObjectWriter writer, CancellationToken cancellationToken) // 3. Changed option keys. // NOTE: keep the serialization in sync with Deserialize method below. + // NOTE: keep this in sync with GetDebugString above. cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index 15189763359f7..dc2f5485df9c5 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Serialization; +using Microsoft.CodeAnalysis.Options; #if DEBUG using System.Diagnostics; @@ -72,6 +73,8 @@ internal static async Task AssertChecksumsAsync( var mismatch6 = await GetAssetFromAssetServiceAsync(allChecksumsFromRequest.Except(assetMapFromIncrementalSolution.Keys)).ConfigureAwait(false); AppendMismatch(mismatch6, "assets only in the request but not in incremental solution", sb); + AppendOptionSets(); + var result = sb.ToString(); if (result.Length > 0) { @@ -79,6 +82,30 @@ internal static async Task AssertChecksumsAsync( Debug.Fail("Differences detected in solution checksum: " + result); } + return; + + void AppendOptionSets() + { + var allOptionSets = new Dictionary(); + foreach (var list in new[] { mismatch1, mismatch2, mismatch3, mismatch4, mismatch5, mismatch6 }) + { + foreach (var (checksum, val) in list) + { + if (val is SerializableOptionSet optionSet) + allOptionSets[checksum] = optionSet; + } + } + + sb.AppendLine("Different option sets:"); + foreach (var (checksum, optionSet) in allOptionSets) + { + sb.AppendLine($"Checksum: {checksum}"); + sb.AppendLine("Options:"); + sb.AppendLine(optionSet.GetDebugString()); + sb.AppendLine(); + } + } + static void AppendMismatch(List> items, string title, StringBuilder stringBuilder) { if (items.Count == 0) From 8d64c123186e08dcc6ee4721ac4816f537cc9a42 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 30 Sep 2021 12:33:56 -0700 Subject: [PATCH 02/21] Simplify --- .../Remote/ServiceHub/Host/TestUtils.cs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index dc2f5485df9c5..bd2fd55aec046 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Serialization; using Microsoft.CodeAnalysis.Options; +using Roslyn.Utilities; #if DEBUG using System.Diagnostics; @@ -86,24 +87,20 @@ internal static async Task AssertChecksumsAsync( void AppendOptionSets() { - var allOptionSets = new Dictionary(); + var seenChecksums = new HashSet(); foreach (var list in new[] { mismatch1, mismatch2, mismatch3, mismatch4, mismatch5, mismatch6 }) { foreach (var (checksum, val) in list) { - if (val is SerializableOptionSet optionSet) - allOptionSets[checksum] = optionSet; + if (seenChecksums.Add(checksum) && val is SerializableOptionSet optionSet) + { + sb.AppendLine($"Checksum: {checksum}"); + sb.AppendLine("Options:"); + sb.AppendLine(optionSet.GetDebugString()); + sb.AppendLine(); + } } } - - sb.AppendLine("Different option sets:"); - foreach (var (checksum, optionSet) in allOptionSets) - { - sb.AppendLine($"Checksum: {checksum}"); - sb.AppendLine("Options:"); - sb.AppendLine(optionSet.GetDebugString()); - sb.AppendLine(); - } } static void AppendMismatch(List> items, string title, StringBuilder stringBuilder) From 2a8fae4b9a60244c650f00c9b9e2051e77a191a3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 30 Sep 2021 13:47:46 -0700 Subject: [PATCH 03/21] NRT --- src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs b/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs index 7e6df4e0ea8a7..5331beda8bd8b 100644 --- a/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs +++ b/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs @@ -252,11 +252,13 @@ public string GetDebugString() } else if (kind is OptionValueKind.CodeStyleOption) { + RoslynDebug.Assert(value != null); var codeStyleOption = (ICodeStyleOption)value; sb.AppendLine(codeStyleOption.ToXElement().ToString()); } else if (kind is OptionValueKind.NamingStylePreferences) { + RoslynDebug.Assert(value != null); var namingStylePreferences = (NamingStylePreferences)value; sb.AppendLine(namingStylePreferences.CreateXElement().ToString()); } From 5ba37e08f13039f15be46f52d8fe3073b31a5492 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Thu, 30 Sep 2021 18:45:44 -0700 Subject: [PATCH 04/21] Delete unnecessary code This was working around the old CPS wrapper IVsHierarchies which we simply don't do anymore. --- .../Def/Interactive/VsResetInteractive.cs | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/VisualStudio/Core/Def/Interactive/VsResetInteractive.cs b/src/VisualStudio/Core/Def/Interactive/VsResetInteractive.cs index e55001a37dc47..f26e64902a17d 100644 --- a/src/VisualStudio/Core/Def/Interactive/VsResetInteractive.cs +++ b/src/VisualStudio/Core/Def/Interactive/VsResetInteractive.cs @@ -161,36 +161,7 @@ private void GetProjectProperties( } internal Project GetProjectFromHierarchy(IVsHierarchy hierarchy) - => _workspace.CurrentSolution.Projects.FirstOrDefault(proj => ProjectIdMatchesHierarchy(_workspace, proj.Id, hierarchy)); - - private static bool ProjectIdMatchesHierarchy(VisualStudioWorkspace workspace, ProjectId projectId, IVsHierarchy hierarchy) - { - var hierarchyForProject = workspace.GetHierarchy(projectId); - - if (hierarchyForProject == null) - { - return false; - } - - if (hierarchyForProject == hierarchy) - { - return true; - } - - // For CPS, the hierarchy for the Roslyn project isn't the same as the one - // we get from Solution Explorer (it's a wrapper implementation), so we'll - // have to compare properties. - - hierarchyForProject.GetProperty((uint)VSConstants.VSITEMID.Root, (int)__VSHPROPID.VSHPROPID_Name, out var rawValue); - - if (rawValue is string projectName) - { - hierarchy.GetProperty((uint)VSConstants.VSITEMID.Root, (int)__VSHPROPID.VSHPROPID_Name, out rawValue); - return projectName == (rawValue as string); - } - - return false; - } + => _workspace.CurrentSolution.Projects.FirstOrDefault(proj => _workspace.GetHierarchy(proj.Id) == hierarchy); private static InteractiveHostPlatform? GetInteractiveHostPlatform(string targetFrameworkMoniker, Platform platform) { From bd8a2226094ffa3e4a8188f2d403de9b26944904 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 30 Sep 2021 19:42:17 -0700 Subject: [PATCH 05/21] Give each process a unique DB, not each workspace --- .../IPersistentStorageConfiguration.cs | 25 +++++++++++++------ .../Host/PersistentStorage/SolutionKey.cs | 12 +++------ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs index 4026e5d1aae7a..023bb71bd847e 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Immutable; using System.Composition; +using System.Diagnostics; using System.IO; using System.Linq; using Microsoft.CodeAnalysis.Host.Mef; @@ -58,20 +59,30 @@ private static string GetCacheDirectory() public string? TryGetStorageLocation(SolutionKey solutionKey) { - if (solutionKey.WorkspaceKind is not (WorkspaceKind.RemoteWorkspace or WorkspaceKind.RemoteTemporaryWorkspace or WorkspaceKind.Host)) - return null; - if (string.IsNullOrWhiteSpace(solutionKey.FilePath)) return null; // Ensure that each unique workspace kind for any given solution has a unique // folder to store their data in. - var cacheDirectory = GetCacheDirectory(); - var kind = StripInvalidPathChars(solutionKey.WorkspaceKind); - var hash = StripInvalidPathChars(Checksum.Create(solutionKey.FilePath).ToString()); + return Path.Combine( + GetCacheDirectory(), + SafeName(Process.GetCurrentProcess().MainModule.FileName), + SafeName(solutionKey.FilePath)); - return Path.Combine(cacheDirectory, kind, hash); + static string SafeName(string fullPath) + { + var fileName = Path.GetFileName(fullPath); + + // we don't want to build too long a path. So only take 16 chars of the text we started with. + // However, we want to avoid collisions, so ensure we also append a safe short piece of text + // that is based on the full text. + const int MaxLength = 20; + var prefix = fileName.Length > MaxLength ? fileName.Substring(0, MaxLength) : fileName; + var suffix = Checksum.Create(fullPath); + var fullName = $"{prefix}-{suffix}"; + return StripInvalidPathChars(fullName); + } static string StripInvalidPathChars(string val) { diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/SolutionKey.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/SolutionKey.cs index 4ea379d322872..8883d1fef2b1f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/SolutionKey.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/SolutionKey.cs @@ -17,20 +17,16 @@ namespace Microsoft.CodeAnalysis.Storage internal readonly struct SolutionKey { [DataMember(Order = 0)] - public readonly string? WorkspaceKind; - - [DataMember(Order = 1)] public readonly SolutionId Id; - [DataMember(Order = 2)] + [DataMember(Order = 1)] public readonly string? FilePath; - [DataMember(Order = 3)] + [DataMember(Order = 2)] public readonly bool IsPrimaryBranch; - public SolutionKey(string? workspaceKind, SolutionId id, string? filePath, bool isPrimaryBranch) + public SolutionKey(SolutionId id, string? filePath, bool isPrimaryBranch) { - WorkspaceKind = workspaceKind; Id = id; FilePath = filePath; IsPrimaryBranch = isPrimaryBranch; @@ -40,6 +36,6 @@ public static SolutionKey ToSolutionKey(Solution solution) => ToSolutionKey(solution.State); public static SolutionKey ToSolutionKey(SolutionState solutionState) - => new(solutionState.Workspace.Kind, solutionState.Id, solutionState.FilePath, solutionState.BranchId == solutionState.Workspace.PrimaryBranchId); + => new(solutionState.Id, solutionState.FilePath, solutionState.BranchId == solutionState.Workspace.PrimaryBranchId); } } From 1ddda1c23444a8b481f1fee1fe2a9f2e1fa323bf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 30 Sep 2021 23:07:57 -0700 Subject: [PATCH 06/21] Fix comment --- .../Host/PersistentStorage/IPersistentStorageConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs index 023bb71bd847e..fe6b481c861ea 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageConfiguration.cs @@ -74,7 +74,7 @@ static string SafeName(string fullPath) { var fileName = Path.GetFileName(fullPath); - // we don't want to build too long a path. So only take 16 chars of the text we started with. + // we don't want to build too long a path. So only take a portion of the text we started with. // However, we want to avoid collisions, so ensure we also append a safe short piece of text // that is based on the full text. const int MaxLength = 20; From d96002ab7d29938270b47eddb640ced9a72c9442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Matou=C5=A1ek?= Date: Fri, 1 Oct 2021 09:10:05 -0700 Subject: [PATCH 07/21] Avoid UI thread dependency in IAsyncServiceProvider.GetServiceAsync (#56884) --- .../Options/LocalUserRegistryOptionPersister.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/Options/LocalUserRegistryOptionPersister.cs b/src/VisualStudio/Core/Def/Implementation/Options/LocalUserRegistryOptionPersister.cs index 878fd0145e445..551c102147a9a 100644 --- a/src/VisualStudio/Core/Def/Implementation/Options/LocalUserRegistryOptionPersister.cs +++ b/src/VisualStudio/Core/Def/Implementation/Options/LocalUserRegistryOptionPersister.cs @@ -35,8 +35,11 @@ private LocalUserRegistryOptionPersister(RegistryKey registryKey) public static async Task CreateAsync(IAsyncServiceProvider provider) { // SLocalRegistry service is free-threaded -- see https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1408594. - var localRegistry = await provider.GetServiceAsync(throwOnFailure: true).ConfigureAwait(false); - Contract.ThrowIfFalse(ErrorHandler.Succeeded(localRegistry!.GetLocalRegistryRootEx((uint)__VsLocalRegistryType.RegType_UserSettings, out var rootHandle, out var rootPath))); + // Note: not using IAsyncServiceProvider.GetServiceAsync since the extension method might switch to UI thread. + // See https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1408619/. + var localRegistry = (ILocalRegistry4?)await provider.GetServiceAsync(typeof(SLocalRegistry)).ConfigureAwait(false); + Contract.ThrowIfNull(localRegistry); + Contract.ThrowIfFalse(ErrorHandler.Succeeded(localRegistry.GetLocalRegistryRootEx((uint)__VsLocalRegistryType.RegType_UserSettings, out var rootHandle, out var rootPath))); var handle = (__VsLocalRegistryRootHandle)rootHandle; Contract.ThrowIfTrue(string.IsNullOrEmpty(rootPath)); From 45dbd92cba3ef709eb7f64bf61a969082232cd06 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 10:18:29 -0700 Subject: [PATCH 08/21] Add telemetry to let us know how often our navto cache is benefitting the user. --- .../Portable/NavigateTo/NavigateToSearcher.cs | 30 +++++++++++++------ .../Compiler/Core/Log/FunctionId.cs | 2 ++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs b/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs index 54965a096acac..82cfca73558c2 100644 --- a/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs +++ b/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs @@ -113,7 +113,7 @@ internal async Task SearchAsync(CancellationToken cancellationToken) private async Task SearchAllProjectsAsync(bool isFullyLoaded, CancellationToken cancellationToken) { var orderedProjects = GetOrderedProjectsToProcess(); - var (itemsReported, projectResults) = await ProcessProjectsAsync(orderedProjects, isFullyLoaded, cancellationToken).ConfigureAwait(false); + var (foundItems, projectResults) = await ProcessProjectsAsync(orderedProjects, isFullyLoaded, cancellationToken).ConfigureAwait(false); // If we're fully loaded then we're done at this point. All the searches would have been against the latest // computed data and we don't need to do anything else. @@ -123,14 +123,24 @@ private async Task SearchAllProjectsAsync(bool isFullyLoaded, CancellationToken // We weren't fully loaded *but* we reported some items to the user, then consider that good enough for now. // The user will have some results they can use, and (in the case that we actually examined the cache for // data) we will tell the user that the results may be incomplete/inaccurate and they should try again soon. - if (itemsReported > 0) + if (foundItems) return; // We didn't have any items reported *and* we weren't fully loaded. If it turns out that some of our // projects were using cached data then we can try searching them again, but this tell them to use the // latest data. The ensures the user at least gets some result instead of nothing. var projectsUsingCache = projectResults.SelectAsArray(t => t.location == NavigateToSearchLocation.Cache, t => t.project); - await ProcessProjectsAsync(ImmutableArray.Create(projectsUsingCache), isFullyLoaded: true, cancellationToken).ConfigureAwait(false); + if (projectsUsingCache.Length == 0) + return; + + var (foundFullItems, _) = await ProcessProjectsAsync(ImmutableArray.Create(projectsUsingCache), isFullyLoaded: true, cancellationToken).ConfigureAwait(false); + + // Report a telemetry even to track if we found uncached items after failing to find cached items. + // In practice if we see that we are always finding uncached items, then it's likely something + // has broken in the caching system since we would expect to normally find values there. Specifically + // we expect: foundFullItems <<< not foundFullItems. + + Logger.Log(FunctionId.NavigateTo_CacheItemsMiss, KeyValueLogMessage.Create(m => m["FoundFullItems"] = foundFullItems)); } /// @@ -212,7 +222,7 @@ private ImmutableArray GetPriorityDocuments(Project project) return result.ToImmutable(); } - private async Task<(int itemsReported, ImmutableArray<(Project project, NavigateToSearchLocation location)>)> ProcessProjectsAsync( + private async Task<(bool foundItems, ImmutableArray<(Project project, NavigateToSearchLocation location)>)> ProcessProjectsAsync( ImmutableArray> orderedProjects, bool isFullyLoaded, CancellationToken cancellationToken) { await _progress.AddItemsAsync(orderedProjects.Sum(p => p.Length), cancellationToken).ConfigureAwait(false); @@ -221,18 +231,20 @@ private ImmutableArray GetPriorityDocuments(Project project) var seenItems = new HashSet(NavigateToSearchResultComparer.Instance); foreach (var projectGroup in orderedProjects) - result.AddRange(await Task.WhenAll(projectGroup.Select(p => Task.Run(() => SearchAsync(p, isFullyLoaded, seenItems, cancellationToken)))).ConfigureAwait(false)); + { + var allTasks = projectGroup.Select(p => Task.Run(async () => (p, await SearchAsync(p, isFullyLoaded, seenItems, cancellationToken).ConfigureAwait(false)))); + result.AddRange(await Task.WhenAll(allTasks).ConfigureAwait(false)); + } - return (seenItems.Count, result.ToImmutable()); + return (foundItems: seenItems.Count > 0, result.ToImmutable()); } - private async Task<(Project project, NavigateToSearchLocation location)> SearchAsync( + private async Task SearchAsync( Project project, bool isFullyLoaded, HashSet seenItems, CancellationToken cancellationToken) { try { - var location = await SearchCoreAsync(project, isFullyLoaded, seenItems, cancellationToken).ConfigureAwait(false); - return (project, location); + return await SearchCoreAsync(project, isFullyLoaded, seenItems, cancellationToken).ConfigureAwait(false); } finally { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index 1b7e3c9e4970a..ee79f6ca9045c 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -530,5 +530,7 @@ internal enum FunctionId LSP_FindDocumentInWorkspace = 494, SuggestedActions_GetSuggestedActionsAsync = 500, + + NavigateTo_CacheItemsMiss = 510, } } From 84ecf57505a0268c76a932c45bae0313c36ad62f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 10:23:27 -0700 Subject: [PATCH 09/21] Add docs --- src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs b/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs index 82cfca73558c2..ae23e33a7a3eb 100644 --- a/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs +++ b/src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs @@ -230,6 +230,12 @@ private ImmutableArray GetPriorityDocuments(Project project) using var _ = ArrayBuilder<(Project project, NavigateToSearchLocation location)>.GetInstance(out var result); var seenItems = new HashSet(NavigateToSearchResultComparer.Instance); + + // Process each group one at a time. However, in each group process all projects in parallel to get results + // as quickly as possible. The net effect of this is that we will search the active doc immediately, then + // the open docs in parallel, then the rest of the projects after that. Because the active/open docs should + // be a far smaller set, those results should come in almost immediately in a prioritized fashion, with the + // rest of the results following soon after as best as we can find them. foreach (var projectGroup in orderedProjects) { var allTasks = projectGroup.Select(p => Task.Run(async () => (p, await SearchAsync(p, isFullyLoaded, seenItems, cancellationToken).ConfigureAwait(false)))); From 6c65f4a4d99e3419689552d8939cad67d1ebbc46 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Sat, 2 Oct 2021 03:48:50 +1000 Subject: [PATCH 10/21] Don't offer to remove variable in situations where we would crash (#56902) --- .../RemoveUnusedVariableTests.cs | 46 +++++++++++++++++++ ...harpRemoveUnusedVariableCodeFixProvider.cs | 8 ++++ ...ractRemoveUnusedVariableCodeFixProvider.cs | 23 ++++++++-- ...asicRemoveUnusedVariableCodeFixProvider.vb | 4 ++ 4 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs b/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs index 878b7edae79e4..833ca30dd8ffd 100644 --- a/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs +++ b/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs @@ -723,5 +723,51 @@ await TestAsync(@" @" ", TestOptions.Regular); } + + [WorkItem(49827, "https://github.com/dotnet/roslyn/issues/49827")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] + public async Task RemoveUnusedVariableJointDeclaredInForStatementInsideIfStatement() + { + await TestInRegularAndScriptAsync( +@"class Class +{ + void Method() + { + if (true) + for(int i = 0[|, j = 0|]; i < 1; i++) + { + + } + } +}", +@"class Class +{ + void Method() + { + if (true) + for(int i = 0; i < 1; i++) + { + + } + } +}"); + } + + [WorkItem(49827, "https://github.com/dotnet/roslyn/issues/49827")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] + public async Task DontCrashOnDeclarationInsideIfStatement() + { + await TestMissingInRegularAndScriptAsync( +@"class Class +{ + void Method(bool test) + { + if (test [|and test|]) + { + + } + } +}"); + } } } diff --git a/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs b/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs index 42333bd82c7bc..d79eba771df03 100644 --- a/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs @@ -69,5 +69,13 @@ protected override void RemoveOrReplaceNode(SyntaxEditor editor, SyntaxNode node protected override SeparatedSyntaxList GetVariables(LocalDeclarationStatementSyntax localDeclarationStatement) => localDeclarationStatement.Declaration.Variables; + + protected override bool ShouldOfferFixForLocalDeclaration(ISyntaxFactsService syntaxFacts, LocalDeclarationStatementSyntax localDeclaration) + { + // Local declarations must be parented by an executable block, or global statement, otherwise + // removing them would be invalid (and more than likely crash the fixer) + return localDeclaration.Parent is GlobalStatementSyntax || + syntaxFacts.IsExecutableBlock(localDeclaration.Parent); + } } } diff --git a/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs b/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs index 3fa2f3d8aba05..c8deec98e9d0d 100644 --- a/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs +++ b/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs @@ -34,13 +34,25 @@ internal abstract class AbstractRemoveUnusedVariableCodeFixProvider GetVariables(TLocalDeclarationStatement localDeclarationStatement); + protected abstract bool ShouldOfferFixForLocalDeclaration(ISyntaxFactsService syntaxFacts, TLocalDeclarationStatement node); + internal sealed override CodeFixCategory CodeFixCategory => CodeFixCategory.CodeQuality; - public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { var diagnostic = context.Diagnostics.First(); - context.RegisterCodeFix(new MyCodeAction(c => FixAsync(context.Document, diagnostic, c)), diagnostic); - return Task.CompletedTask; + + var document = context.Document; + var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var node = root.FindNode(diagnostic.Location.SourceSpan); + + var syntaxFacts = document.GetRequiredLanguageService(); + var localDeclaration = FindLocalDeclaration(node); + + if (localDeclaration is null || ShouldOfferFixForLocalDeclaration(syntaxFacts, localDeclaration)) + { + context.RegisterCodeFix(new MyCodeAction(c => FixAsync(context.Document, diagnostic, c)), diagnostic); + } } protected override async Task FixAllAsync(Document document, ImmutableArray diagnostics, SyntaxEditor syntaxEditor, CancellationToken cancellationToken) @@ -111,11 +123,14 @@ protected override async Task FixAllAsync(Document document, ImmutableArray(); + var localDeclaration = FindLocalDeclaration(node); var removeOptions = CreateSyntaxRemoveOptions(localDeclaration, syntaxFacts); editor.RemoveNode(node, removeOptions); } + private static TLocalDeclarationStatement FindLocalDeclaration(SyntaxNode node) + => node.GetAncestorOrThis(); + private static SyntaxRemoveOptions CreateSyntaxRemoveOptions(TLocalDeclarationStatement localDeclaration, ISyntaxFactsService syntaxFacts) { var removeOptions = SyntaxGenerator.DefaultRemoveOptions; diff --git a/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb b/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb index 853fb12e6fef1..6575653a10701 100644 --- a/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb +++ b/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb @@ -45,5 +45,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedVariable Protected Overrides Function GetVariables(localDeclarationStatement As LocalDeclarationStatementSyntax) As SeparatedSyntaxList(Of SyntaxNode) Return localDeclarationStatement.Declarators End Function + + Protected Overrides Function ShouldOfferFixForLocalDeclaration(syntaxFactsService As ISyntaxFactsService, localDeclaration As LocalDeclarationStatementSyntax) As Boolean + Return True + End Function End Class End Namespace From 327732d7ce51ec5f791630f62690b16e76bc6f43 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 10:54:18 -0700 Subject: [PATCH 11/21] Obsolete the legacy persistence service. --- .../Test/Preview/PreviewWorkspaceTests.cs | 5 +++-- .../IncrementalAnalyzerRunner.cs | 4 ++-- .../FindReferencesBenchmarks.cs | 4 ++-- .../IdeCoreBenchmarks/NavigateToBenchmarks.cs | 4 ++-- .../Storage/LegacyPersistentStorageService.cs | 22 +++++++++---------- .../Workspace/Host/HostWorkspaceServices.cs | 8 ------- .../IPersistentStorageService.cs | 7 ++++-- .../MSBuildTest/MSBuildWorkspaceTests.cs | 1 - 8 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index 90babe247843d..8349b589ca9cb 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -26,6 +26,7 @@ using Roslyn.Test.Utilities; using Roslyn.Utilities; using Xunit; +using Microsoft.CodeAnalysis.Storage; namespace Microsoft.CodeAnalysis.Editor.UnitTests.Preview { @@ -126,9 +127,9 @@ public async Task TestPreviewServices() var service = previewWorkspace.Services.GetService(); Assert.IsType(service); - var persistentService = previewWorkspace.Services.GetRequiredService(); + var persistentService = previewWorkspace.Services.GetPersistentStorageService(previewWorkspace.CurrentSolution.Options); - await using var storage = await persistentService.GetStorageAsync(previewWorkspace.CurrentSolution, CancellationToken.None); + await using var storage = await persistentService.GetStorageAsync(SolutionKey.ToSolutionKey(previewWorkspace.CurrentSolution), checkBranchId: true, CancellationToken.None); Assert.IsType(storage); } diff --git a/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs b/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs index 59a27b0e8093e..429854bdd060b 100644 --- a/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs +++ b/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs @@ -54,8 +54,8 @@ public async Task RunAsync(CancellationToken cancellationToken) if (usePersistentStorage) { - var persistentStorageService = _workspace.Services.GetRequiredService(); - await using var persistentStorage = await persistentStorageService.GetStorageAsync(_workspace.CurrentSolution, cancellationToken).ConfigureAwait(false); + var persistentStorageService = _workspace.Services.GetPersistentStorageService(_workspace.CurrentSolution.Options); + await using var persistentStorage = await persistentStorageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), checkBranchId: true, cancellationToken).ConfigureAwait(false); if (persistentStorage is NoOpPersistentStorage) { throw new InvalidOperationException("Benchmark is not configured to use persistent storage."); diff --git a/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs b/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs index 6346e7b0e1be2..a5520f540e1df 100644 --- a/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs +++ b/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs @@ -71,11 +71,11 @@ public async Task RunFindReferences() // Force a storage instance to be created. This makes it simple to go examine it prior to any operations we // perform, including seeing how big the initial string table is. - var storageService = workspace.Services.GetService(); + var storageService = workspace.Services.GetPersistentStorageService(workspace.CurrentSolution.Options); if (storageService == null) throw new ArgumentException("Couldn't get storage service"); - using (var storage = await storageService.GetStorageAsync(workspace.CurrentSolution, CancellationToken.None)) + using (var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(workspace.CurrentSolution), checkBranchId: true, CancellationToken.None)) { Console.WriteLine(); } diff --git a/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs b/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs index ee641be8e5f79..73f3d921ab274 100644 --- a/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs +++ b/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs @@ -73,11 +73,11 @@ public async Task RunNavigateTo() // Force a storage instance to be created. This makes it simple to go examine it prior to any operations we // perform, including seeing how big the initial string table is. - var storageService = workspace.Services.GetService(); + var storageService = workspace.Services.GetPersistentStorageService(workspace.CurrentSolution.Options); if (storageService == null) throw new ArgumentException("Couldn't get storage service"); - using (var storage = await storageService.GetStorageAsync(workspace.CurrentSolution, CancellationToken.None)) + using (var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(workspace.CurrentSolution), checkBranchId: true, CancellationToken.None)) { Console.WriteLine(); } diff --git a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs index ef48f605430bc..d184149668abf 100644 --- a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs @@ -12,10 +12,10 @@ namespace Microsoft.CodeAnalysis.Storage { /// - /// Implements public . - /// Internally, it is preferred to work directly with , - /// which can be retrieved by . + /// Obsolete. Roslyn no longer supports a mechanism to perform arbitrary persistence of data. If such functionality + /// is needed, consumers are resonsible for providing it themselves with whatever semantics are needed. /// + [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] internal sealed class LegacyPersistentStorageService : IPersistentStorageService { [ExportWorkspaceServiceFactory(typeof(IPersistentStorageService)), Shared] @@ -28,19 +28,17 @@ public Factory() } public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) - => new LegacyPersistentStorageService(workspaceServices); + => new LegacyPersistentStorageService(); } - private readonly HostWorkspaceServices _workspaceServices; - - public LegacyPersistentStorageService(HostWorkspaceServices workspaceServices) - => _workspaceServices = workspaceServices; + public LegacyPersistentStorageService() + { + } public IPersistentStorage GetStorage(Solution solution) - => GetStorageAsync(solution, CancellationToken.None).AsTask().GetAwaiter().GetResult(); + => NoOpPersistentStorage.GetOrThrow(throwOnFailure: false); - public async ValueTask GetStorageAsync(Solution solution, CancellationToken cancellationToken) - => await _workspaceServices.GetPersistentStorageService(solution.Options). - GetStorageAsync(SolutionKey.ToSolutionKey(solution), checkBranchId: true, cancellationToken).ConfigureAwait(false); + public ValueTask GetStorageAsync(Solution solution, CancellationToken cancellationToken) + => new(NoOpPersistentStorage.GetOrThrow(throwOnFailure: false)); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs b/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs index c1c8e44a0642b..a626a40505184 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs @@ -47,14 +47,6 @@ public TWorkspaceService GetRequiredService() where TWorkspac return service; } - /// - /// A service for storing information across that can be retrieved in a separate process. - /// - public virtual IPersistentStorageService PersistentStorage - { - get { return this.GetRequiredService(); } - } - /// /// A service for storing information in a temporary location that only lasts for the duration of the process. /// diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs index c1444874425a2..00c50864b74dc 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs @@ -9,12 +9,15 @@ namespace Microsoft.CodeAnalysis.Host { /// - /// This service allows you to persist information relative to solution, projects and documents. + /// Obsolete. Roslyn no longer supports a mechanism to perform arbitrary persistence of data. If such functionality + /// is needed, consumers are resonsible for providing it themselves with whatever semantics are needed. /// + [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] public interface IPersistentStorageService : IWorkspaceService { - [Obsolete("Use GetStorageAsync instead", error: false)] + [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] IPersistentStorage GetStorage(Solution solution); + [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] ValueTask GetStorageAsync(Solution solution, CancellationToken cancellationToken = default); } } diff --git a/src/Workspaces/MSBuildTest/MSBuildWorkspaceTests.cs b/src/Workspaces/MSBuildTest/MSBuildWorkspaceTests.cs index 15f0b35c9a15d..a33c4f79cc080 100644 --- a/src/Workspaces/MSBuildTest/MSBuildWorkspaceTests.cs +++ b/src/Workspaces/MSBuildTest/MSBuildWorkspaceTests.cs @@ -43,7 +43,6 @@ public void TestCreateMSBuildWorkspace() Assert.NotNull(workspace.Services.Workspace); Assert.Equal(workspace, workspace.Services.Workspace); Assert.NotNull(workspace.Services.HostServices); - Assert.NotNull(workspace.Services.PersistentStorage); Assert.NotNull(workspace.Services.TemporaryStorage); Assert.NotNull(workspace.Services.TextFactory); } From 46e91dc46b97f9cf1adaf19587497c0937096007 Mon Sep 17 00:00:00 2001 From: Fred Silberberg Date: Fri, 1 Oct 2021 11:34:54 -0700 Subject: [PATCH 12/21] Add test verifying VB error ranges (#56772) Also adjusted errors that are currently out of the error range. Fixes https://github.com/dotnet/roslyn/issues/54267. --- .../VisualBasicCompilerDiagnosticAnalyzer.vb | 2 +- .../VisualBasic/Portable/Errors/Errors.vb | 15 ++-- .../Test/CommandLine/CommandLineTests.vb | 2 +- .../Semantic/Diagnostics/DiagnosticTests.vb | 72 ++++++++++++++++++- .../AttributeTests_UnmanagedCallersOnly.vb | 8 +-- .../VisualBasicErrorFactsGenerator/Program.vb | 4 +- 6 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilerDiagnosticAnalyzer.vb b/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilerDiagnosticAnalyzer.vb index 3b43c37d4d920..57743107a0bc3 100644 --- a/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilerDiagnosticAnalyzer.vb +++ b/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilerDiagnosticAnalyzer.vb @@ -31,7 +31,7 @@ Namespace Microsoft.CodeAnalysis.Diagnostics.VisualBasic Continue For End If - If errorCode > ERRID.ERR_None AndAlso errorCode < ERRID.ERRWRN_NextAvailable Then + If errorCode > ERRID.ERR_None AndAlso errorCode < ERRID.WRN_NextAvailable Then builder.Add(errorCode) End If Next diff --git a/src/Compilers/VisualBasic/Portable/Errors/Errors.vb b/src/Compilers/VisualBasic/Portable/Errors/Errors.vb index ace7eb4837eb2..9cd4e9a6c63c4 100644 --- a/src/Compilers/VisualBasic/Portable/Errors/Errors.vb +++ b/src/Compilers/VisualBasic/Portable/Errors/Errors.vb @@ -1760,6 +1760,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic ERR_BadAbstractStaticMemberAccess = 37314 ERR_UnimplementedSharedMember = 37315 + ERR_UnmanagedCallersOnlyNotSupported = 37316 + ERR_MultipleAnalyzerConfigsInSameDir = 37317 + ERR_StdInOptionProvidedButConsoleInputIsNotRedirected = 37318 + + ERR_NextAvailable = 37319 + '// WARNINGS BEGIN HERE WRN_UseOfObsoleteSymbol2 = 40000 WRN_InvalidOverrideDueToTupleNames2 = 40001 @@ -1958,8 +1964,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic 'WRN_PDBConstantStringValueTooLong = 42363 we gave up on this warning. See comments in commonCompilation.Emit() WRN_ReturnTypeAttributeOnWriteOnlyProperty = 42364 - ERR_UnmanagedCallersOnlyNotSupported = 42365 - WRN_InvalidVersionFormat = 42366 WRN_MainIgnored = 42367 WRN_EmptyPrefixAndXmlnsLocalName = 42368 @@ -1983,7 +1987,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic WRN_Experimental = 42380 WRN_AttributeNotSupportedInVB = 42381 - ERR_MultipleAnalyzerConfigsInSameDir = 42500 WRN_GeneratorFailedDuringInitialization = 42501 WRN_GeneratorFailedDuringGeneration = 42502 WRN_AnalyzerReferencesFramework = 42503 @@ -1992,13 +1995,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic WRN_CallerArgumentExpressionAttributeHasInvalidParameterName = 42505 ' // AVAILABLE 42600 - 49998 - ERRWRN_NextAvailable = 42600 + WRN_NextAvailable = 42600 '// HIDDENS AND INFOS BEGIN HERE HDN_UnusedImportClause = 50000 HDN_UnusedImportStatement = 50001 INF_UnableToLoadSomeTypesInAnalyzer = 50002 + HDN_NextAvailable = 50003 + ' // AVAILABLE 50003 - 54999 ' Adding diagnostic arguments from resx file @@ -2015,7 +2020,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic IDS_LangVersions = 56010 IDS_ToolName = 56011 - ERR_StdInOptionProvidedButConsoleInputIsNotRedirected = 56032 + IDS_NextAvailable = 56012 ' Feature codes FEATURE_AutoProperties diff --git a/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb b/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb index 56c4e341eb8f8..ad45528663b8b 100644 --- a/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb +++ b/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb @@ -216,7 +216,7 @@ dotnet_diagnostic.cs0169.severity = suppress" Dim exitCode = cmd.Run(outWriter) Assert.Equal(1, exitCode) Assert.Equal( - $"vbc : error BC42500: Multiple analyzer config files cannot be in the same directory ('{dir.Path}').", + $"vbc : error BC37317: Multiple analyzer config files cannot be in the same directory ('{dir.Path}').", outWriter.ToString().TrimEnd()) End Sub diff --git a/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/DiagnosticTests.vb b/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/DiagnosticTests.vb index 478a08d9be080..b26a37fb77b41 100644 --- a/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/DiagnosticTests.vb +++ b/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/DiagnosticTests.vb @@ -21,7 +21,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests ERRID.Unknown, ERRID.ERR_None, ERRID.ERR_CannotUseGenericBaseTypeAcrossAssemblyBoundaries, ' Not reported. See ImportsBinder.ShouldReportUseSiteErrorForAlias. - ERRID.ERRWRN_NextAvailable + ERRID.ERR_NextAvailable, + ERRID.WRN_NextAvailable, + ERRID.HDN_NextAvailable, + ERRID.IDS_NextAvailable } For Each id As ERRID In [Enum].GetValues(GetType(ERRID)) If Array.IndexOf(excludedIds, id) >= 0 Then @@ -66,6 +69,73 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests Next End Sub + + Public Sub ErrorMessagesWithinCorrectRanges() + Const WarningsStart = ERRID.WRN_UseOfObsoleteSymbol2 + Const HiddenInfoStart = ERRID.HDN_UnusedImportClause + Const IdsStart = ERRID.IDS_ProjectSettingsLocationName + Const FeatureStart = ERRID.FEATURE_AutoProperties + + Dim legacyWarningsInErrorRange = {ERRID.WRN_BadSwitch, ERRID.WRN_NoConfigInResponseFile, ERRID.FTL_InvalidInputFileName, ERRID.WRN_IgnoreModuleManifest, ERRID.WRN_BadUILang} + + For Each errObj In [Enum].GetValues(GetType(ERRID)) + Dim err = DirectCast(errObj, ERRID) + Dim errString = err.ToString() + Select Case err + Case ERRID.Void, ERRID.Unknown + Continue For + + Case <= ERRID.ERR_NextAvailable + If legacyWarningsInErrorRange.Contains(err) Then + Continue For + End If + + Assert.True(errString.StartsWith("ERR_"), GetErrorMessage(errString)) + + Case < WarningsStart + Assert.True(False, GetErrorMessage(errString)) + + Case WarningsStart To ERRID.WRN_NextAvailable + Assert.True(errString.StartsWith("WRN_"), GetErrorMessage(errString)) + + Case < HiddenInfoStart + Assert.True(False, GetErrorMessage(errString)) + + Case HiddenInfoStart To ERRID.HDN_NextAvailable + Assert.True(errString.StartsWith("HDN_") OrElse errString.StartsWith("INF_"), + GetErrorMessage(errString)) + + Case < IdsStart + Assert.True(False, GetErrorMessage(errString)) + + Case IdsStart to ERRID.IDS_NextAvailable + Assert.True(errString.StartsWith("IDS_"), GetErrorMessage(errString)) + + Case < FeatureStart + Assert.True(False, GetErrorMessage(errString)) + + Case >= FeatureStart + Assert.True(errString.StartsWith("FEATURE_"), GetErrorMessage(errString)) + End Select + Next + + End Sub + + Private Shared Function GetErrorMessage(str As String) As String + If str.StartsWith("ERR_") Then + Return $"Error {str} should use {ERRID.ERR_NextAvailable} and increment the value." + ElseIf str.StartsWith("WRN_") Then + Return $"Warning {str} should use {ERRID.WRN_NextAvailable} and increment the value." + ElseIf str.StartsWith("HDN_") OrElse str.StartsWith("INF_") Then + Return $"Hidden or info diagnostic {str} should use {ERRID.HDN_NextAvailable} and increment the value." + ElseIf str.StartsWith("IDS_") Then + Return $"Id {str} should use {ERRID.IDS_NextAvailable} and increment the value." + ElseIf str.StartsWith("FEATURE_") Then + Return $"Feature code should go at the end of {NameOf(ERRID)}" + Else + Return $"{str} does not start with an approved prefix. Use ERR_ for errors, WRN_ for warnings, HDN_/INF_ for hidden or info diagnostics, IDS_ for ids, and FEATURE_ for feature codes" + End If + End Function End Class End Namespace diff --git a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/AttributeTests_UnmanagedCallersOnly.vb b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/AttributeTests_UnmanagedCallersOnly.vb index 6e5fe1199ef68..a2890824bbe67 100644 --- a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/AttributeTests_UnmanagedCallersOnly.vb +++ b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/AttributeTests_UnmanagedCallersOnly.vb @@ -75,16 +75,16 @@ End Class Dim comp = CreateCompilation(source, references:={NetCoreApp.SystemRuntimeInteropServices}, targetFramework:=TargetFramework.NetCoreApp) comp.AssertTheseDiagnostics( ~~~~~~~~~~~~~~~~~~~~ -BC42365: 'UnmanagedCallersOnly' attribute is not supported. +BC37316: 'UnmanagedCallersOnly' attribute is not supported. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -BC42365: 'UnmanagedCallersOnly' attribute is not supported. +BC37316: 'UnmanagedCallersOnly' attribute is not supported. ~~~~~~~~~~~~~~~~~~~~ -BC42365: 'UnmanagedCallersOnly' attribute is not supported. +BC37316: 'UnmanagedCallersOnly' attribute is not supported. ~~~~~~~~~~~~~~~~~~~~ ]]>) diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicErrorFactsGenerator/Program.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicErrorFactsGenerator/Program.vb index 66caf37613794..5c643315803c4 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicErrorFactsGenerator/Program.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicErrorFactsGenerator/Program.vb @@ -27,7 +27,9 @@ Friend Module Program Dim infoCodeNames As New List(Of String) Dim hiddenCodeNames As New List(Of String) For Each line In From l In File.ReadAllLines(inputPath) Select l.Trim - If line.StartsWith("WRN_", StringComparison.OrdinalIgnoreCase) Then + If (line.Contains("_NextAvailable", StringComparison.OrdinalIgnoreCase)) Then + Continue For + ElseIf line.StartsWith("WRN_", StringComparison.OrdinalIgnoreCase) Then warningCodeNames.Add(line.Substring(0, line.IndexOf(" "c))) ElseIf line.StartsWith("FTL_", StringComparison.OrdinalIgnoreCase) Then fatalCodeNames.Add(line.Substring(0, line.IndexOf(" "c))) From 7e02f60ae9b7a2c26db9b452d2cc3e98a730439e Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Fri, 1 Oct 2021 12:34:48 -0700 Subject: [PATCH 13/21] Update src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs Co-authored-by: Joey Robichaud --- .../Core/Portable/Storage/LegacyPersistentStorageService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs index d184149668abf..55b841b24e2b6 100644 --- a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Storage { /// /// Obsolete. Roslyn no longer supports a mechanism to perform arbitrary persistence of data. If such functionality - /// is needed, consumers are resonsible for providing it themselves with whatever semantics are needed. + /// is needed, consumers are responsible for providing it themselves with whatever semantics are needed. /// [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] internal sealed class LegacyPersistentStorageService : IPersistentStorageService From 87c72ca300728a1e5a1d7c1cf4bde22787a551f4 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Fri, 1 Oct 2021 12:34:52 -0700 Subject: [PATCH 14/21] Update src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs Co-authored-by: Joey Robichaud --- .../Host/PersistentStorage/IPersistentStorageService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs index 00c50864b74dc..8fa3356382d82 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs @@ -10,7 +10,7 @@ namespace Microsoft.CodeAnalysis.Host { /// /// Obsolete. Roslyn no longer supports a mechanism to perform arbitrary persistence of data. If such functionality - /// is needed, consumers are resonsible for providing it themselves with whatever semantics are needed. + /// is needed, consumers are responsible for providing it themselves with whatever semantics are needed. /// [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] public interface IPersistentStorageService : IWorkspaceService From 50c93389c1d41d72425a765e9414b3491400fdc3 Mon Sep 17 00:00:00 2001 From: Ankita Khera Date: Fri, 1 Oct 2021 12:54:09 -0700 Subject: [PATCH 15/21] added fatal error reporting --- .../Core.Wpf/InlineHints/InlineHintsTagger.cs | 85 ++++++++++--------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsTagger.cs b/src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsTagger.cs index e83bef7bdda36..572183444d060 100644 --- a/src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsTagger.cs +++ b/src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsTagger.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Composition; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; +using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.InlineHints; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -13,6 +14,7 @@ using Microsoft.VisualStudio.Text.Editor; using Microsoft.VisualStudio.Text.Formatting; using Microsoft.VisualStudio.Text.Tagging; +using Roslyn.Utilities; using static System.Windows.Forms.VisualStyles.VisualStyleElement; namespace Microsoft.CodeAnalysis.Editor.InlineHints @@ -111,57 +113,64 @@ private TextFormattingRunProperties Format public IEnumerable> GetTags(NormalizedSnapshotSpanCollection spans) { - if (spans.Count == 0) + try { - return Array.Empty>(); - } - - var snapshot = spans[0].Snapshot; - if (_cache.Count == 0 || snapshot != _cacheSnapshot) - { - // Calculate UI elements - _cache.Clear(); - _cacheSnapshot = snapshot; + if (spans.Count == 0) + { + return Array.Empty>(); + } - // Calling into the InlineParameterNameHintsDataTaggerProvider which only responds with the current - // active view and disregards and requests for tags not in that view - var fullSpan = new SnapshotSpan(snapshot, 0, snapshot.Length); - var tags = _tagAggregator.GetTags(new NormalizedSnapshotSpanCollection(fullSpan)); - foreach (var tag in tags) + var snapshot = spans[0].Snapshot; + if (_cache.Count == 0 || snapshot != _cacheSnapshot) { - // Gets the associated span from the snapshot span and creates the IntraTextAdornmentTag from the data - // tags. Only dealing with the dataTagSpans if the count is 1 because we do not see a multi-buffer case - // occuring - var dataTagSpans = tag.Span.GetSpans(snapshot); - if (dataTagSpans.Count == 1) + // Calculate UI elements + _cache.Clear(); + _cacheSnapshot = snapshot; + + // Calling into the InlineParameterNameHintsDataTaggerProvider which only responds with the current + // active view and disregards and requests for tags not in that view + var fullSpan = new SnapshotSpan(snapshot, 0, snapshot.Length); + var tags = _tagAggregator.GetTags(new NormalizedSnapshotSpanCollection(fullSpan)); + foreach (var tag in tags) { - _cache.Add((tag, tagSpan: null)); + // Gets the associated span from the snapshot span and creates the IntraTextAdornmentTag from the data + // tags. Only dealing with the dataTagSpans if the count is 1 because we do not see a multi-buffer case + // occuring + var dataTagSpans = tag.Span.GetSpans(snapshot); + if (dataTagSpans.Count == 1) + { + _cache.Add((tag, tagSpan: null)); + } } } - } - var document = snapshot.GetOpenDocumentInCurrentContextWithChanges(); - var classify = document?.Project.Solution.Options.GetOption(InlineHintsOptions.ColorHints, document?.Project.Language) ?? false; + var document = snapshot.GetOpenDocumentInCurrentContextWithChanges(); + var classify = document?.Project.Solution.Options.GetOption(InlineHintsOptions.ColorHints, document?.Project.Language) ?? false; - var selectedSpans = new List>(); - for (var i = 0; i < _cache.Count; i++) - { - var tagSpan = _cache[i].mappingTagSpan.Span.GetSpans(snapshot)[0]; - if (spans.IntersectsWith(tagSpan)) + var selectedSpans = new List>(); + for (var i = 0; i < _cache.Count; i++) { - if (_cache[i].tagSpan is not { } hintTagSpan) + var tagSpan = _cache[i].mappingTagSpan.Span.GetSpans(snapshot)[0]; + if (spans.IntersectsWith(tagSpan)) { - var parameterHintUITag = InlineHintsTag.Create( - _cache[i].mappingTagSpan.Tag.Hint, Format, _textView, tagSpan, _taggerProvider, _formatMap, classify); - hintTagSpan = new TagSpan(tagSpan, parameterHintUITag); - _cache[i] = (_cache[i].mappingTagSpan, hintTagSpan); + if (_cache[i].tagSpan is not { } hintTagSpan) + { + var parameterHintUITag = InlineHintsTag.Create( + _cache[i].mappingTagSpan.Tag.Hint, Format, _textView, tagSpan, _taggerProvider, _formatMap, classify); + hintTagSpan = new TagSpan(tagSpan, parameterHintUITag); + _cache[i] = (_cache[i].mappingTagSpan, hintTagSpan); + } + + selectedSpans.Add(hintTagSpan); } - - selectedSpans.Add(hintTagSpan); } - } - return selectedSpans; + return selectedSpans; + } + catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e)) + { + throw ExceptionUtilities.Unreachable; + } } public void Dispose() From 72a99a8279c9ccbb2ef3a99d5a8bf116a14ebf3b Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 1 Oct 2021 13:09:15 -0700 Subject: [PATCH 16/21] Unskip AttributeTests.GenericAttributeParameter_01 (#56915) --- src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs index 5b9debaaf394e..a97bc40ba8899 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs @@ -10291,7 +10291,7 @@ class Program2 { } Diagnostic(ErrorCode.ERR_BadAttributeArgument, "default(T2)").WithLocation(10, 22)); } - [ConditionalFact(typeof(CoreClrOnly), AlwaysSkip = "https://github.com/dotnet/roslyn/issues/56664"), WorkItem(55190, "https://github.com/dotnet/roslyn/issues/55190")] + [ConditionalFact(typeof(CoreClrOnly)), WorkItem(55190, "https://github.com/dotnet/roslyn/issues/55190")] public void GenericAttributeParameter_01() { var source = @" @@ -10325,9 +10325,7 @@ static void Main() } } "; - // The expected output here will change once we consume a runtime - // which has a fix for https://github.com/dotnet/runtime/issues/56492 - var verifier = CompileAndVerify(source, sourceSymbolValidator: verify, symbolValidator: verifyMetadata, expectedOutput: "CustomAttributeFormatException"); + var verifier = CompileAndVerify(source, sourceSymbolValidator: verify, symbolValidator: verifyMetadata, expectedOutput: "a"); verifier.VerifyTypeIL("Holder", @" .class private auto ansi beforefieldinit Holder From 0a26602eea3d44b1434032b1b85ba514b5b99539 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 13:25:06 -0700 Subject: [PATCH 17/21] Add back --- src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt | 1 - .../Portable/Workspace/Host/HostWorkspaceServices.cs | 10 ++++++++++ .../PersistentStorage/IPersistentStorageService.cs | 2 -- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt b/src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt index d38ba56f7da58..9bc1ad109c553 100644 --- a/src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt +++ b/src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt @@ -4,7 +4,6 @@ const Microsoft.CodeAnalysis.Classification.ClassificationTypeNames.RecordStruct Microsoft.CodeAnalysis.CodeFixes.DocumentBasedFixAllProvider Microsoft.CodeAnalysis.CodeFixes.DocumentBasedFixAllProvider.DocumentBasedFixAllProvider() -> void Microsoft.CodeAnalysis.CommandLineProject -Microsoft.CodeAnalysis.Host.IPersistentStorageService.GetStorageAsync(Microsoft.CodeAnalysis.Solution solution, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask Microsoft.CodeAnalysis.Project.GetSourceGeneratedDocumentAsync(Microsoft.CodeAnalysis.DocumentId documentId, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask Microsoft.CodeAnalysis.Project.GetSourceGeneratedDocumentsAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask> Microsoft.CodeAnalysis.Editing.DeclarationKind.RecordClass = 29 -> Microsoft.CodeAnalysis.Editing.DeclarationKind diff --git a/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs b/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs index a626a40505184..7b8807b9d42af 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs @@ -47,6 +47,16 @@ public TWorkspaceService GetRequiredService() where TWorkspac return service; } + /// + /// Obsolete. Roslyn no longer supports a mechanism to perform arbitrary persistence of data. If such functionality + /// is needed, consumers are resonsible for providing it themselves with whatever semantics are needed. + /// + [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] + public virtual IPersistentStorageService PersistentStorage + { + get { return this.GetRequiredService(); } + } + /// /// A service for storing information in a temporary location that only lasts for the duration of the process. /// diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs index 00c50864b74dc..3bb131c4bcc08 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorageService.cs @@ -17,7 +17,5 @@ public interface IPersistentStorageService : IWorkspaceService { [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] IPersistentStorage GetStorage(Solution solution); - [Obsolete("Roslyn no longer exports a mechanism to perform persistence.", error: true)] - ValueTask GetStorageAsync(Solution solution, CancellationToken cancellationToken = default); } } From 856e580d22e0c9781481e2478503eef9a0177b18 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 13:53:18 -0700 Subject: [PATCH 18/21] Add A/B option so we can test how important recoverable syntax trees are. --- .../Options/FeatureFlagPersister.cs | 27 ++++++++--------- .../CSharpSyntaxTreeFactoryService.cs | 10 +++++-- .../AbstractSyntaxTreeFactoryService.cs | 13 +++++--- .../ISyntaxTreeFactoryService.cs | 1 + .../Workspace/Solution/DocumentState.cs | 1 + .../WorkspaceConfigurationOptions.cs | 30 +++++++++++++++++++ .../VisualBasicSyntaxTreeFactoryService.vb | 11 ++++--- 7 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs diff --git a/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs b/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs index 6e25e473ed4ea..fa2cfc485bdd3 100644 --- a/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs +++ b/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.CodeAnalysis.ErrorReporting; @@ -18,6 +19,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.Options internal sealed class FeatureFlagPersister : IOptionPersister { private readonly IVsFeatureFlags? _featureFlags; + private readonly ConcurrentDictionary _cachedValues = new(); public FeatureFlagPersister(IVsFeatureFlags? featureFlags) { @@ -25,35 +27,31 @@ public FeatureFlagPersister(IVsFeatureFlags? featureFlags) } public bool TryFetch(OptionKey optionKey, [NotNullWhen(true)] out object? value) + { + value = _cachedValues.GetOrAdd(optionKey, key => TryFetchWorker(key)); + return value != null; + } + + private object? TryFetchWorker(OptionKey optionKey) { if (_featureFlags == null) - { - value = null; - return false; - } + return null; var location = optionKey.Option.StorageLocations.OfType().FirstOrDefault(); if (location == null) - { - value = null; - return false; - } + return null; if (optionKey.Option.DefaultValue is not bool defaultValue) - { throw ExceptionUtilities.UnexpectedValue(optionKey.Option.DefaultValue); - } try { - value = _featureFlags.IsFeatureEnabled(location.Name, defaultValue); + return _featureFlags.IsFeatureEnabled(location.Name, defaultValue); } catch (Exception e) when (FatalError.ReportAndCatch(e)) { - value = defaultValue; + return defaultValue; } - - return true; } public bool TryPersist(OptionKey optionKey, object? value) @@ -77,6 +75,7 @@ public bool TryPersist(OptionKey optionKey, object? value) try { ((IVsFeatureFlags2)_featureFlags).EnableFeatureFlag(location.Name, flag); + _cachedValues[optionKey] = flag; } catch (Exception e) when (FatalError.ReportAndCatch(e)) { diff --git a/src/Workspaces/CSharp/Portable/Workspace/LanguageServices/CSharpSyntaxTreeFactoryService.cs b/src/Workspaces/CSharp/Portable/Workspace/LanguageServices/CSharpSyntaxTreeFactoryService.cs index 129277fb68d13..4bf99b01cb2b7 100644 --- a/src/Workspaces/CSharp/Portable/Workspace/LanguageServices/CSharpSyntaxTreeFactoryService.cs +++ b/src/Workspaces/CSharp/Portable/Workspace/LanguageServices/CSharpSyntaxTreeFactoryService.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -21,19 +22,22 @@ namespace Microsoft.CodeAnalysis.CSharp internal partial class CSharpSyntaxTreeFactoryServiceFactory : ILanguageServiceFactory { private static readonly CSharpParseOptions _parseOptionWithLatestLanguageVersion = CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.Preview); + private readonly IGlobalOptionService _optionService; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public CSharpSyntaxTreeFactoryServiceFactory() + public CSharpSyntaxTreeFactoryServiceFactory(IGlobalOptionService optionService) { + _optionService = optionService; } public ILanguageService CreateLanguageService(HostLanguageServices provider) - => new CSharpSyntaxTreeFactoryService(provider); + => new CSharpSyntaxTreeFactoryService(_optionService, provider); private partial class CSharpSyntaxTreeFactoryService : AbstractSyntaxTreeFactoryService { - public CSharpSyntaxTreeFactoryService(HostLanguageServices languageServices) : base(languageServices) + public CSharpSyntaxTreeFactoryService(IGlobalOptionService optionService, HostLanguageServices languageServices) + : base(optionService, languageServices) { } diff --git a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/AbstractSyntaxTreeFactoryService.cs b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/AbstractSyntaxTreeFactoryService.cs index d765733b5167b..5e66df12ce283 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/AbstractSyntaxTreeFactoryService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/AbstractSyntaxTreeFactoryService.cs @@ -7,6 +7,7 @@ using System.IO; using System.Text; using System.Threading; +using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -16,15 +17,19 @@ internal abstract partial class AbstractSyntaxTreeFactoryService : ISyntaxTreeFa { // Recoverable trees only save significant memory for larger trees internal readonly int MinimumLengthForRecoverableTree; - private readonly bool _hasCachingService; + private readonly bool _canCreateRecoverableTrees; internal HostLanguageServices LanguageServices { get; } - public AbstractSyntaxTreeFactoryService(HostLanguageServices languageServices) + public AbstractSyntaxTreeFactoryService( + IGlobalOptionService optionService, + HostLanguageServices languageServices) { this.LanguageServices = languageServices; this.MinimumLengthForRecoverableTree = languageServices.WorkspaceServices.Workspace.Options.GetOption(CacheOptions.RecoverableTreeLengthThreshold); - _hasCachingService = languageServices.WorkspaceServices.GetService() != null; + _canCreateRecoverableTrees = + languageServices.WorkspaceServices.GetService() != null && + !optionService.GetOption(WorkspaceConfigurationOptions.DisableRecoverableTrees); } public abstract ParseOptions GetDefaultParseOptions(); @@ -35,7 +40,7 @@ public AbstractSyntaxTreeFactoryService(HostLanguageServices languageServices) public abstract ParseOptions GetDefaultParseOptionsWithLatestLanguageVersion(); public virtual bool CanCreateRecoverableTree(SyntaxNode root) - => _hasCachingService && root.FullSpan.Length >= this.MinimumLengthForRecoverableTree; + => _canCreateRecoverableTrees && root.FullSpan.Length >= this.MinimumLengthForRecoverableTree; protected static SyntaxNode RecoverNode(SyntaxTree tree, TextSpan textSpan, int kind) { diff --git a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs index ea63ac37a5fda..ce01773735d44 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs @@ -5,6 +5,7 @@ using System.IO; using System.Text; using System.Threading; +using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs index 6b87063f9a3ff..eef80d095a797 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs new file mode 100644 index 0000000000000..1339dbc670825 --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Options.Providers; + +namespace Microsoft.CodeAnalysis +{ + [ExportOptionProvider, Shared] + internal class WorkspaceConfigurationOptions : IOptionProvider + { + public static readonly Option DisableRecoverableTrees = new( + nameof(WorkspaceConfigurationOptions), nameof(DisableRecoverableTrees), defaultValue: false, + new FeatureFlagStorageLocation("Roslyn.DisableRecoverableTrees")); + + ImmutableArray IOptionProvider.Options { get; } = ImmutableArray.Create( + DisableRecoverableTrees); + + [ImportingConstructor] + [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + public WorkspaceConfigurationOptions() + { + } + } +} diff --git a/src/Workspaces/VisualBasic/Portable/Workspace/LanguageServices/VisualBasicSyntaxTreeFactoryService.vb b/src/Workspaces/VisualBasic/Portable/Workspace/LanguageServices/VisualBasicSyntaxTreeFactoryService.vb index 1c9c857bb3e74..8305eee67b2e7 100644 --- a/src/Workspaces/VisualBasic/Portable/Workspace/LanguageServices/VisualBasicSyntaxTreeFactoryService.vb +++ b/src/Workspaces/VisualBasic/Portable/Workspace/LanguageServices/VisualBasicSyntaxTreeFactoryService.vb @@ -8,6 +8,7 @@ Imports System.Text Imports System.Threading Imports Microsoft.CodeAnalysis.Host Imports Microsoft.CodeAnalysis.Host.Mef +Imports Microsoft.CodeAnalysis.Options Imports Microsoft.CodeAnalysis.Text Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -17,21 +18,23 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Implements ILanguageServiceFactory Private Shared ReadOnly _parseOptionsWithLatestLanguageVersion As VisualBasicParseOptions = VisualBasicParseOptions.Default.WithLanguageVersion(LanguageVersion.Latest) + Private ReadOnly _optionService As IGlobalOptionService - Public Sub New() + Public Sub New(optionService As IGlobalOptionService) + _optionService = optionService End Sub Public Function CreateLanguageService(provider As HostLanguageServices) As ILanguageService Implements ILanguageServiceFactory.CreateLanguageService - Return New VisualBasicSyntaxTreeFactoryService(provider) + Return New VisualBasicSyntaxTreeFactoryService(_optionService, provider) End Function Partial Friend Class VisualBasicSyntaxTreeFactoryService Inherits AbstractSyntaxTreeFactoryService - Public Sub New(languageServices As HostLanguageServices) - MyBase.New(languageServices) + Public Sub New(optionService As IGlobalOptionService, languageServices As HostLanguageServices) + MyBase.New(optionService, languageServices) End Sub Public Overloads Overrides Function GetDefaultParseOptions() As ParseOptions From cde4870d368d20c1704819730eb1e1d3629208cd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 14:13:10 -0700 Subject: [PATCH 19/21] REvert --- .../Options/FeatureFlagPersister.cs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs b/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs index fa2cfc485bdd3..6e25e473ed4ea 100644 --- a/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs +++ b/src/VisualStudio/Core/Def/Implementation/Options/FeatureFlagPersister.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.CodeAnalysis.ErrorReporting; @@ -19,7 +18,6 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.Options internal sealed class FeatureFlagPersister : IOptionPersister { private readonly IVsFeatureFlags? _featureFlags; - private readonly ConcurrentDictionary _cachedValues = new(); public FeatureFlagPersister(IVsFeatureFlags? featureFlags) { @@ -27,31 +25,35 @@ public FeatureFlagPersister(IVsFeatureFlags? featureFlags) } public bool TryFetch(OptionKey optionKey, [NotNullWhen(true)] out object? value) - { - value = _cachedValues.GetOrAdd(optionKey, key => TryFetchWorker(key)); - return value != null; - } - - private object? TryFetchWorker(OptionKey optionKey) { if (_featureFlags == null) - return null; + { + value = null; + return false; + } var location = optionKey.Option.StorageLocations.OfType().FirstOrDefault(); if (location == null) - return null; + { + value = null; + return false; + } if (optionKey.Option.DefaultValue is not bool defaultValue) + { throw ExceptionUtilities.UnexpectedValue(optionKey.Option.DefaultValue); + } try { - return _featureFlags.IsFeatureEnabled(location.Name, defaultValue); + value = _featureFlags.IsFeatureEnabled(location.Name, defaultValue); } catch (Exception e) when (FatalError.ReportAndCatch(e)) { - return defaultValue; + value = defaultValue; } + + return true; } public bool TryPersist(OptionKey optionKey, object? value) @@ -75,7 +77,6 @@ public bool TryPersist(OptionKey optionKey, object? value) try { ((IVsFeatureFlags2)_featureFlags).EnableFeatureFlag(location.Name, flag); - _cachedValues[optionKey] = flag; } catch (Exception e) when (FatalError.ReportAndCatch(e)) { From d33494b9e110f45352b1e8e4a900e817ee8567e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 14:13:43 -0700 Subject: [PATCH 20/21] REvert --- .../Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs | 1 - src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs index ce01773735d44..ea63ac37a5fda 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/ISyntaxTreeFactoryService.cs @@ -5,7 +5,6 @@ using System.IO; using System.Text; using System.Threading; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs index eef80d095a797..6b87063f9a3ff 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs @@ -13,7 +13,6 @@ using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Internal.Log; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; From e96cac618a22d8e23dd3a10382ecbb2960c95b57 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 1 Oct 2021 14:15:12 -0700 Subject: [PATCH 21/21] Remove --- .../Core/Portable/Storage/LegacyPersistentStorageService.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs index 55b841b24e2b6..fcafe75beec69 100644 --- a/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/LegacyPersistentStorageService.cs @@ -37,8 +37,5 @@ public LegacyPersistentStorageService() public IPersistentStorage GetStorage(Solution solution) => NoOpPersistentStorage.GetOrThrow(throwOnFailure: false); - - public ValueTask GetStorageAsync(Solution solution, CancellationToken cancellationToken) - => new(NoOpPersistentStorage.GetOrThrow(throwOnFailure: false)); } }