Skip to content

Commit c5c2e19

Browse files
rjmurillorjmurillo
authored
Add Item #6 from the book (#7)
* Fix naming issue with ECS0002 benchmark * Add Live Unit Test configuration * Fix SpanAnalyzer tests to conform to naming conventions * Add ECS0006 - Item #6 Related to BillWagner/EffectiveCSharpAnalyzers#6 * Add ECS0006 - Item #6 Related to BillWagner/EffectiveCSharpAnalyzers#6
1 parent 4125c5c commit c5c2e19

File tree

10 files changed

+428
-4
lines changed

10 files changed

+428
-4
lines changed

Diff for: ‎docs/rules/ECS0006.md

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# ECS0006: Avoid stringly-typed APIs
2+
3+
This rule is described in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/)
4+
5+
## Cause
6+
7+
Using string literals to represent member names or parameter names in APIs.
8+
9+
## Rule description
10+
11+
This rule identifies instances where string literals are used to refer to member names or parameter names. Using string literals in such contexts is prone to errors, especially during refactoring, as the string literals do not update automatically. The `nameof` operator should be used instead to ensure type safety and to facilitate easier refactoring.
12+
13+
## How to fix violations
14+
15+
Replace the string literal with the `nameof` operator to reference the member or parameter name.
16+
17+
## When to suppress warnings
18+
19+
Suppress warnings only if you have a valid reason for using string literals that cannot be replaced with the `nameof` operator. For example, if the string literal represents a dynamic value that cannot be determined at compile time.
20+
21+
## Example of a violation
22+
23+
### Description
24+
25+
Using a string literal to reference a member name.
26+
27+
### Code
28+
29+
```csharp
30+
public class MyClass
31+
{
32+
public static void ExceptionMessage(object thisCantBeNull)
33+
{
34+
if (thisCantBeNull == null)
35+
{
36+
throw new ArgumentNullException(
37+
"thisCantBeNull",
38+
"We told you this cant be null");
39+
}
40+
}
41+
}
42+
```
43+
44+
## Example of how to fix
45+
46+
### Description
47+
48+
Replacing the string literal with the `nameof` operator.
49+
50+
### Code
51+
52+
```csharp
53+
public class MyClass
54+
{
55+
public static void ExceptionMessage(object thisCantBeNull)
56+
{
57+
if (thisCantBeNull == null)
58+
{
59+
throw new ArgumentNullException(
60+
nameof(thisCantBeNull),
61+
"We told you this cant be null");
62+
}
63+
}
64+
}
65+
```

Diff for: ‎effectivecsharpanalyzers.lutconfig

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<LUTConfig Version="1.0">
2+
<Repository />
3+
<ParallelBuilds>true</ParallelBuilds>
4+
<ParallelTestRuns>true</ParallelTestRuns>
5+
<TestCaseTimeout>180000</TestCaseTimeout>
6+
</LUTConfig>

Diff for: ‎src/EffectiveCSharp.Analyzers/AnalyzerReleases.Unshipped.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@
55
Rule ID | Category | Severity | Notes
66
--------|----------|----------|-------
77
ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
8+
ECS0006 | Refactoring | Warning | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/main/docs/ECS0006.md)
89
ECS1000 | Performance | Info | SpanAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/d00a4cc9f61e7d5b392894aad859e46c43a5611c/docs/ECS1000.md)

Diff for: ‎src/EffectiveCSharp.Analyzers/AvoidStringlyTypedApisAnalyzer.cs

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
namespace EffectiveCSharp.Analyzers;
2+
3+
/// <summary>
4+
/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #6 - Avoid stringly typed APIs.
5+
/// </summary>
6+
/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
7+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
8+
public class AvoidStringlyTypedApisAnalyzer : DiagnosticAnalyzer
9+
{
10+
private const string DiagnosticId = DiagnosticIds.AvoidStringlyTypedApis;
11+
private const string Title = "Avoid stringly-typed APIs";
12+
private const string MessageFormat = "Use 'nameof({0})' instead of the string literal \"{0}\"";
13+
14+
private const string Description =
15+
"Replace string literals representing member names with the nameof operator to ensure type safety.";
16+
17+
private const string Category = "Refactoring";
18+
19+
private static readonly DiagnosticDescriptor Rule = new(
20+
DiagnosticId,
21+
Title,
22+
MessageFormat,
23+
Category,
24+
DiagnosticSeverity.Warning,
25+
isEnabledByDefault: true,
26+
description: Description,
27+
helpLinkUri:
28+
$"https://github.com/rjmurillo/EffectiveCSharp.Analyzers{ThisAssembly.GitCommitId}/docs/{DiagnosticId}.md");
29+
30+
/// <inheritdoc/>
31+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
32+
33+
/// <inheritdoc/>
34+
public override void Initialize(AnalysisContext context)
35+
{
36+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
37+
context.EnableConcurrentExecution();
38+
context.RegisterSyntaxNodeAction(AnalyzeLiteralExpression, SyntaxKind.StringLiteralExpression);
39+
}
40+
41+
private static void AnalyzeLiteralExpression(SyntaxNodeAnalysisContext context)
42+
{
43+
LiteralExpressionSyntax literalExpression = (LiteralExpressionSyntax)context.Node;
44+
string literalValue = literalExpression.Token.ValueText;
45+
46+
// Walk up the syntax tree to find the containing class or method
47+
TypeDeclarationSyntax? containingClass = literalExpression.FirstAncestorOrSelf<TypeDeclarationSyntax>();
48+
MethodDeclarationSyntax? containingMethod = literalExpression.FirstAncestorOrSelf<MethodDeclarationSyntax>();
49+
50+
SemanticModel semanticModel = context.SemanticModel;
51+
52+
if (containingClass != null
53+
&& semanticModel.GetDeclaredSymbol(containingClass, context.CancellationToken) is { } containingTypeSymbol)
54+
{
55+
IEnumerable<string> memberNames = containingTypeSymbol.GetMembers().Select(member => member.Name);
56+
57+
if (memberNames.Contains(literalValue, StringComparer.Ordinal))
58+
{
59+
Diagnostic diagnostic = literalExpression.GetLocation().CreateDiagnostic(Rule, literalValue);
60+
context.ReportDiagnostic(diagnostic);
61+
}
62+
}
63+
64+
if (containingMethod != null
65+
&& semanticModel.GetDeclaredSymbol(containingMethod, context.CancellationToken) is { } methodSymbol)
66+
{
67+
IEnumerable<string> parameterNames = methodSymbol.Parameters.Select(parameter => parameter.Name);
68+
69+
if (parameterNames.Contains(literalValue, StringComparer.Ordinal))
70+
{
71+
Diagnostic diagnostic = literalExpression.GetLocation().CreateDiagnostic(Rule, literalValue);
72+
context.ReportDiagnostic(diagnostic);
73+
}
74+
}
75+
}
76+
}

Diff for: ‎src/EffectiveCSharp.Analyzers/AvoidStringlyTypedApisCodeFixProvider.cs

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
namespace EffectiveCSharp.Analyzers;
2+
3+
/// <summary>
4+
/// A <see cref="CodeFixProvider"/> that provides a code fix for the <see cref="AvoidStringlyTypedApisAnalyzer"/>.
5+
/// </summary>
6+
/// <seealso cref="Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider" />
7+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AvoidStringlyTypedApisCodeFixProvider))]
8+
[Shared]
9+
public class AvoidStringlyTypedApisCodeFixProvider : CodeFixProvider
10+
{
11+
private const string Title = "Use nameof operator";
12+
13+
/// <inheritdoc/>
14+
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.AvoidStringlyTypedApis);
15+
16+
/// <inheritdoc/>
17+
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
18+
19+
/// <inheritdoc/>
20+
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
21+
{
22+
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
23+
Diagnostic diagnostic = context.Diagnostics.First();
24+
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
25+
26+
SyntaxNode? node = root?.FindNode(diagnosticSpan);
27+
28+
LiteralExpressionSyntax? literalExpression = node switch
29+
{
30+
// Check if the node is a LiteralExpressionSyntax directly
31+
LiteralExpressionSyntax literalNode => literalNode,
32+
33+
// Check if the node is an ArgumentSyntax containing a LiteralExpressionSyntax
34+
ArgumentSyntax { Expression: LiteralExpressionSyntax argLiteralNode } => argLiteralNode,
35+
36+
_ => null
37+
};
38+
39+
if (literalExpression != null)
40+
{
41+
context.RegisterCodeFix(
42+
CodeAction.Create(
43+
title: Title,
44+
createChangedSolution: c => UseNameofOperatorAsync(context.Document, literalExpression, c),
45+
equivalenceKey: Title),
46+
diagnostic);
47+
}
48+
}
49+
50+
private static async Task<Solution> UseNameofOperatorAsync(Document document, LiteralExpressionSyntax? literalExpression, CancellationToken cancellationToken)
51+
{
52+
string literalValue = literalExpression?.Token.ValueText;
53+
54+
// Walk up the syntax tree to find the containing class or method
55+
TypeDeclarationSyntax? containingClass = literalExpression?.FirstAncestorOrSelf<TypeDeclarationSyntax>();
56+
MethodDeclarationSyntax? containingMethod = literalExpression?.FirstAncestorOrSelf<MethodDeclarationSyntax>();
57+
58+
string? nameofExpressionText = null;
59+
60+
if (containingClass != null)
61+
{
62+
SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
63+
64+
if (semanticModel.GetDeclaredSymbol(containingClass, cancellationToken) is { } containingTypeSymbol)
65+
{
66+
IEnumerable<string> memberNames = containingTypeSymbol.GetMembers().Select(member => member.Name);
67+
68+
if (memberNames.Contains(literalValue))
69+
{
70+
nameofExpressionText = $"nameof({literalValue})";
71+
}
72+
}
73+
}
74+
75+
if (nameofExpressionText == null && containingMethod != null)
76+
{
77+
SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
78+
79+
if (semanticModel.GetDeclaredSymbol(containingMethod, cancellationToken) is { } methodSymbol)
80+
{
81+
IEnumerable<string> parameterNames = methodSymbol.Parameters.Select(parameter => parameter.Name);
82+
83+
if (parameterNames.Contains(literalValue))
84+
{
85+
nameofExpressionText = $"nameof({literalValue})";
86+
}
87+
}
88+
}
89+
90+
if (nameofExpressionText == null)
91+
{
92+
return document.Project.Solution;
93+
}
94+
95+
if (literalExpression != null)
96+
{
97+
ExpressionSyntax nameofExpression = SyntaxFactory.ParseExpression(nameofExpressionText)
98+
.WithTriviaFrom(literalExpression);
99+
100+
SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
101+
SyntaxNode? newRoot = root?.ReplaceNode(literalExpression, nameofExpression);
102+
103+
if (newRoot != null)
104+
{
105+
return document.WithSyntaxRoot(newRoot).Project.Solution;
106+
}
107+
}
108+
109+
return document.Project.Solution;
110+
}
111+
}

Diff for: ‎src/EffectiveCSharp.Analyzers/DiagnosticIds.cs

+1
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
internal static class DiagnosticIds
44
{
55
internal const string PreferReadonlyOverConst = "ECS0002";
6+
internal const string AvoidStringlyTypedApis = "ECS0006";
67
internal const string UseSpanInstead = "ECS1000";
78
}

Diff for: ‎tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0002Benchmarks.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ internal class {name}
3434
}
3535

3636
[Benchmark]
37-
public async Task Ecs1000WithDiagnostics()
37+
public async Task Ecs0002WithDiagnostics()
3838
{
3939
ImmutableArray<Diagnostic> diagnostics =
4040
(await TestCompilation!
@@ -50,7 +50,7 @@ public async Task Ecs1000WithDiagnostics()
5050
}
5151

5252
[Benchmark(Baseline = true)]
53-
public async Task Ecs1000Baseline()
53+
public async Task Ecs0002Baseline()
5454
{
5555
ImmutableArray<Diagnostic> diagnostics =
5656
(await BaselineCompilation!

Diff for: ‎tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0006Benchmarks.cs

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
namespace EffectiveCSharp.Analyzers.Benchmarks;
2+
3+
[InProcess]
4+
[MemoryDiagnoser]
5+
public class Ecs0006Benchmarks
6+
{
7+
private static CompilationWithAnalyzers? BaselineCompilation { get; set; }
8+
9+
private static CompilationWithAnalyzers? TestCompilation { get; set; }
10+
11+
[IterationSetup]
12+
[SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "Async setup not supported in BenchmarkDotNet.See https://github.com/dotnet/BenchmarkDotNet/issues/2442.")]
13+
public static void SetupCompilation()
14+
{
15+
List<(string Name, string Content)> sources = [];
16+
for (int index = 0; index < Constants.NumberOfCodeFiles; index++)
17+
{
18+
string name = $"TypeName{index}";
19+
sources.Add((name, @$"
20+
using System;
21+
22+
public class {name}
23+
{{
24+
public static void ExceptionMessage(object thisCantBeNull)
25+
{{
26+
if (thisCantBeNull == null)
27+
{{
28+
throw new ArgumentNullException(
29+
""thisCantBeNull"",
30+
""We told you this cant be null"");
31+
}}
32+
}}
33+
}}
34+
"));
35+
}
36+
37+
(BaselineCompilation, TestCompilation) =
38+
BenchmarkCSharpCompilationFactory
39+
.CreateAsync<AvoidStringlyTypedApisAnalyzer>(sources.ToArray())
40+
.GetAwaiter()
41+
.GetResult();
42+
}
43+
44+
[Benchmark]
45+
public async Task Ecs0006WithDiagnostics()
46+
{
47+
ImmutableArray<Diagnostic> diagnostics =
48+
(await TestCompilation!
49+
.GetAnalysisResultAsync(CancellationToken.None)
50+
.ConfigureAwait(false))
51+
.AssertValidAnalysisResult()
52+
.GetAllDiagnostics();
53+
54+
if (diagnostics.Length != Constants.NumberOfCodeFiles)
55+
{
56+
throw new InvalidOperationException($"Expected '{Constants.NumberOfCodeFiles:N0}' analyzer diagnostics but found '{diagnostics.Length}'");
57+
}
58+
}
59+
60+
[Benchmark(Baseline = true)]
61+
public async Task Ecs0006Baseline()
62+
{
63+
ImmutableArray<Diagnostic> diagnostics =
64+
(await BaselineCompilation!
65+
.GetAnalysisResultAsync(CancellationToken.None)
66+
.ConfigureAwait(false))
67+
.AssertValidAnalysisResult()
68+
.GetAllDiagnostics();
69+
70+
if (diagnostics.Length != 0)
71+
{
72+
throw new InvalidOperationException($"Expected no analyzer diagnostics but found '{diagnostics.Length}'");
73+
}
74+
}
75+
}

0 commit comments

Comments
 (0)