Skip to content

Commit

Permalink
RequireNamedArgumentsAnalyzer: add support for exemptions (#961)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
omsmith authored Aug 12, 2024
1 parent ed3ae60 commit 385f34a
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 2 deletions.
100 changes: 100 additions & 0 deletions src/D2L.CodeStyle.Analyzers/Helpers/ExemptSymbolsBuilder.cs
Original file line number Diff line number Diff line change
@@ -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<ISymbol> 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<ISymbol> Build() {
ThrowIfBuilt();

m_built = true;
return m_exemptions;
}

/// <summary>
/// Loads exemptions from AdditionalFiles matching "<paramref name="fileNameBase"/>.txt" and "<paramref name="fileNameBase"/>.*.txt".
/// Each file should contain a series of lines, each starting with a DocumentationCommentDelcarationId, with an optional trailing comment (//).
/// </summary>
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<char> 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<ISymbol> 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" );
}
}

}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -52,13 +53,29 @@ private static void OnCompilationStart( CompilationStartAnalysisContext context
return;
}

HashSet<ISymbol> 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
Expand All @@ -68,7 +85,8 @@ private static void OnCompilationStart( CompilationStartAnalysisContext context
private static void AnalyzeInvocation(
OperationAnalysisContext ctx,
INamedTypeSymbol requireNamedArgumentsSymbol,
INamedTypeSymbol? lambdaExpresssionSymbol
INamedTypeSymbol? lambdaExpresssionSymbol,
HashSet<ISymbol> exemptions
) {
(IMethodSymbol targetMethod, ImmutableArray<IArgumentOperation> args) = ctx.Operation switch {
IInvocationOperation op => (op.TargetMethod, op.Arguments),
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>( T1 value1 ) { }
public static int Combine<T1, T2>( T1 value1, T2 value2 ) { }
public static int Combine<T1, T2, T3>( T1 value1, T2 value2, T3 value3 ) { }
public static int Combine<T1, T2, T3, T4>( T1 value1, T2 value2, T3 value3, T4 value4 ) { }
public static int Combine<T1, T2, T3, T4, T5>( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5 ) { }
public static int Combine<T1, T2, T3, T4, T5, T6>( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6 ) { }
public static int Combine<T1, T2, T3, T4, T5, T6, T7>( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6, T7 value7 ) { }
public static int Combine<T1, T2, T3, T4, T5, T6, T7, T8>( T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6, T7 value7, T8 value8 ) { }
}
}

namespace D2L {

public static class Foo {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 385f34a

Please sign in to comment.