From 0fe5c986338fb41d5adc86455fec382323283ebd Mon Sep 17 00:00:00 2001 From: Shahar Prish Date: Sun, 30 Jun 2024 10:13:54 +0300 Subject: [PATCH 1/3] Add fix to parameters. --- .../ExhaustiveInitializationAnalyzerTests.cs | 32 ++++++++ .../ExhaustiveInitializationCodeFixTests.cs | 59 ++++++++++++-- .../DiagnosticsDetails.cs | 3 + .../ExhaustiveInitializationAnalyzer.cs | 77 ++++++++++++++----- .../ExhaustiveInitializationCodeFix.cs | 60 ++++++++++++++- 5 files changed, 201 insertions(+), 30 deletions(-) diff --git a/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationAnalyzerTests.cs b/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationAnalyzerTests.cs index 3472ebd..548af98 100644 --- a/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationAnalyzerTests.cs +++ b/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationAnalyzerTests.cs @@ -207,6 +207,38 @@ namespace A await sut.RunAsync(); } + [Theory] + [InlineData("record")] + [InlineData("record struct")] + public async Task RecordWithDefaultPrimaryCtorValuesFails(string type) + { + string code = $$""" + namespace A + { + [SubtleEngineering.Analyzers.Decorators.ExhaustiveInitialization] + {{type}} AllPropsB(string MyString = "xxx") + { + } + } + """; + + var columnDelta = type.Length - "{{type}}".Length; + List expected = + [ + VerifyAn.Diagnostic( + ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId)) + .WithLocation(4, 14 + columnDelta) + .WithArguments("A.AllPropsB"), + VerifyAn.Diagnostic( + ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId)) + .WithLocation(4, 24 + columnDelta) + .WithArguments("A.AllPropsB", "MyString"), + ]; + + var sut = CreateSut(code, expected); + await sut.RunAsync(); + } + [Theory] [InlineData("class")] [InlineData("struct")] diff --git a/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationCodeFixTests.cs b/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationCodeFixTests.cs index 8f0d6ed..a3d7cf0 100644 --- a/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationCodeFixTests.cs +++ b/src/SubtleEngineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationCodeFixTests.cs @@ -6,6 +6,7 @@ namespace SubtleEngineering.Analyzers.Tests.ExhaustiveInitialization; using SubtleEngineering.Analyzers.Decorators; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; +using Microsoft; public class ExhaustiveInitializationCodeFixTests { @@ -52,14 +53,51 @@ class AllPropsB await sut.RunAsync(); } - [Fact(Skip = "In UTs modifies one of the props with two required")] + [Fact] + public async Task RemoveDefaultValue() + { + string code = $$""" + namespace A + { + [SubtleEngineering.Analyzers.Decorators.ExhaustiveInitialization] + record AllPropsB(int MyInt = 0) + { + } + } + """; + + const string fixedCode = """ + namespace A + { + [SubtleEngineering.Analyzers.Decorators.ExhaustiveInitialization] + record AllPropsB(int MyInt ) + { + } + } + """; + + List expected = [ + VerifyCf.Diagnostic( + ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId)) + .WithLocation(4, 12) + .WithArguments("A.AllPropsB"), + VerifyCf.Diagnostic( + ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId)) + .WithLocation(4, 22) + .WithArguments("A.AllPropsB", "MyInt"), + ]; + var sut = CreateSut(code, fixedCode, DiagnosticsDetails.ExhaustiveInitialization.ParameterEquivalenceKey, expected); + await sut.RunAsync(); + } + + [Fact(Skip = "For some reason emits two required in the fix")] public async Task FixEntireType() { string code = $$""" namespace A { [SubtleEngineering.Analyzers.Decorators.ExhaustiveInitialization] - class AllPropsB + record AllPropsB(double MyDouble = 1.0) { public int MyInt { get; set; } public string MyString { get; set; } @@ -72,7 +110,7 @@ class AllPropsB namespace A { [SubtleEngineering.Analyzers.Decorators.ExhaustiveInitialization] - class AllPropsB + record AllPropsB(double MyDouble ) { required public int MyInt { get; set; } required required public string MyString { get; set; } @@ -83,7 +121,7 @@ class AllPropsB List expected = [ VerifyCf.Diagnostic( ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId)) - .WithLocation(4, 11) + .WithLocation(4, 12) .WithArguments("A.AllPropsB"), VerifyCf.Diagnostic( ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.PropertyIsMissingRequiredId)) @@ -93,6 +131,10 @@ class AllPropsB ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.PropertyIsMissingRequiredId)) .WithLocation(7, 23) .WithArguments("A.AllPropsB", "MyString"), + VerifyCf.Diagnostic( + ExhaustiveInitializationAnalyzer.Rules.Find(DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId)) + .WithLocation(4, 22) + .WithArguments("A.AllPropsB", "MyDouble"), ]; var sut = CreateSut(code, fixedCode, DiagnosticsDetails.ExhaustiveInitialization.TypeEquivalenceKey, expected); await sut.RunAsync(); @@ -163,12 +205,13 @@ private VerifyCf.Test CreateSut(string code, string fixedCode, string equivalenc }, }; + test.CodeActionEquivalenceKey = equivalenceKey; test.ExpectedDiagnostics.AddRange(expected); - test.NumberOfFixAllIterations = 1; - test.NumberOfFixAllInDocumentIterations = 1; - test.NumberOfFixAllInProjectIterations = 1; - test.CodeFixTestBehaviors = CodeFixTestBehaviors.FixOne; + //test.NumberOfFixAllIterations = 1; + //test.NumberOfFixAllInDocumentIterations = 1; + //test.NumberOfFixAllInProjectIterations = 1; + //test.CodeFixTestBehaviors = CodeFixTestBehaviors.FixOne; return test; } diff --git a/src/SubtleEngineering.Analyzers/DiagnosticsDetails.cs b/src/SubtleEngineering.Analyzers/DiagnosticsDetails.cs index 2977969..e902bd0 100644 --- a/src/SubtleEngineering.Analyzers/DiagnosticsDetails.cs +++ b/src/SubtleEngineering.Analyzers/DiagnosticsDetails.cs @@ -27,13 +27,16 @@ public class ExhaustiveInitialization public const string PropertyIsMissingRequiredId = "SE1031"; public const string OnlyOneConstructorAllowedId = "SE1032"; public const string NotAllowedOnTypeId = "SE1033"; + public const string PrimaryDefaultConstructorValuesNotAllowedId = "SE1034"; public const string PrimaryCtorOnNonRecordReason = "primary constructors on classes and structs cannot be analyzed for assignment to non-required properties. Use a regular constructor if you need to initialize properties that are not set as required."; public const string PropertyEquivalenceKey = "Property"; public const string TypeEquivalenceKey = "Property"; + public const string ParameterEquivalenceKey = "Parameter"; public const string BadPropertyPrefix = "BadProperty_"; + public const string BadParameterPrefix = "BadParameter_"; } } } diff --git a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationAnalyzer.cs b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationAnalyzer.cs index 31df3aa..79de692 100644 --- a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationAnalyzer.cs +++ b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationAnalyzer.cs @@ -16,6 +16,7 @@ public class ExhaustiveInitializationAnalyzer : DiagnosticAnalyzer private const int SE1031 = 1; private const int SE1032 = 2; private const int SE1033 = 3; + private const int SE1034 = 4; public static readonly ImmutableArray Rules = ImmutableArray.Create( new DiagnosticDescriptor( @@ -45,6 +46,13 @@ public class ExhaustiveInitializationAnalyzer : DiagnosticAnalyzer "Type '{0}' cannot have the ExhaustiveInitialization attribute applied to it because {1}.", "Usage", DiagnosticSeverity.Warning, + isEnabledByDefault: true), + new DiagnosticDescriptor( + DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId, + "Record with default values in primary constructor", + "Record type '{0}' constructor parameter/property {1} has a default initialization value", + "Usage", + DiagnosticSeverity.Warning, isEnabledByDefault: true) ); @@ -60,6 +68,9 @@ public override void Initialize(AnalysisContext context) private void AnalyzeSymbol(SymbolAnalysisContext context) { + bool needToEmitTypeWarning = false; + List potentiallyBadProperties = new List(); + List badParameters = new List(); var namedTypeSymbol = (INamedTypeSymbol)context.Symbol; if ((namedTypeSymbol.TypeKind == TypeKind.Class || namedTypeSymbol.TypeKind == TypeKind.Struct) && namedTypeSymbol.HasAttribute()) @@ -69,9 +80,9 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) .OfType() .Where(x => x.DeclaredAccessibility != Accessibility.Private && x.SetMethod != null && !x.IsStatic && !x.HasAttribute()); - var potentiallyBad = allNonePrivateProperties.Where(x => !x.IsRequired).ToList(); + potentiallyBadProperties = allNonePrivateProperties.Where(x => !x.IsRequired).ToList(); - if (potentiallyBad.Count == 0) + if (potentiallyBadProperties.Count == 0) { return; } @@ -84,16 +95,16 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) if (constructors.Count() > 1) { var diagnostic = Diagnostic.Create(Rules[SE1032], namedTypeSymbol.Locations[0], namedTypeSymbol.ToDisplayString()); - context.ReportDiagnostic(diagnostic); + ReportDiagnostic(diagnostic); return; } var constructor = constructors.SingleOrDefault(); - + ConstructorDeclarationSyntax constructorSyntax = null; if (constructor != null) { - var constructorSyntax = constructor + constructorSyntax = constructor .DeclaringSyntaxReferences .FirstOrDefault() ?.GetSyntax(context.CancellationToken) @@ -107,26 +118,25 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) .OfType() .Select(x => x.Left.DescendantNodesAndSelf().FirstOrDefault(statement => statement is IdentifierNameSyntax) as IdentifierNameSyntax) .Where(x => x != null) - .Select(x => potentiallyBad.FirstOrDefault(prop => x.IsPropertyIdentifier(prop))) + .Select(x => potentiallyBadProperties.FirstOrDefault(prop => x.IsPropertyIdentifier(prop))) .Where(x => x != null) .ToList(); - potentiallyBad.RemoveAll(x => assignedProperties.Contains(x, SymbolEqualityComparer.Default)); + potentiallyBadProperties.RemoveAll(x => assignedProperties.Contains(x, SymbolEqualityComparer.Default)); } else if (constructorSyntax == null && !namedTypeSymbol.IsRecord) { var typeDiagnostic = Diagnostic.Create(Rules[SE1033], namedTypeSymbol.Locations[0], namedTypeSymbol.ToDisplayString(), DiagnosticsDetails.ExhaustiveInitialization.PrimaryCtorOnNonRecordReason); - context.ReportDiagnostic(typeDiagnostic); + ReportDiagnostic(typeDiagnostic); } else if (namedTypeSymbol.IsRecord) { - potentiallyBad.RemoveAll(x => HasMatchingParameterName(constructor, x)); + potentiallyBadProperties.RemoveAll(x => HasMatchingParameterName(constructor, x)); } } - bool emittedTypeError = false; - foreach (var property in potentiallyBad) + foreach (var property in potentiallyBadProperties) { if (property.DeclaringSyntaxReferences.Length > 0) { @@ -135,19 +145,46 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) if (!property.IsRequired) { - if (!emittedTypeError) - { - emittedTypeError = true; - var props = potentiallyBad.Select(x => x.Name).ToImmutableDictionary(x => $"{DiagnosticsDetails.ExhaustiveInitialization.BadPropertyPrefix}_{x}", x => x); - var typeDiagnostic = Diagnostic.Create(Rules[SE1030], namedTypeSymbol.Locations[0], props, namedTypeSymbol.ToDisplayString()); - context.ReportDiagnostic(typeDiagnostic); - } - var diagnostic = Diagnostic.Create(Rules[SE1031], property.Locations[0], namedTypeSymbol.ToDisplayString(), property.Name); - context.ReportDiagnostic(diagnostic); + ReportDiagnostic(diagnostic); } } } + + // If this is a record, check for default values in the primary constructor + if (namedTypeSymbol.IsRecord && constructor != null && constructorSyntax == null) + { + var defaultValues = constructor + .Parameters + .Where(x => x.HasExplicitDefaultValue) + .ToList(); + + foreach (var param in defaultValues) + { + var syntaxReference = param.DeclaringSyntaxReferences[0]; + var syntax = syntaxReference.GetSyntax(); + var diagnostic = Diagnostic.Create(Rules[SE1034], syntax.GetLocation(), namedTypeSymbol.ToDisplayString(), param.Name); + badParameters.Add(param); + ReportDiagnostic(diagnostic); + } + } + } + + if (needToEmitTypeWarning) + { + var props = potentiallyBadProperties + .Select(x => (DiagnosticsDetails.ExhaustiveInitialization.BadPropertyPrefix, x.Name)) + .Concat(badParameters.Select(x => (DiagnosticsDetails.ExhaustiveInitialization.BadParameterPrefix, x.Name))) + .ToImmutableDictionary(x => $"{x.Item1}_{x.Item2}", x => x.Item2); + + var typeDiagnostic = Diagnostic.Create(Rules[SE1030], namedTypeSymbol.Locations[0], props, namedTypeSymbol.ToDisplayString()); + context.ReportDiagnostic(typeDiagnostic); + } + + void ReportDiagnostic(Diagnostic diagnostic) + { + context.ReportDiagnostic(diagnostic); + needToEmitTypeWarning = true; } } diff --git a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs index 037ac7b..d2ddffe 100644 --- a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs +++ b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs @@ -17,7 +17,10 @@ [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ExhaustiveInitializationCodeFix)), Shared] public class ExhaustiveInitializationCodeFix : CodeFixProvider { - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticsDetails.ExhaustiveInitialization.PropertyIsMissingRequiredId, DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId); + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create( + DiagnosticsDetails.ExhaustiveInitialization.PropertyIsMissingRequiredId, + DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId, + DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId); public override FixAllProvider GetFixAllProvider() { @@ -27,7 +30,7 @@ public override FixAllProvider GetFixAllProvider() public override async Task RegisterCodeFixesAsync(CodeFixContext context) { - foreach (var diagnostic in context.Diagnostics.Where(x => x.Id == DiagnosticsDetails.ExhaustiveInitialization.PropertyIsMissingRequiredId || x.Id == DiagnosticsDetails.ExhaustiveInitialization.TypeInitializationIsNonExhaustiveId)) + foreach (var diagnostic in context.Diagnostics.Where(x => x.Id.StartsWith("SE"))) { var diagnosticSpan = diagnostic.Location.SourceSpan; @@ -58,9 +61,41 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) equivalenceKey: DiagnosticsDetails.ExhaustiveInitialization.TypeEquivalenceKey), diagnostic); } + else if (diagnostic.Id == DiagnosticsDetails.ExhaustiveInitialization.PrimaryDefaultConstructorValuesNotAllowedId) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var paramSyntax = root.FindNode(diagnosticSpan).FirstAncestorOrSelf(); + + // Register a code action that will invoke the fix. + context.RegisterCodeFix( + CodeAction.Create( + title: "Make type exhaustive", + createChangedDocument: c => FixParameter(context, paramSyntax, diagnostic, c), + equivalenceKey: DiagnosticsDetails.ExhaustiveInitialization.ParameterEquivalenceKey), + diagnostic); + } } } + private async Task FixParameter(CodeFixContext context, ParameterSyntax paramSyntax, Diagnostic diagnostic, CancellationToken ct) + { + var document = context.Document; + var semanticModel = await document.GetSemanticModelAsync(ct).ConfigureAwait(false); + + // Add the 'required' modifier to the member declaration + var newParamSyntax = GetFixedParameter(paramSyntax); + + var root = await document.GetSyntaxRootAsync(ct).ConfigureAwait(false); + var newRoot = root.ReplaceNode(paramSyntax, newParamSyntax); + + return document.WithSyntaxRoot(newRoot); + } + + private ParameterSyntax GetFixedParameter(ParameterSyntax paramSyntax) + { + return paramSyntax.WithDefault(null); + } + private async Task FixProperty(CodeFixContext context, MemberDeclarationSyntax memberDeclarationSyntax, Diagnostic diagnostic, CancellationToken ct) { var document = context.Document; @@ -107,6 +142,27 @@ private async Task FixEntireType(CodeFixContext context, TypeDeclarati var newMember = GetFixedProperty(member); editor.ReplaceNode(member, newMember); } + + // Find all relevant properties. + var parametersToFix = diagnostic + .Properties + .Where(x => x.Key.StartsWith(DiagnosticsDetails.ExhaustiveInitialization.BadParameterPrefix)) + .Select(x => x.Value) + .ToArray(); + + if (typeSyntax is RecordDeclarationSyntax recordDeclaration) + { + var constructorParameters = recordDeclaration + .ParameterList + .Parameters + .ToArray() ?? Array.Empty(); + + foreach (var prop in constructorParameters) + { + var newMember = GetFixedParameter(prop); + editor.ReplaceNode(prop, newMember); + } + } var newRoot = editor.GetChangedRoot(); var newDocument = document.WithSyntaxRoot(newRoot); From 481c4381dc48ad83abb2e7c3dda7c24b4c6cef93 Mon Sep 17 00:00:00 2001 From: Shahar Prish Date: Sun, 30 Jun 2024 10:17:53 +0300 Subject: [PATCH 2/3] Update readme. --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index 43645c6..67ec97b 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,13 @@ public class MyClass **How it helps engineering**: See SE1030 +#### SE1034: PrimaryDefaultConstructorValuesNotAllowed + +**What it looks for**: Primary constructors are not allowed to have default values on implicit constructor properties. + +**Why**: Default arguments on the primary constructor are not allowed on exhauste initialization types. + +**How it helps engineering**: See SE1030 #### Code fix available From 1305c1d45f88ff82b354461847ac84841f4235cc Mon Sep 17 00:00:00 2001 From: Shahar Prish Date: Sun, 30 Jun 2024 10:21:00 +0300 Subject: [PATCH 3/3] Fix title.s --- .../ExhaustiveInitializationCodeFix.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs index d2ddffe..5920b5b 100644 --- a/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs +++ b/src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationCodeFix.cs @@ -56,7 +56,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) // Register a code action that will invoke the fix. context.RegisterCodeFix( CodeAction.Create( - title: "Make all properties required", + title: "Make type exhaustive", createChangedDocument: c => FixEntireType(context, typeSyntax, diagnostic, c), equivalenceKey: DiagnosticsDetails.ExhaustiveInitialization.TypeEquivalenceKey), diagnostic); @@ -69,7 +69,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) // Register a code action that will invoke the fix. context.RegisterCodeFix( CodeAction.Create( - title: "Make type exhaustive", + title: "Remove default parameter/proprerty value", createChangedDocument: c => FixParameter(context, paramSyntax, diagnostic, c), equivalenceKey: DiagnosticsDetails.ExhaustiveInitialization.ParameterEquivalenceKey), diagnostic);