From f078ffd0d725363e47343f238ed48a64e2bc5bbf Mon Sep 17 00:00:00 2001 From: tmat Date: Wed, 9 Feb 2022 16:02:14 -0800 Subject: [PATCH] Feedback --- .../AbstractAddImportCodeFixProvider.cs | 51 +++++++++---------- .../AddImport/SymbolReferenceFinder.cs | 6 --- .../Diagnostics/WorkspaceAnalyzerOptions.cs | 2 +- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportCodeFixProvider.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportCodeFixProvider.cs index 4e387dab8bdb7..25fcf19a4d6f3 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportCodeFixProvider.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportCodeFixProvider.cs @@ -2,14 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Immutable; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CodeGeneration; -using Microsoft.CodeAnalysis.Completion; using Microsoft.CodeAnalysis.Packaging; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.SymbolSearch; @@ -20,15 +16,15 @@ internal abstract partial class AbstractAddImportCodeFixProvider : CodeFixProvid { private const int MaxResults = 5; - private readonly IPackageInstallerService _packageInstallerService; - private readonly ISymbolSearchService _symbolSearchService; + private readonly IPackageInstallerService? _packageInstallerService; + private readonly ISymbolSearchService? _symbolSearchService; /// /// Values for these parameters can be provided (during testing) for mocking purposes. /// protected AbstractAddImportCodeFixProvider( - IPackageInstallerService packageInstallerService = null, - ISymbolSearchService symbolSearchService = null) + IPackageInstallerService? packageInstallerService = null, + ISymbolSearchService? symbolSearchService = null) { _packageInstallerService = packageInstallerService; _symbolSearchService = symbolSearchService; @@ -42,7 +38,7 @@ protected AbstractAddImportCodeFixProvider( private protected override CodeActionRequestPriority ComputeRequestPriority() => CodeActionRequestPriority.High; - public sealed override FixAllProvider GetFixAllProvider() + public sealed override FixAllProvider? GetFixAllProvider() { // Currently Fix All is not supported for this provider // https://github.com/dotnet/roslyn/issues/34457 @@ -56,28 +52,34 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var cancellationToken = context.CancellationToken; var diagnostics = context.Diagnostics; - var addImportService = document.GetLanguageService(); + var addImportService = document.GetRequiredLanguageService(); + var services = document.Project.Solution.Workspace.Services; - var solution = document.Project.Solution; + var searchOptions = context.Options.SearchOptions; - var placement = await AddImportPlacementOptions.FromDocumentAsync(document, cancellationToken).ConfigureAwait(false); + var symbolSearchService = _symbolSearchService ?? services.GetRequiredService(); - var options = new AddImportOptions( - context.Options.SearchOptions, - context.Options.HideAdvancedMembers, - placement); + var installerService = searchOptions.SearchNuGetPackages ? + _packageInstallerService ?? services.GetService() : null; - var symbolSearchService = options.SearchOptions.SearchReferenceAssemblies - ? _symbolSearchService ?? solution.Workspace.Services.GetService() - : null; - - var installerService = GetPackageInstallerService(document); - var packageSources = options.SearchOptions.SearchNuGetPackages && symbolSearchService != null && installerService?.IsEnabled(document.Project.Id) == true + var packageSources = installerService?.IsEnabled(document.Project.Id) == true ? installerService.TryGetPackageSources() : ImmutableArray.Empty; + if (packageSources.IsEmpty) + { + searchOptions = searchOptions with { SearchNuGetPackages = false }; + } + + var placement = await AddImportPlacementOptions.FromDocumentAsync(document, cancellationToken).ConfigureAwait(false); + + var addImportOptions = new AddImportOptions( + searchOptions, + context.Options.HideAdvancedMembers, + placement); + var fixesForDiagnostic = await addImportService.GetFixesForDiagnosticsAsync( - document, span, diagnostics, MaxResults, symbolSearchService, options, packageSources, cancellationToken).ConfigureAwait(false); + document, span, diagnostics, MaxResults, symbolSearchService, addImportOptions, packageSources, cancellationToken).ConfigureAwait(false); foreach (var (diagnostic, fixes) in fixesForDiagnostic) { @@ -86,8 +88,5 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterFixes(codeActions, diagnostic); } } - - private IPackageInstallerService GetPackageInstallerService(Document document) - => _packageInstallerService ?? document.Project.Solution.Workspace.Services.GetService(); } } diff --git a/src/Features/Core/Portable/AddImport/SymbolReferenceFinder.cs b/src/Features/Core/Portable/AddImport/SymbolReferenceFinder.cs index 38e3c7d9bcbae..6514dc0f4cb92 100644 --- a/src/Features/Core/Portable/AddImport/SymbolReferenceFinder.cs +++ b/src/Features/Core/Portable/AddImport/SymbolReferenceFinder.cs @@ -62,12 +62,6 @@ public SymbolReferenceFinder( _symbolSearchService = symbolSearchService; _options = options; _packageSources = packageSources; - - if (options.SearchOptions.SearchReferenceAssemblies || packageSources.Length > 0) - { - Contract.ThrowIfNull(symbolSearchService); - } - _syntaxFacts = document.GetLanguageService(); _namespacesInScope = GetNamespacesInScope(cancellationToken); diff --git a/src/Features/Core/Portable/Diagnostics/WorkspaceAnalyzerOptions.cs b/src/Features/Core/Portable/Diagnostics/WorkspaceAnalyzerOptions.cs index e897792497da1..180f99a8a86d7 100644 --- a/src/Features/Core/Portable/Diagnostics/WorkspaceAnalyzerOptions.cs +++ b/src/Features/Core/Portable/Diagnostics/WorkspaceAnalyzerOptions.cs @@ -37,7 +37,7 @@ public IdeAnalyzerOptions GetIdeOptions(string language) language, static (language, solution) => new IdeAnalyzerOptions( FadeOutUnusedImports: solution.Options.GetOption(Fading.FadingOptions.Metadata.FadeOutUnusedImports, language), - FadeOutUnreachableCode: solution.Options.GetOption(Fading.FadingOptions.Metadata.FadeOutUnusedImports, language), + FadeOutUnreachableCode: solution.Options.GetOption(Fading.FadingOptions.Metadata.FadeOutUnreachableCode, language), ReportInvalidPlaceholdersInStringDotFormatCalls: solution.Options.GetOption(ValidateFormatString.ValidateFormatStringOption.ReportInvalidPlaceholdersInStringDotFormatCalls, language), ReportInvalidRegexPatterns: solution.Options.GetOption(Features.EmbeddedLanguages.RegularExpressions.LanguageServices.RegularExpressionsOptions.ReportInvalidRegexPatterns, language)), _solution);