-
Notifications
You must be signed in to change notification settings - Fork 465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ConstructorParamShouldMatchPropNames analyzer #4877
Draft
psxvoid
wants to merge
64
commits into
dotnet:main
Choose a base branch
from
psxvoid:feature/33796-ctor-params-match-props
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 59 commits
Commits
Show all changes
64 commits
Select commit
Hold shift + click to select a range
3dedbb2
Add ConstructorParamShouldMatchPropNames analyzer
psxvoid 6dc3ef9
Fix - remove unused package reference
psxvoid 6b71577
Fix json-ctor order in well-known-names
psxvoid 52360bb
Fix code alignment in tests
psxvoid d9308a4
Mark the diagnostic as non-fx-cop-ported
psxvoid 700c4ac
Add missing msbuild autogenerated assets
psxvoid d3769d6
Fix - remove unused package reference, part 2
psxvoid 5c82803
Update rule's message
psxvoid 43ad9b1
Rollback unrelated analyzer to the original state
psxvoid 41d44b2
Update the description message for the analyzer
psxvoid e3db946
Fix misplaced comma and update the message
psxvoid e9bc939
Fix incorrect helper test method name prefix
psxvoid 481f5ed
Update the message title with the recommended one
psxvoid 93c0429
Use net50 references instead netstandard20 in tests
psxvoid 3efee13
Replace IsJsonCtor extension-method with private method
psxvoid 2b49119
Rename the analyzer - add "field" in its name
psxvoid c2051bf
Remove unused using in tests
psxvoid e9aa041
Replace banned-API with non-banned in tests
psxvoid 9579d93
Add a code fix provider, partially tested with C#
psxvoid 5a62547
[POC] Verify tuple assignment is working with CA1071
psxvoid 33dccdd
Revert "[POC] Verify tuple assignment is working with CA1071"
psxvoid 57437ba
Add C# tests for classes with fields and props
psxvoid 727423b
Add VB tests for classes with fields and props
psxvoid 6c83c6e
The analyzer - ensure bound properties are public
psxvoid 2482c4c
Fix minor dis-alignment in tests
psxvoid d4bec8b
Fix tuple assignment reference not detected
psxvoid 9089e0a
Simplify member reference retrieval
psxvoid 5aff394
Merge reference symbol null and static checks
psxvoid 5fba883
Rename local variable in member reference retrival
psxvoid cd373f6
Add tuple assignment test for records
psxvoid 1fb8c27
Analyzer - group tests for records below tests for classes
psxvoid da07af5
Analyzer - sort tests for classes
psxvoid 0c3e2a9
Analyzer - ensure bound fields are public
psxvoid 2309cb0
Fix parameter alignment in analyzer tests
psxvoid 7285658
Analyzer - add tests for private fields with JsonInclude
psxvoid 52dce8e
Analyzer - add tests with JsonPropertyName
psxvoid c603b66
Analyzer - report unreferenced params (no messages)
psxvoid afb760a
Analyzer - add messages for unreferenced parameters
psxvoid 4d30c8f
Analyzer - free unused parameter set on symbol-end
psxvoid 641721d
Analyzer - sort methods by visibility and logically
psxvoid 65ea1ad
Analyzer - fix reversed words match (should not)
psxvoid defb3c2
Analyzer - add regions, cleanup and organize tests
psxvoid d4345d5
Analyzer - simplify return in ShouldAnalyzerMethod
psxvoid 2008be6
Analyzer - simplify supported members matching
psxvoid c0687c6
Add TODO - referenced members are not static diagnostic
psxvoid 313bdfd
Analyzer - format compilation start action registration
psxvoid fa0fd41
Analyzer - merge name match logic into a single method
psxvoid 4d4a1ec
Add "key" postfix to diagnostic parameter keys
psxvoid e059159
Analyzer - remove unused parameter analysis
psxvoid a95f803
Analyzer - init using scoped context variable
psxvoid 1e62744
Analyzer - fix build: invalid whitespace in tests
psxvoid 1e1d382
Analyzer - inline ExportFixProvider and Shared attributes
psxvoid 9a727e4
Revert "Analyzer - inline ExportFixProvider and Shared attributes"
psxvoid 8dacfcb
Fixer - inline ExportFixProvider and Shared attributes (fixed)
psxvoid ab492ca
Analyzer - use single diagnostic rule per analyzer
psxvoid b1deb98
CodeFix - make bound field public (no message, not aligned)
psxvoid 2008bfe
Analyzer - remove unreferenced parameter diagnostic reason
psxvoid 194c3a2
Analyzer - fix broken tests due to additional diagnostic location
psxvoid a487621
CodeFix - add tests for single private property
psxvoid f38390d
CodeFix - add tests for multiple private fields and props
psxvoid 051d067
Analyzer - use "Add" instead of "SetItem"
psxvoid f86bf43
Analyzer - init jsonConstructorAttribute symbol using out var
psxvoid 35b2346
Analyzer - Use ContainingType.Name instead of ToDisplayString
psxvoid aafe0de
Analyzer - Remove parentheses from lambda param while passing context
psxvoid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,5 @@ | ||
; Please do not edit this file manually, it should only be updated through code fix application. | ||
### New Rules | ||
Rule ID | Category | Severity | Notes | ||
--------|----------|----------|------- | ||
CA1071 | Design | Warning | ConstructorParametersShouldMatchPropertyNamesAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1071) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
....NetCore.Analyzers/Runtime/ConstructorParametersShouldMatchPropertyAndFieldNames.Fixer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.Globalization; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CodeActions; | ||
using Microsoft.CodeAnalysis.CodeFixes; | ||
using Microsoft.CodeAnalysis.Editing; | ||
using Microsoft.CodeAnalysis.Rename; | ||
using Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines; | ||
using static Microsoft.NetCore.Analyzers.Runtime.ConstructorParametersShouldMatchPropertyAndFieldNamesAnalyzer; | ||
|
||
namespace Microsoft.NetCore.Analyzers.Runtime | ||
{ | ||
/// <summary> | ||
/// CA1071: Constructor parameters should match property and field names. | ||
/// Based on <see cref="ParameterNamesShouldMatchBaseDeclarationFixer"/>. | ||
/// </summary> | ||
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared] | ||
public sealed class ConstructorParametersShouldMatchPropertyAndFieldNamesFixer : CodeFixProvider | ||
{ | ||
private static readonly Type DiagnosticReasonEnumType = typeof(ParameterDiagnosticReason); | ||
|
||
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(RuleId); | ||
|
||
public sealed override FixAllProvider GetFixAllProvider() | ||
{ | ||
// See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md for more information on Fix All Providers | ||
return WellKnownFixAllProviders.BatchFixer; | ||
} | ||
|
||
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) | ||
{ | ||
SyntaxNode syntaxRoot = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); | ||
SemanticModel semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); | ||
|
||
foreach (Diagnostic diagnostic in context.Diagnostics) | ||
{ | ||
SyntaxNode node = syntaxRoot.FindNode(context.Span); | ||
ISymbol declaredSymbol = semanticModel.GetDeclaredSymbol(node, context.CancellationToken); | ||
|
||
if (declaredSymbol.Kind != SymbolKind.Parameter) | ||
{ | ||
continue; | ||
} | ||
|
||
var diagnosticReason = (ParameterDiagnosticReason)Enum.Parse(DiagnosticReasonEnumType, diagnostic.Properties[DiagnosticReasonKey]); | ||
|
||
switch (diagnosticReason) | ||
{ | ||
case ParameterDiagnosticReason.NameMismatch: | ||
RegisterParameterRenameCodeFix(context, diagnostic, declaredSymbol); | ||
break; | ||
case ParameterDiagnosticReason.FieldInappropriateVisibility: | ||
case ParameterDiagnosticReason.PropertyInappropriateVisibility: | ||
SyntaxNode fieldOrProperty = syntaxRoot.FindNode(diagnostic.AdditionalLocations[0].SourceSpan); | ||
RegisterMakeFieldOrPropertyPublicCodeFix(context, diagnostic, fieldOrProperty); | ||
break; | ||
default: | ||
throw new InvalidOperationException(); | ||
}; | ||
} | ||
} | ||
|
||
private static void RegisterParameterRenameCodeFix(CodeFixContext context, Diagnostic diagnostic, ISymbol declaredSymbol) | ||
{ | ||
// This approach is very naive. Most likely we want to support NamingStyleOptions, available in Roslyn. | ||
string newName = LowerFirstLetter(diagnostic.Properties[ReferencedFieldOrPropertyNameKey]); | ||
|
||
context.RegisterCodeFix( | ||
CodeAction.Create( | ||
string.Format(CultureInfo.CurrentCulture, MicrosoftNetCoreAnalyzersResources.ConstructorParametersShouldMatchPropertyOrFieldNamesTitle, newName), | ||
cancellationToken => GetUpdatedDocumentForParameterRenameAsync(context.Document, declaredSymbol, newName, cancellationToken), | ||
nameof(ConstructorParametersShouldMatchPropertyAndFieldNamesFixer)), | ||
diagnostic); | ||
} | ||
|
||
private static void RegisterMakeFieldOrPropertyPublicCodeFix(CodeFixContext context, Diagnostic diagnostic, SyntaxNode fieldOrProperty) | ||
{ | ||
context.RegisterCodeFix( | ||
CodeAction.Create( | ||
string.Format(CultureInfo.CurrentCulture, MicrosoftNetCoreAnalyzersResources.ConstructorParametersShouldMatchPropertyOrFieldNamesTitle, fieldOrProperty), | ||
cancellationToken => GetUpdatedDocumentForMakingFieldOrPropertyPublicAsync(context.Document, fieldOrProperty, cancellationToken), | ||
nameof(ConstructorParametersShouldMatchPropertyAndFieldNamesFixer)), | ||
diagnostic); | ||
} | ||
|
||
private static string LowerFirstLetter(string targetName) | ||
{ | ||
return $"{targetName[0].ToString().ToLower(CultureInfo.CurrentCulture)}{targetName[1..]}"; | ||
} | ||
|
||
private static async Task<Document> GetUpdatedDocumentForParameterRenameAsync(Document document, ISymbol parameter, string newName, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it duplicates a method of |
||
{ | ||
Solution newSolution = await Renamer.RenameSymbolAsync(document.Project.Solution, parameter, newName, null, cancellationToken).ConfigureAwait(false); | ||
return newSolution.GetDocument(document.Id)!; | ||
} | ||
|
||
private static async Task<Document> GetUpdatedDocumentForMakingFieldOrPropertyPublicAsync(Document document, SyntaxNode fieldOrProperty, CancellationToken cancellationToken) | ||
{ | ||
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); | ||
|
||
editor.SetAccessibility(fieldOrProperty, Accessibility.Public); | ||
|
||
return editor.GetChangedDocument(); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To Do: remove