Skip to content

Commit

Permalink
Merge pull request #2 from ShaharPrishMSFT/shaharp/defparams
Browse files Browse the repository at this point in the history 
Add fix to record parameters with default values.
  • Loading branch information
@ShaharPrishMSFT
ShaharPrishMSFT authored Jun 30, 2024
2 parents 1ae3cf9 + 1305c1d commit c1ffd66
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 31 deletions.
7 changes: 7 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions ...neering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationAnalyzerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<DiagnosticResult> 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")]
Expand Down
59 changes: 51 additions & 8 deletions ...ineering.Analyzers.Tests/ExhaustiveInitialization/ExhaustiveInitializationCodeFixTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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<DiagnosticResult> 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; }
Expand All @@ -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; }
Expand All @@ -83,7 +121,7 @@ class AllPropsB
List<DiagnosticResult> 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))
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions src/SubtleEngineering.Analyzers/DiagnosticsDetails.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_";
}
}
}
77 changes: 57 additions & 20 deletions src/SubtleEngineering.Analyzers/ExhaustiveInitialization/ExhaustiveInitializationAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<DiagnosticDescriptor> Rules = ImmutableArray.Create(
new DiagnosticDescriptor(
Expand Down Expand Up @@ -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)
);

Expand All @@ -60,6 +68,9 @@ public override void Initialize(AnalysisContext context)

private void AnalyzeSymbol(SymbolAnalysisContext context)
{
bool needToEmitTypeWarning = false;
List<IPropertySymbol> potentiallyBadProperties = new List<IPropertySymbol>();
List<IParameterSymbol> badParameters = new List<IParameterSymbol>();
var namedTypeSymbol = (INamedTypeSymbol)context.Symbol;
if ((namedTypeSymbol.TypeKind == TypeKind.Class || namedTypeSymbol.TypeKind == TypeKind.Struct) &&
namedTypeSymbol.HasAttribute<ExhaustiveInitializationAttribute>())
Expand All @@ -69,9 +80,9 @@ private void AnalyzeSymbol(SymbolAnalysisContext context)
.OfType<IPropertySymbol>()
.Where(x => x.DeclaredAccessibility != Accessibility.Private && x.SetMethod != null && !x.IsStatic && !x.HasAttribute<ExcludeFromExhaustiveAnalysisAttribute>());

var potentiallyBad = allNonePrivateProperties.Where(x => !x.IsRequired).ToList();
potentiallyBadProperties = allNonePrivateProperties.Where(x => !x.IsRequired).ToList();

if (potentiallyBad.Count == 0)
if (potentiallyBadProperties.Count == 0)
{
return;
}
Expand All @@ -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)
Expand All @@ -107,26 +118,25 @@ private void AnalyzeSymbol(SymbolAnalysisContext context)
.OfType<AssignmentExpressionSyntax>()
.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)
{
Expand All @@ -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;
}
}

Expand Down
Loading

0 comments on commit c1ffd66

Please sign in to comment.