From c052c70ebf0a40058670772956bc949212124bde Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 31 Jul 2018 09:28:46 -0500 Subject: [PATCH 01/85] Initial documentation for the TestAccessor Pattern --- docs/TestAccessor Pattern.md | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/TestAccessor Pattern.md diff --git a/docs/TestAccessor Pattern.md b/docs/TestAccessor Pattern.md new file mode 100644 index 0000000000000..c762a5323e1ef --- /dev/null +++ b/docs/TestAccessor Pattern.md @@ -0,0 +1,45 @@ +# The `TestAccessor` Pattern + +The `TestAccessor` pattern allows production code to expose internal functionality for test purposes without making the internal functionality available to other production code. The pattern has two primary components: + +1. A `TestAccessor` type, which contains the functionality available only for testing +2. A `GetTestAccessor()` method, which returns an instance of `TestAccessor` + +The pattern relies on enforcement of a simple rule that no production code is allowed to call a `GetTestAccessor()` method. This is enforceable either through code reviews or through an analyzer. This pattern has many advantages over alternatives: + +* The pattern does not require expanded accessibility (e.g. `internal` instead of `private`) for the purpose of testing +* The pattern is self-documenting: all properties and methods within a `TestAccessor` type are intended only for use in test code +* The pattern is consistent enough to enforce through static analysis (analyzers) +* The pattern is simple enough to enforce manually (code reviews) + +## The `TestAccessor` Type + +The `TestAccessor` type is typically defined as a nested structure. In the following example, the `SomeProductionType.TestAccessor.PrivateStateData` property allows test code to read and write the value of the private field `SomeProductionType._privateStateData` without exposing the `_privateStateData` field to other production code. + +```csharp +internal class SomeProductionType +{ + private int _privateStateData; + + internal readonly struct TestAccessor + { + private readonly SomeProductionType _instance; + + internal TestAccessor(SomeProductionType instance) + { + _instance = instance; + } + + internal ref int PrivateStateData => ref _instance._privateStateData; + } +} +``` + +## The `GetTestAccessor()` Method + +The `GetTestAccessor()` method is always defined as follows: + +```csharp +internal TestAccessor GetTestAccessor() + => new TestAccessor(this); +``` From ee7d7a425a44f601b7452a748d253a190777919e Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Sun, 17 Apr 2022 08:42:18 -0700 Subject: [PATCH 02/85] Add new background analysis scope for compiler diagnostics --- .../DiagnosticAnalyzerServiceTests.cs | 3 ++ .../DocumentAnalysisExecutor_Helpers.cs | 7 ++- .../DiagnosticIncrementalAnalyzer.Executor.cs | 52 +++++++++++++++---- ...gnosticIncrementalAnalyzer.ProjectState.cs | 4 +- .../EngineV2/DiagnosticIncrementalAnalyzer.cs | 3 +- ...ncrementalAnalyzer_BuildSynchronization.cs | 2 +- ...IncrementalAnalyzer_IncrementalAnalyzer.cs | 24 +++++++-- .../Options/SolutionCrawlerOptionsStorage.cs | 40 ++++++++++++++ .../WorkspacePullDiagnosticHandler.cs | 2 +- .../Options/AdvancedOptionPageControl.xaml | 9 ++++ .../Options/AdvancedOptionPageControl.xaml.cs | 1 + .../Impl/Options/AdvancedOptionPageStrings.cs | 27 ++++++++++ .../VisualStudioDiagnosticAnalyzerService.cs | 7 ++- .../Core/Def/ServicesVSResources.resx | 6 +++ .../ExternalErrorDiagnosticUpdateSource.cs | 2 +- .../Core/Def/xlf/ServicesVSResources.cs.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.de.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.es.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.fr.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.it.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.ja.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.ko.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.pl.xlf | 10 ++++ .../Def/xlf/ServicesVSResources.pt-BR.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.ru.xlf | 10 ++++ .../Core/Def/xlf/ServicesVSResources.tr.xlf | 10 ++++ .../Def/xlf/ServicesVSResources.zh-Hans.xlf | 10 ++++ .../Def/xlf/ServicesVSResources.zh-Hant.xlf | 10 ++++ .../VisualStudioWorkspace_OutOfProc.cs | 4 ++ .../TestUtilities/WellKnownGlobalOptions.cs | 2 + .../Options/AdvancedOptionPageControl.xaml | 9 ++++ .../Options/AdvancedOptionPageControl.xaml.vb | 1 + .../Impl/Options/AdvancedOptionPageStrings.vb | 27 ++++++++++ .../BackgroundAnalysisScopeExtensions.cs | 22 ++++++++ .../CompilerDiagnosticsScope.cs | 32 ++++++++++++ 35 files changed, 393 insertions(+), 23 deletions(-) create mode 100644 src/Workspaces/Core/Portable/Shared/Extensions/BackgroundAnalysisScopeExtensions.cs create mode 100644 src/Workspaces/Core/Portable/SolutionCrawler/CompilerDiagnosticsScope.cs diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs index fd1680d31acea..d045507a74d09 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs @@ -874,6 +874,9 @@ void M() workspace.GlobalOptions.SetGlobalOption(new OptionKey(SolutionCrawlerOptionsStorage.BackgroundAnalysisScopeOption, LanguageNames.CSharp), analysisScope); + var compilerDiagnosticsScope = analysisScope.ToEquivalentCompilerDiagnosticsScope(); + workspace.GlobalOptions.SetGlobalOption(new OptionKey(SolutionCrawlerOptionsStorage.CompilerDiagnosticsScopeOption, LanguageNames.CSharp), compilerDiagnosticsScope); + workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { analyzerReference })); var project = workspace.CurrentSolution.Projects.Single(); diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs index 7711702e0f4f1..c8f79815d9421 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs @@ -194,11 +194,16 @@ private static void AssertCompilation(Project project, Compilation compilation1) public static bool IsAnalyzerEnabledForProject(DiagnosticAnalyzer analyzer, Project project, IGlobalOptionService globalOptions) { var options = project.CompilationOptions; - if (options == null || analyzer == FileContentLoadAnalyzer.Instance || analyzer.IsCompilerAnalyzer()) + if (options == null || analyzer == FileContentLoadAnalyzer.Instance) { return true; } + if (analyzer.IsCompilerAnalyzer()) + { + return globalOptions.GetOption(SolutionCrawlerOptionsStorage.CompilerDiagnosticsScopeOption, project.Language) != CompilerDiagnosticsScope.None; + } + // Check if user has disabled analyzer execution for this project or via options. if (!project.State.RunAnalyzers || globalOptions.GetBackgroundAnalysisScope(project.Language) == BackgroundAnalysisScope.None) { diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs index 90d027d4efa16..4b7f6ba97e9cb 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs @@ -29,7 +29,9 @@ internal partial class DiagnosticIncrementalAnalyzer private DocumentAnalysisData? TryGetCachedDocumentAnalysisData( TextDocument document, StateSet stateSet, AnalysisKind kind, VersionStamp version, - BackgroundAnalysisScope analysisScope, bool isActiveDocument, + BackgroundAnalysisScope analysisScope, + CompilerDiagnosticsScope compilerDiagnosticsScope, + bool isActiveDocument, bool isVisibleDocument, bool isOpenDocument, bool isGeneratedRazorDocument, CancellationToken cancellationToken) { @@ -47,7 +49,8 @@ internal partial class DiagnosticIncrementalAnalyzer // Perf optimization: Check whether analyzer is suppressed for project or document and avoid getting diagnostics if suppressed. if (!DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(stateSet.Analyzer, document.Project, GlobalOptions) || - !IsAnalyzerEnabledForDocument(stateSet.Analyzer, analysisScope, isActiveDocument, isOpenDocument, isGeneratedRazorDocument)) + !IsAnalyzerEnabledForDocument(stateSet.Analyzer, existingData, analysisScope, compilerDiagnosticsScope, + isActiveDocument, isVisibleDocument, isOpenDocument, isGeneratedRazorDocument)) { return new DocumentAnalysisData(version, existingData.Items, ImmutableArray.Empty); } @@ -61,8 +64,11 @@ internal partial class DiagnosticIncrementalAnalyzer static bool IsAnalyzerEnabledForDocument( DiagnosticAnalyzer analyzer, + DocumentAnalysisData previousData, BackgroundAnalysisScope analysisScope, + CompilerDiagnosticsScope compilerDiagnosticsScope, bool isActiveDocument, + bool isVisibleDocument, bool isOpenDocument, bool isGeneratedRazorDocument) { @@ -76,10 +82,22 @@ static bool IsAnalyzerEnabledForDocument( if (analyzer.IsCompilerAnalyzer()) { - // Compiler analyzer is treated specially. - // It is executed for all documents (open and closed) for 'BackgroundAnalysisScope.FullSolution' - // and executed for just open documents for other analysis scopes. - return analysisScope == BackgroundAnalysisScope.FullSolution || isOpenDocument; + return compilerDiagnosticsScope switch + { + // Compiler diagnostics are disabled for all documents. + CompilerDiagnosticsScope.None => false, + + // Compiler diagnostics are enabled for visible documents and open documents which had errors/warnings in prior snapshot. + CompilerDiagnosticsScope.VisibleFiles => isVisibleDocument || isOpenDocument && !previousData.Items.IsEmpty, + + // Compiler diagnostics are enabled for all open documents. + CompilerDiagnosticsScope.OpenFiles => isOpenDocument, + + // Compiler diagnostics are enabled for all documents. + CompilerDiagnosticsScope.FullSolution => true, + + _ => throw ExceptionUtilities.UnexpectedValue(analysisScope) + }; } else { @@ -163,13 +181,23 @@ private async Task GetProjectAnalysisDataAsync( } // PERF: Check whether we want to analyze this project or not. - if (!FullAnalysisEnabled(project, forceAnalyzerRun)) + if (!FullAnalysisEnabled(project, forceAnalyzerRun, out var compilerFullAnalysisEnabled, out var analyzersFullAnalysisEnabled)) { Logger.Log(FunctionId.Diagnostics_ProjectDiagnostic, p => $"FSA off ({p.FilePath ?? p.Name})", project); return new ProjectAnalysisData(project.Id, VersionStamp.Default, existingData.Result, ImmutableDictionary.Empty); } + if (!compilerFullAnalysisEnabled) + { + Debug.Assert(analyzersFullAnalysisEnabled); + stateSets = stateSets.Where(s => !s.Analyzer.IsCompilerAnalyzer()); + } + else if (!analyzersFullAnalysisEnabled) + { + stateSets = stateSets.Where(s => s.Analyzer.IsCompilerAnalyzer()); + } + var result = await ComputeDiagnosticsAsync(compilationWithAnalyzers, project, ideOptions, stateSets, forceAnalyzerRun, existingData.Result, cancellationToken).ConfigureAwait(false); // If project is not loaded successfully, get rid of any semantic errors from compiler analyzer. @@ -467,15 +495,21 @@ private void UpdateAnalyzerTelemetryData(ImmutableDictionary p.Language)) { - if (GlobalOptions.GetBackgroundAnalysisScope(projectsByLanguage.Key) == BackgroundAnalysisScope.FullSolution) + if (GlobalOptions.IsFullSolutionAnalysisEnabled(projectsByLanguage.Key, out _, out _)) { AnalyzerService.Reanalyze(Workspace, projectsByLanguage.Select(p => p.Id)); } diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index 73b8f5c895116..94ac901134cb6 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -53,6 +53,11 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync(document.Project, stateSets, cancellationToken).ConfigureAwait(false); var version = await GetDiagnosticVersionAsync(document.Project, cancellationToken).ConfigureAwait(false); var backgroundAnalysisScope = GlobalOptions.GetBackgroundAnalysisScope(document.Project.Language); + var compilerDiagnosticsScope = GlobalOptions.GetOption(SolutionCrawlerOptionsStorage.CompilerDiagnosticsScopeOption, document.Project.Language); + + // TODO: Switch to a more reliable service to determine visible documents. + // DocumentTrackingService is known be unreliable at times. + var isVisibleDocument = _documentTrackingService.GetVisibleDocuments().Contains(document.Id); // We split the diagnostic computation for document into following steps: // 1. Try to get cached diagnostics for each analyzer, while computing the set of analyzers that do not have cached diagnostics. @@ -66,7 +71,8 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi foreach (var stateSet in stateSets) { var data = TryGetCachedDocumentAnalysisData(document, stateSet, kind, version, - backgroundAnalysisScope, isActiveDocument, isOpenDocument, isGeneratedRazorDocument, cancellationToken); + backgroundAnalysisScope, compilerDiagnosticsScope, isActiveDocument, isVisibleDocument, + isOpenDocument, isGeneratedRazorDocument, cancellationToken); if (data.HasValue) { // We need to persist and raise diagnostics for suppressed analyzer. @@ -121,7 +127,7 @@ void OnAnalysisException() public async Task AnalyzeProjectAsync(Project project, bool semanticsChanged, InvocationReasons reasons, CancellationToken cancellationToken) { // Perf optimization. check whether we want to analyze this project or not. - if (!FullAnalysisEnabled(project, forceAnalyzerRun: false)) + if (!FullAnalysisEnabled(project, forceAnalyzerRun: false, out _, out _)) { return; } @@ -249,7 +255,9 @@ private void RaiseDiagnosticsRemovedIfRequiredForClosedOrResetDocument(TextDocum { // if there was no diagnostic reported for this document OR Full solution analysis is enabled, nothing to clean up if (!documentHadDiagnostics || - FullAnalysisEnabled(document.Project, forceAnalyzerRun: false)) + FullAnalysisEnabled(document.Project, forceAnalyzerRun: false, out var compilerFullAnalysisEnabled, out var analyzersFullAnalysisEnabled) && + compilerFullAnalysisEnabled && + analyzersFullAnalysisEnabled) { // this is Perf to reduce raising events unnecessarily. return; @@ -368,9 +376,15 @@ private IReadOnlyList GetStateSetsForFullSolutionAnalysis(IEnumerable< // If full analysis is off, remove state that is created from build. // this will make sure diagnostics from build (converted from build to live) will never be cleared // until next build. - if (GlobalOptions.GetBackgroundAnalysisScope(project.Language) != BackgroundAnalysisScope.FullSolution) + _ = GlobalOptions.IsFullSolutionAnalysisEnabled(project.Language, out var compilerFullSolutionAnalysisEnabled, out var analyzersFullSolutionAnalysisEnabled); + if (!compilerFullSolutionAnalysisEnabled) + { + stateSets = stateSets.Where(s => !s.Analyzer.IsCompilerAnalyzer() || !s.FromBuild(project.Id)); + } + + if (!analyzersFullSolutionAnalysisEnabled) { - stateSets = stateSets.Where(s => !s.FromBuild(project.Id)); + stateSets = stateSets.Where(s => s.Analyzer.IsCompilerAnalyzer() || !s.FromBuild(project.Id)); } // Compute analyzer config options for computing effective severity. diff --git a/src/Features/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsStorage.cs b/src/Features/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsStorage.cs index d10ce27861020..e8bca79292b89 100644 --- a/src/Features/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsStorage.cs +++ b/src/Features/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsStorage.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Options; namespace Microsoft.CodeAnalysis.SolutionCrawler; @@ -22,6 +23,13 @@ internal static class SolutionCrawlerOptionsStorage public static readonly Option2 SolutionBackgroundAnalysisScopeOption = new( "SolutionCrawlerOptionsStorage", "SolutionBackgroundAnalysisScopeOption", defaultValue: null); + /// + /// Option to configure compiler diagnostics scope for the current user. + /// + public static readonly PerLanguageOption2 CompilerDiagnosticsScopeOption = new( + "SolutionCrawlerOptionsStorage", "CompilerDiagnosticsScopeOption", defaultValue: CompilerDiagnosticsScope.OpenFiles, + storageLocation: new RoamingProfileStorageLocation($"TextEditor.%LANGUAGE%.Specific.CompilerDiagnosticsScopeOption")); + public static readonly PerLanguageOption2 RemoveDocumentDiagnosticsOnDocumentClose = new( "ServiceFeatureOnOffOptions", "RemoveDocumentDiagnosticsOnDocumentClose", defaultValue: false, storageLocation: new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.RemoveDocumentDiagnosticsOnDocumentClose")); @@ -48,4 +56,36 @@ public static BackgroundAnalysisScope GetBackgroundAnalysisScope(this IGlobalOpt return globalOptions.GetOption(SolutionBackgroundAnalysisScopeOption) ?? globalOptions.GetOption(BackgroundAnalysisScopeOption, language); } + + public static bool IsFullSolutionAnalysisEnabled(this DiagnosticAnalyzer analyzer, IGlobalOptionService globalOptions, string language) + { + if (analyzer.IsCompilerAnalyzer()) + { + return globalOptions.GetOption(CompilerDiagnosticsScopeOption, language) == CompilerDiagnosticsScope.FullSolution; + } + + return GetBackgroundAnalysisScope(globalOptions, language) == BackgroundAnalysisScope.FullSolution; + } + + public static bool IsFullSolutionAnalysisEnabled( + this IGlobalOptionService globalOptions, + string language, + out bool compilerFullSolutionAnalysisEnabled, + out bool analyzersFullSolutionAnalysisEnabled) + { + compilerFullSolutionAnalysisEnabled = globalOptions.GetOption(CompilerDiagnosticsScopeOption, language) == CompilerDiagnosticsScope.FullSolution; + analyzersFullSolutionAnalysisEnabled = GetBackgroundAnalysisScope(globalOptions, language) == BackgroundAnalysisScope.FullSolution; + return compilerFullSolutionAnalysisEnabled || analyzersFullSolutionAnalysisEnabled; + } + + public static bool IsAnalysisDisabled( + this IGlobalOptionService globalOptions, + string language, + out bool compilerDiagnosticsDisabled, + out bool analyzersDisabled) + { + compilerDiagnosticsDisabled = globalOptions.GetOption(CompilerDiagnosticsScopeOption, language) == CompilerDiagnosticsScope.None; + analyzersDisabled = GetBackgroundAnalysisScope(globalOptions, language) == BackgroundAnalysisScope.None; + return compilerDiagnosticsDisabled && analyzersDisabled; + } } diff --git a/src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs b/src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs index fbb96641c73c7..0a677552dd618 100644 --- a/src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs @@ -121,7 +121,7 @@ async Task AddDocumentsFromProjectAsync(Project? project, ImmutableArray // solution analysis on. if (!isOpen) { - if (globalOptions.GetBackgroundAnalysisScope(project.Language) != BackgroundAnalysisScope.FullSolution) + if (!globalOptions.IsFullSolutionAnalysisEnabled(project.Language, out var _, out var _)) { context.TraceInformation($"Skipping project '{project.Name}' as it has no open document and Full Solution Analysis is off"); return; diff --git a/src/VisualStudio/CSharp/Impl/Options/AdvancedOptionPageControl.xaml b/src/VisualStudio/CSharp/Impl/Options/AdvancedOptionPageControl.xaml index a8f8a799d42b0..4b969b5e123c9 100644 --- a/src/VisualStudio/CSharp/Impl/Options/AdvancedOptionPageControl.xaml +++ b/src/VisualStudio/CSharp/Impl/Options/AdvancedOptionPageControl.xaml @@ -24,6 +24,15 @@ +