Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequireNamedArgumentsAnalyzer: add support for exemptions #961

Merged
merged 2 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading