From 385f34aa95cf965b729c1373eeebb1c190c5dd88 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 12 Aug 2024 14:31:07 -0400 Subject: [PATCH] RequireNamedArgumentsAnalyzer: add support for exemptions (#961) ExemptSymbolsBuilder is a helper for building sets of exempt symbols without bespoke logic per-analyzer. The AdditionalFile format is similar to Roslyn's BannedApiAnalyzer: https://github.com/dotnet/roslyn-analyzers/blob/3211f48253bc18560156d90dc5e710d35f7d03fa/src/Microsoft.CodeAnalysis.BannedApiAnalyzers/BannedApiAnalyzers.Help.md In this case the optional [;Description Text] is excluded from the format, as exemptions do not lead to user-facing messages. Comments are supported for developers to leave notes justifying the exemption. --- .../Helpers/ExemptSymbolsBuilder.cs | 100 ++++++++++++++++++ .../Language/RequireNamedArgumentsAnalyzer.cs | 26 ++++- .../Specs/RequireNamedArgumentsAnalyzer.cs | 25 +++++ 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 src/D2L.CodeStyle.Analyzers/Helpers/ExemptSymbolsBuilder.cs diff --git a/src/D2L.CodeStyle.Analyzers/Helpers/ExemptSymbolsBuilder.cs b/src/D2L.CodeStyle.Analyzers/Helpers/ExemptSymbolsBuilder.cs new file mode 100644 index 00000000..3b9411b9 --- /dev/null +++ b/src/D2L.CodeStyle.Analyzers/Helpers/ExemptSymbolsBuilder.cs @@ -0,0 +1,100 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; + +namespace D2L.CodeStyle.Analyzers.Helpers; + +internal sealed class ExemptSymbolsBuilder { + + private readonly Compilation m_compilation; + private readonly AnalyzerOptions m_analyzerOptions; + private readonly CancellationToken m_cancellationToken; + + private readonly HashSet m_exemptions = new( SymbolEqualityComparer.Default ); + private bool m_built = false; + + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "MicrosoftCodeAnalysisPerformance", + "RS1012:Start action has no registered actions", + Justification = "Not an analyzer" + )] + public ExemptSymbolsBuilder( + CompilationStartAnalysisContext context + ) { + m_compilation = context.Compilation; + m_analyzerOptions = context.Options; + m_cancellationToken = context.CancellationToken; + } + + public HashSet Build() { + ThrowIfBuilt(); + + m_built = true; + return m_exemptions; + } + + /// + /// Loads exemptions from AdditionalFiles matching ".txt" and ".*.txt". + /// Each file should contain a series of lines, each starting with a DocumentationCommentDelcarationId, with an optional trailing comment (//). + /// + public ExemptSymbolsBuilder AddFromAdditionalFiles( + string fileNameBase + ) { + ThrowIfBuilt(); + + fileNameBase += "."; + + foreach( AdditionalText file in m_analyzerOptions.AdditionalFiles ) { + string fileName = Path.GetFileName( file.Path ); + if( fileName is null ) { + continue; + } + + bool fileNameMatch = fileName.StartsWith( fileNameBase, StringComparison.Ordinal ) && fileName.EndsWith( ".txt", StringComparison.Ordinal ); + if( !fileNameMatch ) { + continue; + } + + SourceText? sourceText = file.GetText( m_cancellationToken ); + if( sourceText is null ) { + continue; + } + + foreach( TextLine line in sourceText.Lines ) { + ReadOnlySpan text = line.ToString().AsSpan(); + + int commentIndex = text.IndexOf( "//".AsSpan(), StringComparison.Ordinal ); + if( commentIndex != -1 ) { + text = text.Slice( 0, commentIndex ); + } + + text = text.TrimEnd(); + + if( text.Length > 0 ) { + AddFromDocumentationCommentId( text.ToString() ); + } + } + } + + return this; + } + + public ExemptSymbolsBuilder AddFromDocumentationCommentId( string id ) { + ThrowIfBuilt(); + + ImmutableArray symbols = DocumentationCommentId.GetSymbolsForDeclarationId( id, m_compilation ); + foreach( var symbol in symbols ) { + m_exemptions.Add( symbol ); + } + + return this; + } + + private void ThrowIfBuilt() { + if( m_built ) { + throw new InvalidOperationException( "Builder instance has already been used" ); + } + } + +} diff --git a/src/D2L.CodeStyle.Analyzers/Language/RequireNamedArgumentsAnalyzer.cs b/src/D2L.CodeStyle.Analyzers/Language/RequireNamedArgumentsAnalyzer.cs index e9db6766..4d3cf214 100644 --- a/src/D2L.CodeStyle.Analyzers/Language/RequireNamedArgumentsAnalyzer.cs +++ b/src/D2L.CodeStyle.Analyzers/Language/RequireNamedArgumentsAnalyzer.cs @@ -1,6 +1,7 @@ #nullable enable using System.Collections.Immutable; +using D2L.CodeStyle.Analyzers.Helpers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -52,13 +53,29 @@ private static void OnCompilationStart( CompilationStartAnalysisContext context return; } + HashSet exemptions = new ExemptSymbolsBuilder( context ) + .AddFromAdditionalFiles( "D2L.CodeStyle.RequireNamedArguments.Exemptions" ) + + // HashCode.Combine takes a series of args named value1..valueN which are not useful to name + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``1(``0)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``2(``0,``1)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``3(``0,``1,``2)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``4(``0,``1,``2,``3)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``5(``0,``1,``2,``3,``4)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``6(``0,``1,``2,``3,``4,``5)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``7(``0,``1,``2,``3,``4,``5,``6)" ) + .AddFromDocumentationCommentId( "M:System.HashCode.Combine``8(``0,``1,``2,``3,``4,``5,``6,``7)" ) + + .Build(); + INamedTypeSymbol? lambdaExpresssion = context.Compilation.GetTypeByMetadataName( LambdaExpressionMetadataName ); context.RegisterOperationAction( ctx => AnalyzeInvocation( ctx, requiredNamedArguments, - lambdaExpresssion + lambdaExpresssion, + exemptions ), OperationKind.Invocation, OperationKind.ObjectCreation @@ -68,7 +85,8 @@ private static void OnCompilationStart( CompilationStartAnalysisContext context private static void AnalyzeInvocation( OperationAnalysisContext ctx, INamedTypeSymbol requireNamedArgumentsSymbol, - INamedTypeSymbol? lambdaExpresssionSymbol + INamedTypeSymbol? lambdaExpresssionSymbol, + HashSet exemptions ) { (IMethodSymbol targetMethod, ImmutableArray args) = ctx.Operation switch { IInvocationOperation op => (op.TargetMethod, op.Arguments), @@ -84,6 +102,10 @@ private static void AnalyzeInvocation( return; } + if( exemptions.Contains( targetMethod.OriginalDefinition ) ) { + return; + } + bool requireNamedArguments = HasRequireNamedArgumentsAttribute( requireNamedArgumentsSymbol, targetMethod ); // Don't complain about single argument functions because they're diff --git a/tests/D2L.CodeStyle.Analyzers.Test/Specs/RequireNamedArgumentsAnalyzer.cs b/tests/D2L.CodeStyle.Analyzers.Test/Specs/RequireNamedArgumentsAnalyzer.cs index d3e60f13..7de22396 100644 --- a/tests/D2L.CodeStyle.Analyzers.Test/Specs/RequireNamedArgumentsAnalyzer.cs +++ b/tests/D2L.CodeStyle.Analyzers.Test/Specs/RequireNamedArgumentsAnalyzer.cs @@ -1,7 +1,21 @@ // analyzer: D2L.CodeStyle.Analyzers.Language.RequireNamedArgumentsAnalyzer, D2L.CodeStyle.Analyzers +using System; using D2L.CodeStyle.Annotations.Contract; +namespace System { + public struct HashCode { + public static int Combine( T1 value1 ) { } + public static int Combine( T1 value1, T2 value2 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3, T4 value4 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6, T7 value7 ) { } + public static int Combine( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6, T7 value7, T8 value8 ) { } + } +} + namespace D2L { public static class Foo { @@ -240,6 +254,17 @@ public static void Test() { /* NamedArgumentsRequired */ funcWithOutParameter( out int out2 ) /**/; } #endregion + + #region exempted methods do not trigger a diagnostic + HashCode.Combine( 1 ); + HashCode.Combine( 1, 2 ); + HashCode.Combine( 1, 2, 3 ); + HashCode.Combine( 1, 2, 3, 4 ); + HashCode.Combine( 1, 2, 3, 4, 5 ); + HashCode.Combine( 1, 2, 3, 4, 5, 6 ); + HashCode.Combine( 1, 2, 3, 4, 5, 6, 7 ); + HashCode.Combine( 1, 2, 3, 4, 5, 6, 7, 8 ); + #endregion } public abstract class SomeBaseClass {