Skip to content

Commit b254c4d

Browse files
committed
D2LXXXX: don't call default construct of a struct
generic version of the ImmutableArray<> analyzer has a codefix which can replace with known initializers
1 parent 847d0b4 commit b254c4d

File tree

8 files changed

+345
-0
lines changed

8 files changed

+345
-0
lines changed

src/D2L.CodeStyle.Analyzers/Diagnostics.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,5 +485,15 @@ public static class Diagnostics {
485485
description: "[ReadOnly] paramaters must not be assigned to or passed by non-readonly reference."
486486
);
487487

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 struct constructor",
492+
description: "The default parameterless constructor for a struct is overridable, but not removable. It is quite possible it's not doing what you expect. Try another constructor or initializer?",
493+
category: "Correctness",
494+
defaultSeverity: DiagnosticSeverity.Info,
495+
isEnabledByDefault: true
496+
);
497+
488498
}
489499
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
using System.Collections.Immutable;
2+
using System.Threading.Tasks;
3+
using D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements;
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CodeActions;
6+
using Microsoft.CodeAnalysis.CodeFixes;
7+
using Microsoft.CodeAnalysis.CSharp;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
10+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {
11+
partial class DefaultStructCreationAnalyzer {
12+
13+
[ExportCodeFixProvider(
14+
LanguageNames.CSharp,
15+
Name = nameof( UseSuggestedInitializerCodefix )
16+
)]
17+
public sealed class UseSuggestedInitializerCodefix : CodeFixProvider {
18+
19+
public override ImmutableArray<string> FixableDiagnosticIds
20+
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor.Id );
21+
22+
public override FixAllProvider GetFixAllProvider()
23+
=> WellKnownFixAllProviders.BatchFixer;
24+
25+
public override async Task RegisterCodeFixesAsync(
26+
CodeFixContext ctx
27+
) {
28+
var root = await ctx.Document
29+
.GetSyntaxRootAsync( ctx.CancellationToken )
30+
.ConfigureAwait( false );
31+
32+
var model = await ctx.Document
33+
.GetSemanticModelAsync( ctx.CancellationToken )
34+
.ConfigureAwait( false );
35+
36+
var compilation = model.Compilation;
37+
38+
var replacers = ImmutableArray.Create<IDefaultStructCreationReplacer>(
39+
new NewGuidBackedIdTypeReplacer(),
40+
new NewGuidReplacer( compilation ),
41+
new NewImmutableArrayReplacer( compilation )
42+
);
43+
44+
foreach( var diagnostic in ctx.Diagnostics ) {
45+
var span = diagnostic.Location.SourceSpan;
46+
47+
SyntaxNode syntax = root.FindNode( span, getInnermostNodeForTie: true );
48+
if( !( syntax is ObjectCreationExpressionSyntax creationExpression ) ) {
49+
continue;
50+
}
51+
52+
INamedTypeSymbol structType = model.GetTypeInfo( creationExpression ).Type as INamedTypeSymbol;
53+
if( structType == null || structType.TypeKind == TypeKind.Error ) {
54+
continue;
55+
}
56+
57+
foreach( var replacer in replacers ) {
58+
59+
if( !replacer.CanReplace( structType ) ) {
60+
continue;
61+
}
62+
63+
ctx.RegisterCodeFix(
64+
CodeAction.Create(
65+
title: replacer.Title,
66+
createChangedDocument: ct => {
67+
SyntaxNode replacement = replacer.GetReplacement(
68+
structType,
69+
creationExpression.Type
70+
);
71+
72+
replacement = replacement.WithTriviaFrom( creationExpression );
73+
74+
SyntaxNode newRoot = root.ReplaceNode( creationExpression, replacement );
75+
Document newDoc = ctx.Document.WithSyntaxRoot( newRoot );
76+
77+
return Task.FromResult( newDoc );
78+
}
79+
),
80+
diagnostic
81+
);
82+
83+
}
84+
}
85+
}
86+
}
87+
}
88+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.Diagnostics;
4+
using Microsoft.CodeAnalysis.Operations;
5+
6+
namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {
7+
8+
[DiagnosticAnalyzer( LanguageNames.CSharp )]
9+
internal sealed partial class DefaultStructCreationAnalyzer : DiagnosticAnalyzer {
10+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
11+
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor );
12+
13+
public override void Initialize( AnalysisContext context ) {
14+
context.EnableConcurrentExecution();
15+
16+
context.ConfigureGeneratedCodeAnalysis(
17+
GeneratedCodeAnalysisFlags.None
18+
);
19+
20+
context.RegisterOperationAction(
21+
ctx => AnalyzeObjectCreation(
22+
context: ctx,
23+
operation: ctx.Operation as IObjectCreationOperation
24+
),
25+
OperationKind.ObjectCreation
26+
);
27+
}
28+
29+
private static void AnalyzeObjectCreation(
30+
OperationAnalysisContext context,
31+
IObjectCreationOperation operation
32+
) {
33+
ITypeSymbol type = operation.Type;
34+
if( type.TypeKind != TypeKind.Struct ) {
35+
return;
36+
}
37+
38+
IMethodSymbol constructor = operation.Constructor;
39+
if( constructor.Parameters.Length > 0 ) {
40+
return;
41+
}
42+
43+
context.ReportDiagnostic( Diagnostic.Create(
44+
Diagnostics.DontCallDefaultStructConstructor,
45+
operation.Syntax.GetLocation()
46+
) );
47+
}
48+
}
49+
}
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+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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+
string IDefaultStructCreationReplacer.Title { get; } = "Use GenerateNew()";
11+
12+
bool IDefaultStructCreationReplacer.CanReplace(
13+
INamedTypeSymbol structType
14+
) {
15+
ImmutableArray<ISymbol> maybeGenerateNew = structType.GetMembers( "GenerateNew" );
16+
if( maybeGenerateNew.Length != 1 ) {
17+
return false;
18+
}
19+
20+
if( !( maybeGenerateNew[0] is IMethodSymbol generateNew ) ) {
21+
return false;
22+
}
23+
24+
if( generateNew.Parameters.Length != 0 ) {
25+
return false;
26+
}
27+
28+
// Sure seems like one of our Guid-backed IdTypes
29+
return true;
30+
}
31+
32+
SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
33+
INamedTypeSymbol structType,
34+
TypeSyntax structName
35+
) {
36+
return SyntaxFactory
37+
.InvocationExpression(
38+
SyntaxFactory.MemberAccessExpression(
39+
SyntaxKind.SimpleMemberAccessExpression,
40+
structName,
41+
SyntaxFactory.IdentifierName( "GenerateNew" )
42+
)
43+
);
44+
}
45+
46+
}
47+
}
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+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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 NewImmutableArrayReplacer : IDefaultStructCreationReplacer {
8+
string IDefaultStructCreationReplacer.Title { get; } = "Use ImmutableArray<>.Empty";
9+
10+
private readonly INamedTypeSymbol m_immutableArrayType;
11+
12+
public NewImmutableArrayReplacer(
13+
Compilation compilation
14+
) {
15+
m_immutableArrayType = compilation
16+
.GetTypeByMetadataName( "System.Collections.Immutable.ImmutableArray`1" );
17+
}
18+
19+
bool IDefaultStructCreationReplacer.CanReplace(
20+
INamedTypeSymbol structType
21+
) {
22+
if( m_immutableArrayType == null ) {
23+
return false;
24+
}
25+
26+
if( m_immutableArrayType.TypeKind == TypeKind.Error ) {
27+
return false;
28+
}
29+
30+
if( m_immutableArrayType != structType.OriginalDefinition ) {
31+
return false;
32+
}
33+
34+
return true;
35+
}
36+
37+
SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
38+
INamedTypeSymbol structType,
39+
TypeSyntax structName
40+
) {
41+
return SyntaxFactory.MemberAccessExpression(
42+
SyntaxKind.SimpleMemberAccessExpression,
43+
structName,
44+
SyntaxFactory.IdentifierName( "Empty" )
45+
);
46+
}
47+
}
48+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// analyzer: D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.DefaultStructCreationAnalyzer
2+
3+
using System;
4+
5+
namespace SpecTests {
6+
7+
public struct StructWithOnlyDefault { }
8+
public struct StructWithAdditional {
9+
public StructWithAdditional( int x ) { }
10+
}
11+
public struct StructWithOverridenDefault {
12+
public StructWithOverridenDefault() { }
13+
}
14+
15+
public class SpecTests {
16+
public void SpecTests() {
17+
18+
{ var x = /* DontCallDefaultStructConstructor() */ new StructWithOnlyDefault() /**/; }
19+
{ var x = /* DontCallDefaultStructConstructor() */ new StructWithAdditional() /**/; }
20+
{ var x = new StructWithAdditional( 1 ); }
21+
{ var x = /* DontCallDefaultStructConstructor() */ new StructWithOverridenDefault() /**/; }
22+
{ var x = /* DontCallDefaultStructConstructor() */ new Guid() /**/; }
23+
{ var x = /* DontCallDefaultStructConstructor() */ new System.Guid() /**/; }
24+
{
25+
// this constructor doesn't actually exist
26+
var x = new StructWithOnlyDefault( 1 );
27+
}
28+
29+
}
30+
}
31+
32+
}

0 commit comments

Comments
 (0)