Skip to content
Open
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

This file was deleted.

20 changes: 10 additions & 10 deletions src/D2L.CodeStyle.Analyzers/Diagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,6 @@ public static class Diagnostics {
description: "ImmutableGeneric can only be applied to closed (fully bound) generic types."
);

public static readonly DiagnosticDescriptor DontUseImmutableArrayConstructor = new DiagnosticDescriptor(
id: "D2L0029",
title: "Don't use the default constructor for ImmutableArray<T>",
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.",
category: "Correctness",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
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."
);

public static readonly DiagnosticDescriptor UnnecessaryMutabilityAnnotation = new DiagnosticDescriptor(
id: "D2L0030",
title: "Unnecessary mutability annotation should be removed to keep the code base clean",
Expand Down Expand Up @@ -505,5 +495,15 @@ public static class Diagnostics {
isEnabledByDefault: true
);

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

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using System.Collections.Immutable;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {
partial class DefaultStructCreationAnalyzer {

[ExportCodeFixProvider(
LanguageNames.CSharp,
Name = nameof( UseSuggestedInitializerCodefix )
)]
public sealed class UseSuggestedInitializerCodefix : CodeFixProvider {

public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor.Id );

public override FixAllProvider GetFixAllProvider()
=> WellKnownFixAllProviders.BatchFixer;

public override async Task RegisterCodeFixesAsync(
CodeFixContext ctx
) {
var root = await ctx.Document
.GetSyntaxRootAsync( ctx.CancellationToken )
.ConfigureAwait( false );

foreach( var diagnostic in ctx.Diagnostics ) {
var span = diagnostic.Location.SourceSpan;

SyntaxNode syntax = root.FindNode( span, getInnermostNodeForTie: true );
if( !( syntax is ObjectCreationExpressionSyntax creationExpression ) ) {
continue;
}

ctx.RegisterCodeFix(
CodeAction.Create(
title: diagnostic.Properties[ACTION_TITLE_KEY],
createChangedDocument: ct => {
SyntaxNode replacement = SyntaxFactory.ParseExpression(
diagnostic.Properties[REPLACEMENT_KEY]
);

SyntaxNode newRoot = root.ReplaceNode( creationExpression, replacement );
Document newDoc = ctx.Document.WithSyntaxRoot( newRoot );

return Task.FromResult( newDoc );
}
),
diagnostic
);
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System.Collections.Immutable;
using D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation {

[DiagnosticAnalyzer( LanguageNames.CSharp )]
internal sealed partial class DefaultStructCreationAnalyzer : DiagnosticAnalyzer {

internal const string ACTION_TITLE_KEY = nameof( DefaultStructCreationAnalyzer ) + ".ActionTitle";
internal const string REPLACEMENT_KEY = nameof( DefaultStructCreationAnalyzer ) + ".Replacement";

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
=> ImmutableArray.Create( Diagnostics.DontCallDefaultStructConstructor );

public override void Initialize( AnalysisContext context ) {
context.EnableConcurrentExecution();

context.ConfigureGeneratedCodeAnalysis(
GeneratedCodeAnalysisFlags.None
);

context.RegisterCompilationStartAction( RegisterAnalyzer );
}

private static void RegisterAnalyzer(
CompilationStartAnalysisContext context
) {
var replacers = ImmutableArray.Create(
NewGuidBackedIdTypeReplacer.Instance,
new NewGuidReplacer( context.Compilation ),
new NewImmutableArrayReplacer( context.Compilation )
);

context.RegisterOperationAction(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to handle = default( T ) at some point

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From IRL: = default( T ) is pretty explicit so maybe don't ban it.

ctx => AnalyzeObjectCreation(
context: ctx,
operation: ctx.Operation as IObjectCreationOperation,
replacers: replacers
),
OperationKind.ObjectCreation
);
}

private static void AnalyzeObjectCreation(
OperationAnalysisContext context,
IObjectCreationOperation operation,
ImmutableArray<IDefaultStructCreationReplacer> replacers
) {
if( operation.Type.TypeKind != TypeKind.Struct ) {
return;
}

IMethodSymbol constructor = operation.Constructor;
if( constructor.Parameters.Length > 0 ) {
return;
}

INamedTypeSymbol structType = operation.Type as INamedTypeSymbol;

foreach( var replacer in replacers ) {

if( !replacer.CanReplace( structType ) ) {
continue;
}

var syntax = operation.Syntax as ObjectCreationExpressionSyntax;
SyntaxNode replacement = replacer
.GetReplacement( structType, syntax.Type )
.WithTriviaFrom( syntax );

context.ReportDiagnostic( Diagnostic.Create(
descriptor: Diagnostics.DontCallDefaultStructConstructor,
location: operation.Syntax.GetLocation(),
properties: ImmutableDictionary<string, string>
.Empty
.Add( ACTION_TITLE_KEY, replacer.Title )
.Add( REPLACEMENT_KEY, replacement.ToFullString() ),
structType.ToDisplayString( SymbolDisplayFormat.MinimallyQualifiedFormat )
) );

break;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {

internal interface IDefaultStructCreationReplacer {

string Title { get; }

bool CanReplace(
INamedTypeSymbol structType
);

SyntaxNode GetReplacement(
INamedTypeSymbol structType,
TypeSyntax structName
);

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {

internal sealed class NewGuidBackedIdTypeReplacer : IDefaultStructCreationReplacer {

public static readonly IDefaultStructCreationReplacer Instance = new NewGuidBackedIdTypeReplacer();

private NewGuidBackedIdTypeReplacer() { }

string IDefaultStructCreationReplacer.Title { get; } = "Use GenerateNew()";

bool IDefaultStructCreationReplacer.CanReplace(
INamedTypeSymbol structType
) {
ImmutableArray<ISymbol> maybeGenerateNew = structType.GetMembers( "GenerateNew" );
if( maybeGenerateNew.Length != 1 ) {
return false;
}

if( !( maybeGenerateNew[0] is IMethodSymbol generateNew ) ) {
return false;
}

if( generateNew.Parameters.Length != 0 ) {
return false;
}

// Sure seems like one of our Guid-backed IdTypes
return true;
}

SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
INamedTypeSymbol structType,
TypeSyntax structName
) {
return SyntaxFactory
.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
structName,
SyntaxFactory.IdentifierName( "GenerateNew" )
)
);
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace D2L.CodeStyle.Analyzers.Language.DefaultStructCreation.Replacements {

internal sealed class NewGuidReplacer : IDefaultStructCreationReplacer {
string IDefaultStructCreationReplacer.Title { get; } = "Use Guid.NewGuid()";

private readonly INamedTypeSymbol m_guidType;

public NewGuidReplacer(
Compilation compilation
) {
m_guidType = compilation.GetTypeByMetadataName( "System.Guid" );
}

bool IDefaultStructCreationReplacer.CanReplace(
INamedTypeSymbol structType
) {
if( m_guidType == null ) {
return false;
}

if( m_guidType.TypeKind == TypeKind.Error ) {
return false;
}

if( m_guidType != structType ) {
return false;
}

return true;
}

SyntaxNode IDefaultStructCreationReplacer.GetReplacement(
INamedTypeSymbol structType,
TypeSyntax structName
) {
return SyntaxFactory
.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
structName,
SyntaxFactory.IdentifierName( "NewGuid" )
)
);
}

}
}
Loading