Skip to content

Commit

Permalink
New rule MA0143: Primary constructor parameters should be readonly (#652
Browse files Browse the repository at this point in the history
)
  • Loading branch information
meziantou authored Nov 25, 2023
1 parent bb33336 commit e1e8e37
Show file tree
Hide file tree
Showing 11 changed files with 311 additions and 5 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ jobs:
- run: dotnet build src/Meziantou.Analyzer/Meziantou.Analyzer.csproj --configuration Release /p:RoslynVersion=roslyn4.2 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer/Meziantou.Analyzer.csproj --configuration Release /p:RoslynVersion=roslyn4.4 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer/Meziantou.Analyzer.csproj --configuration Release /p:RoslynVersion=roslyn4.6 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer/Meziantou.Analyzer.csproj --configuration Release /p:RoslynVersion=roslyn4.8 /p:Version=${{ needs.compute_package_version.outputs.package_version }}

- run: dotnet build src/Meziantou.Analyzer.CodeFixers/Meziantou.Analyzer.CodeFixers.csproj --configuration Release /p:RoslynVersion=roslyn3.8 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer.CodeFixers/Meziantou.Analyzer.CodeFixers.csproj --configuration Release /p:RoslynVersion=roslyn4.2 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer.CodeFixers/Meziantou.Analyzer.CodeFixers.csproj --configuration Release /p:RoslynVersion=roslyn4.4 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer.CodeFixers/Meziantou.Analyzer.CodeFixers.csproj --configuration Release /p:RoslynVersion=roslyn4.6 /p:Version=${{ needs.compute_package_version.outputs.package_version }}
- run: dotnet build src/Meziantou.Analyzer.CodeFixers/Meziantou.Analyzer.CodeFixers.csproj --configuration Release /p:RoslynVersion=roslyn4.8 /p:Version=${{ needs.compute_package_version.outputs.package_version }}

- run: dotnet restore src/Meziantou.Analyzer.pack.csproj
- run: dotnet pack src/Meziantou.Analyzer.pack.csproj --configuration Release --no-build /p:Version=${{ needs.compute_package_version.outputs.package_version }}
Expand Down Expand Up @@ -106,8 +108,8 @@ jobs:
strategy:
matrix:
runs-on: [ ubuntu-latest, windows-latest ]
configuration: [ Debug, Release ]
roslyn-version: [ 'roslyn3.8', 'roslyn4.2', 'roslyn4.4', 'roslyn4.6', 'default' ]
configuration: [ Release ]
roslyn-version: [ 'roslyn3.8', 'roslyn4.2', 'roslyn4.4', 'roslyn4.6', 'roslyn4.8', 'default' ]
fail-fast: false
steps:
- uses: actions/checkout@v4
Expand Down
14 changes: 14 additions & 0 deletions Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@
</ItemGroup>
<PropertyGroup>
<DefineConstants>$(DefineConstants);ROSLYN_4_6;ROSLYN_4_2_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_6_OR_GREATER</DefineConstants>
<DefineConstants>$(DefineConstants);CSHARP9_OR_GREATER;CSHARP10_OR_GREATER;CSHARP11_OR_GREATER</DefineConstants>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>
</When>

<When Condition="$(RoslynVersion) == 'roslyn4.8'">
<ItemGroup>
<PackageReference Update="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp" Version="4.8.0-3.final" />
<PackageReference Update="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.8.0-3.final" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.8.0-3.final" />
</ItemGroup>
<PropertyGroup>
<DefineConstants>$(DefineConstants);ROSLYN_4_8;ROSLYN_4_2_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_6_OR_GREATER;ROSLYN_4_8_OR_GREATER</DefineConstants>
<DefineConstants>$(DefineConstants);CSHARP9_OR_GREATER;CSHARP10_OR_GREATER;CSHARP11_OR_GREATER;CSHARP12_OR_GREATER</DefineConstants>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ If you are already using other analyzers, you can check [which rules are duplica
|[MA0140](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0140.md)|Design|Both if and else branch have identical code|⚠️|✔️||
|[MA0141](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0141.md)|Usage|Use pattern matching instead of inequality operators|ℹ️||✔️|
|[MA0142](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0142.md)|Usage|Use pattern matching instead of equality operators|ℹ️||✔️|
|[MA0143](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0143.md)|Design|Primary constructor parameters should be readonly|⚠️|||

<!-- rules -->

Expand Down
7 changes: 7 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
|[MA0140](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0140.md)|Design|Both if and else branch have identical code|<span title='Warning'>⚠️</span>|✔️||
|[MA0141](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0141.md)|Usage|Use pattern matching instead of inequality operators|<span title='Info'>ℹ️</span>||✔️|
|[MA0142](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0142.md)|Usage|Use pattern matching instead of equality operators|<span title='Info'>ℹ️</span>||✔️|
|[MA0143](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0143.md)|Design|Primary constructor parameters should be readonly|<span title='Warning'>⚠️</span>|||

|Id|Suppressed rule|Justification|
|--|---------------|-------------|
Expand Down Expand Up @@ -574,6 +575,9 @@ dotnet_diagnostic.MA0141.severity = none
# MA0142: Use pattern matching instead of equality operators
dotnet_diagnostic.MA0142.severity = none
# MA0143: Primary constructor parameters should be readonly
dotnet_diagnostic.MA0143.severity = none
```

# .editorconfig - all rules disabled
Expand Down Expand Up @@ -1001,4 +1005,7 @@ dotnet_diagnostic.MA0141.severity = none
# MA0142: Use pattern matching instead of equality operators
dotnet_diagnostic.MA0142.severity = none
# MA0143: Primary constructor parameters should be readonly
dotnet_diagnostic.MA0143.severity = none
```
14 changes: 14 additions & 0 deletions docs/Rules/MA0143.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# MA0143 - Primary constructor parameters should be readonly

The rule reports all assignments to primary constructor parameters

````c#
class Sample(int p)
{
void A()
{
p = 0; // non-compliant
(p, _) = (0, 1); // non-compliant
}
}
````
3 changes: 3 additions & 0 deletions src/Meziantou.Analyzer.pack.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,8 @@

<None Include="$(MSBuildThisFileDirectory)\Meziantou.Analyzer\bin\roslyn4.6\$(Configuration)\netstandard2.0\Meziantou.Analyzer.dll" Pack="true" PackagePath="analyzers/dotnet/roslyn4.6/cs/" />
<None Include="$(MSBuildThisFileDirectory)\Meziantou.Analyzer.CodeFixers\bin\roslyn4.6\$(Configuration)\netstandard2.0\Meziantou.Analyzer.CodeFixers.dll" Pack="true" PackagePath="analyzers/dotnet/roslyn4.6/cs/" />

<None Include="$(MSBuildThisFileDirectory)\Meziantou.Analyzer\bin\roslyn4.8\$(Configuration)\netstandard2.0\Meziantou.Analyzer.dll" Pack="true" PackagePath="analyzers/dotnet/roslyn4.8/cs/" />
<None Include="$(MSBuildThisFileDirectory)\Meziantou.Analyzer.CodeFixers\bin\roslyn4.8\$(Configuration)\netstandard2.0\Meziantou.Analyzer.CodeFixers.dll" Pack="true" PackagePath="analyzers/dotnet/roslyn4.8/cs/" />
</ItemGroup>
</Project>
1 change: 1 addition & 0 deletions src/Meziantou.Analyzer/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ internal static class RuleIdentifiers
public const string IfElseBranchesAreIdentical = "MA0140";
public const string UsePatternMatchingForNullCheck = "MA0141";
public const string UsePatternMatchingForNullEquality = "MA0142";
public const string PrimaryConstructorParameterShouldBeReadOnly = "MA0143";

public static string GetHelpUri(string identifier)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
#if CSHARP12_OR_GREATER
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Meziantou.Analyzer.Rules;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PrimaryConstructorParameterShouldBeReadOnlyAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor s_rule = new(
RuleIdentifiers.PrimaryConstructorParameterShouldBeReadOnly,
title: "Primary constructor parameters should be readonly",
messageFormat: "Primary constructor parameters should be readonly",
RuleCategories.Design,
DiagnosticSeverity.Warning,
isEnabledByDefault: false,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.PrimaryConstructorParameterShouldBeReadOnly));

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

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

context.RegisterCompilationStartAction(context =>
{
if (context.Compilation.GetCSharpLanguageVersion() < Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp12)
return;
context.RegisterOperationAction(AnalyzerAssignment, OperationKind.SimpleAssignment);
context.RegisterOperationAction(AnalyzerAssignment, OperationKind.CompoundAssignment);
context.RegisterOperationAction(AnalyzerAssignment, OperationKind.CoalesceAssignment);
context.RegisterOperationAction(AnalyzerAssignment, OperationKind.DeconstructionAssignment);
context.RegisterOperationAction(AnalyzerIncrementOrDecrement, OperationKind.Increment);
context.RegisterOperationAction(AnalyzerIncrementOrDecrement, OperationKind.Decrement);
});
}

private void AnalyzerIncrementOrDecrement(OperationAnalysisContext context)
{
var operation = (IIncrementOrDecrementOperation)context.Operation;
var target = operation.Target;

if (IsPrimaryConstructorParameter(target, context.CancellationToken))
{
context.ReportDiagnostic(s_rule, target);
}
}

private void AnalyzerAssignment(OperationAnalysisContext context)
{
var operation = (IAssignmentOperation)context.Operation;
var target = operation.Target;
if (target is ITupleOperation)
{
foreach (var innerTarget in GetAllAssignmentTargets(target))
{
if (IsPrimaryConstructorParameter(innerTarget, context.CancellationToken))
{
context.ReportDiagnostic(s_rule, innerTarget);
}
}
}
else if (IsPrimaryConstructorParameter(target, context.CancellationToken))
{
context.ReportDiagnostic(s_rule, target);
}
}

private static List<IOperation> GetAllAssignmentTargets(IOperation operation)
{
var result = new List<IOperation>();
GetAllAssignmentTargets(result, operation);
return result;

static void GetAllAssignmentTargets(List<IOperation> operations, IOperation operation)
{
if (operation is ITupleOperation tuple)
{
foreach (var element in tuple.Elements)
{
GetAllAssignmentTargets(operations, element);
}
}
else
{
operations.Add(operation);
}
}
}

private static bool IsPrimaryConstructorParameter(IOperation operation, CancellationToken cancellationToken)
{
if (operation is IParameterReferenceOperation parameterReferenceOperation)
{
if (parameterReferenceOperation.Parameter.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor } ctor)
{
foreach (var syntaxRef in ctor.DeclaringSyntaxReferences)
{
var syntax = syntaxRef.GetSyntax(cancellationToken);
if (syntax is ClassDeclarationSyntax or StructDeclarationSyntax)
return true;
}
}
}

return false;
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ private async Task<Diagnostic[]> GetSortedDiagnosticsFromDocuments(IList<Diagnos
var compilationWithAnalyzers = compilation.WithAnalyzers(
ImmutableArray.CreateRange(analyzers),
new AnalyzerOptions(additionalFiles, analyzerOptionsProvider));
var diags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(compilationWithAnalyzers.CancellationToken).ConfigureAwait(false);
var diags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(CancellationToken.None).ConfigureAwait(false);
foreach (var diag in diags)
{
#if ROSLYN_3_8
Expand All @@ -329,7 +329,7 @@ private async Task<Diagnostic[]> GetSortedDiagnosticsFromDocuments(IList<Diagnos
for (var i = 0; i < documents.Length; i++)
{
var document = documents[i];
var tree = await document.GetSyntaxTreeAsync(compilationWithAnalyzers.CancellationToken).ConfigureAwait(false);
var tree = await document.GetSyntaxTreeAsync(CancellationToken.None).ConfigureAwait(false);
if (tree == diag.Location.SourceTree)
{
diagnostics.Add(diag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ public class Sample(string id)
"""";
await CreateProjectBuilder()
.WithSourceCode(SourceCode)
.WithLanguageVersion(Microsoft.CodeAnalysis.CSharp.LanguageVersion.Preview)
.WithLanguageVersion(Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp12)
.ValidateAsync();
}
#endif
Expand Down
Loading

0 comments on commit e1e8e37

Please sign in to comment.