Skip to content

Commit 2611ff8

Browse files
omsmithomsmith
committed
D2LXXXX: ban default constructor of certain structs
This is a generic version of, and supercedes D2L0029. Includes codefixes to use the initializer for the likely-to-be-intended usecase. This may be a miss for some things, such as Guid.Empty vs Guid.NewGuid(). Could theoretically suggest both.
1 parent df53873 commit 2611ff8

10 files changed

Lines changed: 385 additions & 124 deletions

File tree

‎src/D2L.CodeStyle.Analyzers/ApiUsage/System.Collections.Immutable/ImmutableCollectionsAnalyzer.cs‎

Lines changed: 0 additions & 70 deletions
This file was deleted.

‎src/D2L.CodeStyle.Analyzers/Diagnostics.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -225,16 +225,6 @@ public static class Diagnostics {
225225
description: "ImmutableGeneric can only be applied to closed (fully bound) generic types."
226226
);
227227

228-
public static readonly DiagnosticDescriptor DontUseImmutableArrayConstructor = new DiagnosticDescriptor(
229-
id: "D2L0029",
230-
title: "Don't use the default constructor for ImmutableArray<T>",
231-
messageFormat: "The default constructor for ImmutableArray<T> doesn't correctly initialize the object and leads to runtime errors. Use ImmutableArray<T>.Empty for empty arrays, ImmutableArray.Create() for simple cases and ImmutableArray.Builder<T> for more complicated cases.",
232-
category: "Correctness",
233-
defaultSeverity: DiagnosticSeverity.Error,
234-
isEnabledByDefault: true,
235-
description: "The default constructor for ImmutableArray<T> doesn't correctly initialize the object and leads to runtime errors. Use ImmutableArray<T>.Empty for empty arrays, ImmutableArray.Create() for simple cases and ImmutableArray.Builder<T> for more complicated cases."
236-
);
237-
238228
public static readonly DiagnosticDescriptor UnnecessaryMutabilityAnnotation = new DiagnosticDescriptor(
239229
id: "D2L0030",
240230
title: "Unnecessary mutability annotation should be removed to keep the code base clean",
@@ -495,5 +485,15 @@ public static class Diagnostics {
495485
description: "Awaited task should have 'continueOnCapturedContext' configured (preferably with 'false')."
496486
);
497487

488+
public static readonly DiagnosticDescriptor DontCallDefaultStructConstructor = new DiagnosticDescriptor(
489+
id: "D2LXXXX",
490+
title: "Don't call default struct constructor",
491+
messageFormat: "Don't call default constructor of '{0}'",
492+
description: "The default constructor of this type is likely to lead to unexpected results and another initializer should be called instead.",
493+
category: "Correctness",
494+
defaultSeverity: DiagnosticSeverity.Error,
495+
isEnabledByDefault: true
496+
);
497+
498498
}
499499
}

‎src/D2L.CodeStyle.Analyzers/Language/DefaultStructCreation/DefaultStructCreationAnalyzer.Codefix.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
using System.Collections.Immutable;
2+
using System.Threading.Tasks;
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CodeActions;
5+
using Microsoft.CodeAnalysis.CodeFixes;
6+
using Microsoft.CodeAnalysis.CSharp;
7+
using Microsoft.CodeAnalysis.CSharp.Syntax;
8+
9+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {
10+
partial class DefaultStructCreationAnalyzer {
11+
12+
[ExportCodeFixProvider(
13+
LanguageNames.CSharp,
14+
Name = nameof( UseSuggestedInitializerCodefix )
15+
)]
16+
public sealed class UseSuggestedInitializerCodefix : CodeFixProvider {
17+
18+
public override ImmutableArray<string> FixableDiagnosticIds
19+
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor.Id );
20+
21+
public override FixAllProvider GetFixAllProvider()
22+
=> WellKnownFixAllProviders.BatchFixer;
23+
24+
public override async Task RegisterCodeFixesAsync(
25+
CodeFixContext ctx
26+
) {
27+
var root = await ctx.Document
28+
.GetSyntaxRootAsync( ctx.CancellationToken )
29+
.ConfigureAwait( false );
30+
31+
foreach( var diagnostic in ctx.Diagnostics ) {
32+
var span = diagnostic.Location.SourceSpan;
33+
34+
SyntaxNode syntax = root.FindNode( span, getInnermostNodeForTie: true );
35+
if( !( syntax is ObjectCreationExpressionSyntax creationExpression ) ) {
36+
continue;
37+
}
38+
39+
ctx.RegisterCodeFix(
40+
CodeAction.Create(
41+
title: diagnostic.Properties[ACTION_TITLE_KEY],
42+
createChangedDocument: ct => {
43+
SyntaxNode replacement = SyntaxFactory.ParseExpression(
44+
diagnostic.Properties[REPLACEMENT_KEY]
45+
);
46+
47+
SyntaxNode newRoot = root.ReplaceNode( creationExpression, replacement );
48+
Document newDoc = ctx.Document.WithSyntaxRoot( newRoot );
49+
50+
return Task.FromResult( newDoc );
51+
}
52+
),
53+
diagnostic
54+
);
55+
}
56+
}
57+
}
58+
}
59+
}

‎src/D2L.CodeStyle.Analyzers/Language/DefaultStructCreation/DefaultStructCreationAnalyzer.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
using System.Collections.Immutable;
2+
using D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements;
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
using Microsoft.CodeAnalysis.Operations;
7+
8+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {
9+
10+
[DiagnosticAnalyzer( LanguageNames.CSharp )]
11+
internal sealed partial class DefaultStructCreationAnalyzer : DiagnosticAnalyzer {
12+
13+
internal const string ACTION_TITLE_KEY = nameof( DefaultStructCreationAnalyzer ) + ".ActionTitle";
14+
internal const string REPLACEMENT_KEY = nameof( DefaultStructCreationAnalyzer ) + ".Replacement";
15+
16+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
17+
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor );
18+
19+
public override void Initialize( AnalysisContext context ) {
20+
//context.EnableConcurrentExecution();
21+
22+
context.ConfigureGeneratedCodeAnalysis(
23+
GeneratedCodeAnalysisFlags.None
24+
);
25+
26+
context.RegisterCompilationStartAction( RegisterAnalyzer );
27+
}
28+
29+
private static void RegisterAnalyzer(
30+
CompilationStartAnalysisContext context
31+
) {
32+
var replacers = ImmutableArray.Create(
33+
NewGuidBackedIdTypeReplacer.Instance,
34+
new NewGuidReplacer( context.Compilation ),
35+
new NewImmutableArrayReplacer( context.Compilation )
36+
);
37+
38+
context.RegisterOperationAction(
39+
ctx => AnalyzeObjectCreation(
40+
context: ctx,
41+
operation: ctx.Operation as IObjectCreationOperation,
42+
replacers: replacers
43+
),
44+
OperationKind.ObjectCreation
45+
);
46+
}
47+
48+
private static void AnalyzeObjectCreation(
49+
OperationAnalysisContext context,
50+
IObjectCreationOperation operation,
51+
ImmutableArray<IDefaultStructCreationReplacer> replacers
52+
) {
53+
if( operation.Type.TypeKind != TypeKind.Struct ) {
54+
return;
55+
}
56+
57+
IMethodSymbol constructor = operation.Constructor;
58+
if( constructor.Parameters.Length > 0 ) {
59+
return;
60+
}
61+
62+
INamedTypeSymbol structType = operation.Type as INamedTypeSymbol;
63+
64+
foreach( var replacer in replacers ) {
65+
66+
if( !replacer.CanReplace( structType ) ) {
67+
continue;
68+
}
69+
70+
var syntax = operation.Syntax as ObjectCreationExpressionSyntax;
71+
SyntaxNode replacement = replacer
72+
.GetReplacement( structType, syntax.Type )
73+
.WithTriviaFrom( syntax );
74+
75+
context.ReportDiagnostic( Diagnostic.Create(
76+
descriptor: Diagnostics.DontCallDefaultStructConstructor,
77+
location: operation.Syntax.GetLocation(),
78+
properties: ImmutableDictionary<string, string>
79+
.Empty
80+
.Add( ACTION_TITLE_KEY, replacer.Title )
81+
.Add( REPLACEMENT_KEY, replacement.ToFullString() ),
82+
structType.ToDisplayString( SymbolDisplayFormat.MinimallyQualifiedFormat )
83+
) );
84+
}
85+
}
86+
}
87+
}

‎src/D2L.CodeStyle.Analyzers/Language/DefaultStructCreation/Replacements/IDefaultStructCreationReplacer.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using Microsoft.CodeAnalysis;
2+
using Microsoft.CodeAnalysis.CSharp.Syntax;
3+
4+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {
5+
6+
internal interface IDefaultStructCreationReplacer {
7+
8+
string Title { get; }
9+
10+
bool CanReplace(
11+
INamedTypeSymbol structType
12+
);
13+
14+
SyntaxNode GetReplacement(
15+
INamedTypeSymbol structType,
16+
TypeSyntax structName
17+
);
18+
19+
}
20+
}

‎src/D2L.CodeStyle.Analyzers/Language/DefaultStructCreation/Replacements/NewGuidBackedIdTypeReplacer.cs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
6+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {
7+
8+
internal sealed class NewGuidBackedIdTypeReplacer : IDefaultStructCreationReplacer {
9+
10+
public static readonly IDefaultStructCreationReplacer Instance = new NewGuidBackedIdTypeReplacer();
11+
12+
private NewGuidBackedIdTypeReplacer() { }
13+
14+
string IDefaultStructCreationReplacer.Title { get; } = "Use GenerateNew()";
15+
16+
bool IDefaultStructCreationReplacer.CanReplace(
17+
INamedTypeSymbol structType
18+
) {
19+
ImmutableArray<ISymbol> maybeGenerateNew = structType.GetMembers( "GenerateNew" );
20+
if( maybeGenerateNew.Length != 1 ) {
21+
return false;
22+
}
23+
24+
if( !( maybeGenerateNew[0] is IMethodSymbol generateNew ) ) {
25+
return false;
26+
}
27+
28+
if( generateNew.Parameters.Length != 0 ) {
29+
return false;
30+
}
31+
32+
// Sure seems like one of our Guid-backed IdTypes
33+
return true;
34+
}
35+
36+
SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
37+
INamedTypeSymbol structType,
38+
TypeSyntax structName
39+
) {
40+
return SyntaxFactory
41+
.InvocationExpression(
42+
SyntaxFactory.MemberAccessExpression(
43+
SyntaxKind.SimpleMemberAccessExpression,
44+
structName,
45+
SyntaxFactory.IdentifierName( "GenerateNew" )
46+
)
47+
);
48+
}
49+
50+
}
51+
}

‎src/D2L.CodeStyle.Analyzers/Language/DefaultStructCreation/Replacements/NewGuidReplacer.cs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
using Microsoft.CodeAnalysis;
2+
using Microsoft.CodeAnalysis.CSharp;
3+
using Microsoft.CodeAnalysis.CSharp.Syntax;
4+
5+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {
6+
7+
internal sealed class NewGuidReplacer : IDefaultStructCreationReplacer {
8+
string IDefaultStructCreationReplacer.Title { get; } = "Use Guid.NewGuid()";
9+
10+
private readonly INamedTypeSymbol m_guidType;
11+
12+
public NewGuidReplacer(
13+
Compilation compilation
14+
) {
15+
m_guidType = compilation.GetTypeByMetadataName( "System.Guid" );
16+
}
17+
18+
bool IDefaultStructCreationReplacer.CanReplace(
19+
INamedTypeSymbol structType
20+
) {
21+
if( m_guidType == null ) {
22+
return false;
23+
}
24+
25+
if( m_guidType.TypeKind == TypeKind.Error ) {
26+
return false;
27+
}
28+
29+
if( m_guidType != structType ) {
30+
return false;
31+
}
32+
33+
return true;
34+
}
35+
36+
SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
37+
INamedTypeSymbol structType,
38+
TypeSyntax structName
39+
) {
40+
return SyntaxFactory
41+
.InvocationExpression(
42+
SyntaxFactory.MemberAccessExpression(
43+
SyntaxKind.SimpleMemberAccessExpression,
44+
structName,
45+
SyntaxFactory.IdentifierName( "NewGuid" )
46+
)
47+
);
48+
}
49+
50+
}
51+
}

0 commit comments

Comments
 (0)