From ceccae01575d51a4078d782331e02fe019855d71 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Nov 2022 10:12:15 -0800 Subject: [PATCH 1/3] Fix PublicAPI enablement checks The current logic was broken for projects that only have public or only have internal APIs. I refactored the registration logic to run completely separately for both public and internal to fix this. --- .../Analyzers/DeclarePublicApiAnalyzer.cs | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs index 10121813a1..f565b53cf7 100644 --- a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs +++ b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs @@ -102,47 +102,45 @@ public override void Initialize(AnalysisContext context) private void OnCompilationStart(CompilationStartAnalysisContext compilationContext) { - var errors = new List(); - // Switch to "RegisterAdditionalFileAction" available in Microsoft.CodeAnalysis "3.8.x" to report additional file diagnostics: https://github.com/dotnet/roslyn-analyzers/issues/3918 - if (!TryGetAndValidateApiFiles(compilationContext.Options, compilationContext.Compilation, isPublic: true, compilationContext.CancellationToken, errors, out ApiData publicShippedData, out ApiData publicUnshippedData) - || !TryGetAndValidateApiFiles(compilationContext.Options, compilationContext.Compilation, isPublic: false, compilationContext.CancellationToken, errors, out ApiData internalShippedData, out ApiData internalUnshippedData)) + CheckAndRegisterImplementation(isPublic: true); + CheckAndRegisterImplementation(isPublic: false); + + void CheckAndRegisterImplementation(bool isPublic) { - compilationContext.RegisterCompilationEndAction(context => + var errors = new List(); + // Switch to "RegisterAdditionalFileAction" available in Microsoft.CodeAnalysis "3.8.x" to report additional file diagnostics: https://github.com/dotnet/roslyn-analyzers/issues/3918 + if (!TryGetAndValidateApiFiles(compilationContext.Options, compilationContext.Compilation, isPublic, compilationContext.CancellationToken, errors, out ApiData shippedData, out ApiData unshippedData)) { - foreach (Diagnostic cur in errors) + compilationContext.RegisterCompilationEndAction(context => { - context.ReportDiagnostic(cur); - } - }); - - return; - } - - Debug.Assert(errors.Count == 0); + foreach (Diagnostic cur in errors) + { + context.ReportDiagnostic(cur); + } + }); + } - var publicImpl = new Impl(compilationContext.Compilation, publicShippedData, publicUnshippedData, isPublic: true, compilationContext.Options); - var internalImpl = new Impl(compilationContext.Compilation, internalShippedData, internalUnshippedData, isPublic: false, compilationContext.Options); - RegisterImplActions(compilationContext, publicImpl); - RegisterImplActions(compilationContext, internalImpl); + RegisterImplActions(compilationContext, new Impl(compilationContext.Compilation, shippedData, unshippedData, isPublic, compilationContext.Options)); - static bool TryGetAndValidateApiFiles(AnalyzerOptions options, Compilation compilation, bool isPublic, CancellationToken cancellationToken, List errors, out ApiData shippedData, out ApiData unshippedData) - { - return TryGetApiData(options, compilation, isPublic, errors, cancellationToken, out shippedData, out unshippedData) - && ValidateApiFiles(shippedData, unshippedData, isPublic, errors); - } + static bool TryGetAndValidateApiFiles(AnalyzerOptions options, Compilation compilation, bool isPublic, CancellationToken cancellationToken, List errors, out ApiData shippedData, out ApiData unshippedData) + { + return TryGetApiData(options, compilation, isPublic, errors, cancellationToken, out shippedData, out unshippedData) + && ValidateApiFiles(shippedData, unshippedData, isPublic, errors); + } - static void RegisterImplActions(CompilationStartAnalysisContext compilationContext, Impl impl) - { - compilationContext.RegisterSymbolAction( - impl.OnSymbolAction, - SymbolKind.NamedType, - SymbolKind.Event, - SymbolKind.Field, - SymbolKind.Method); - compilationContext.RegisterSymbolAction( - impl.OnPropertyAction, - SymbolKind.Property); - compilationContext.RegisterCompilationEndAction(impl.OnCompilationEnd); + static void RegisterImplActions(CompilationStartAnalysisContext compilationContext, Impl impl) + { + compilationContext.RegisterSymbolAction( + impl.OnSymbolAction, + SymbolKind.NamedType, + SymbolKind.Event, + SymbolKind.Field, + SymbolKind.Method); + compilationContext.RegisterSymbolAction( + impl.OnPropertyAction, + SymbolKind.Property); + compilationContext.RegisterCompilationEndAction(impl.OnCompilationEnd); + } } } From 1a7266aa92250841219a0c9355f549582c5d3b6b Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Nov 2022 10:13:55 -0800 Subject: [PATCH 2/3] Add back missing return and assert. --- .../Core/Analyzers/DeclarePublicApiAnalyzer.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs index f565b53cf7..0b4ebc0ff2 100644 --- a/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs +++ b/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs @@ -118,8 +118,12 @@ void CheckAndRegisterImplementation(bool isPublic) context.ReportDiagnostic(cur); } }); + + return; } + Debug.Assert(errors.Count == 0); + RegisterImplActions(compilationContext, new Impl(compilationContext.Compilation, shippedData, unshippedData, isPublic, compilationContext.Options)); static bool TryGetAndValidateApiFiles(AnalyzerOptions options, Compilation compilation, bool isPublic, CancellationToken cancellationToken, List errors, out ApiData shippedData, out ApiData unshippedData) From 37b5291c268b178ffed91ef48042fea436682adf Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Nov 2022 11:57:37 -0800 Subject: [PATCH 3/3] Add test verifying fix. --- .../DeclarePublicAPIAnalyzerTestsBase.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/PublicApiAnalyzers/UnitTests/DeclarePublicAPIAnalyzerTestsBase.cs b/src/PublicApiAnalyzers/UnitTests/DeclarePublicAPIAnalyzerTestsBase.cs index 9ce30748a9..45183671e8 100644 --- a/src/PublicApiAnalyzers/UnitTests/DeclarePublicAPIAnalyzerTestsBase.cs +++ b/src/PublicApiAnalyzers/UnitTests/DeclarePublicAPIAnalyzerTestsBase.cs @@ -310,6 +310,25 @@ private C() { } await VerifyCSharpAsync(source, shippedText, unshippedText, $"[*]\r\n{editorconfigText}", expectedDiagnostics); } + [Fact] + public async Task AnalyzerFilePresent_MissingNonEnabledText() + { + var source = $$""" + + {{EnabledModifierCSharp}} class C + { + private C() { } + } + """; + + string? shippedText = ""; + string? unshippedText = ""; + + var expectedDiagnostics = new[] { GetCSharpResultAt(2, 8 + EnabledModifierCSharp.Length, DeclareNewApiRule, "C") }; + + await VerifyCSharpAsync(source, shippedText, unshippedText, $"[*]\r\ndotnet_public_api_analyzer.require_api_files = true", expectedDiagnostics); + } + [Fact] public async Task EmptyPublicAPIFilesAsync() {