Add ConstructorParamShouldMatchPropNames analyzer

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -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)

src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/Maintainability/ReviewUnusedParameters.cs

@@ -196,6 +196,7 @@ private bool ShouldAnalyzeMethod(
             }
 
             // Ignore primary constructor (body-less) of positional records.
+            // TODO: I have to handle a similar situation
             if (IsPositionalRecordPrimaryConstructor(method))
             {
                 return false;

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--
+    Microsoft ResX Schema
+
     Version 2.0
-    
-    The primary goals of this format is to allow a simple XML format 
-    that is mostly human readable. The generation and parsing of the 
-    various data types are done through the TypeConverter classes 
+
+    The primary goals of this format is to allow a simple XML format
+    that is mostly human readable. The generation and parsing of the
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
-    There are any number of "resheader" rows that contain simple 
+
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
-    Each data row contains a name, and value. The row also contains a 
-    type or mimetype. Type corresponds to a .NET class that support 
-    text/value conversion through the TypeConverter architecture. 
-    Classes that don't support this are serialized and stored with the 
+
+    Each data row contains a name, and value. The row also contains a
+    type or mimetype. Type corresponds to a .NET class that support
+    text/value conversion through the TypeConverter architecture.
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
-    The mimetype is used for serialized objects, and tells the 
-    ResXResourceReader how to depersist the object. This is currently not 
+
+    The mimetype is used for serialized objects, and tells the
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
-    that the ResXResourceWriter will generate, however the reader can 
+
+    Note - application/x-microsoft.net.object.binary.base64 is the format
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -1519,4 +1519,13 @@
     <value>and all other platforms</value>
     <comment>This call site is reachable on: 'windows' 10.0.2000 and later, and all other platforms</comment>
   </data>
+  <data name="ConstructorParameterShouldMatchPropertyOrFieldName" xml:space="preserve">
+    <value>For proper deserialization, change the name of parameter '{1}' in the constructor of '{0}' to match the bound property or field '{2}'</value>
+  </data>
+  <data name="ConstructorParametersShouldMatchPropertyOrFieldNamesDescription" xml:space="preserve">
+    <value>For a constructor having [JsonConstructor] attribute each parameter name must match with a public property or field name for proper deserialization.</value>
+  </data>
+  <data name="ConstructorParametersShouldMatchPropertyOrFieldNamesTitle" xml:space="preserve">
+    <value>Constructor parameter names should match the bound property or field names</value>
+  </data>
 </root>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ConstructorParametersShouldMatchPropertyAndFieldNames.Fixer.cs

@@ -0,0 +1,69 @@
+// 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.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.Rename;
+using Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines;
+
+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
+    {
+        public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(ConstructorParametersShouldMatchPropertyAndFieldNamesAnalyzer.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;
+                }
+
+                // This approach is very naive. Most likely we want to support NamingStyleOptions, available in Roslyn.
+                string newName = LowerFirstLetter(diagnostic.Properties[ConstructorParametersShouldMatchPropertyAndFieldNamesAnalyzer.ReferencedFieldOrPropertyNameKey]);
+
+                context.RegisterCodeFix(
+                    CodeAction.Create(
+                        string.Format(CultureInfo.CurrentCulture, MicrosoftNetCoreAnalyzersResources.ConstructorParametersShouldMatchPropertyOrFieldNamesTitle, newName),
+                        cancellationToken => GetUpdatedDocumentForParameterRenameAsync(context.Document, declaredSymbol, newName, 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)
+        {
+            Solution newSolution = await Renamer.RenameSymbolAsync(document.Project.Solution, parameter, newName, null, cancellationToken).ConfigureAwait(false);
+            return newSolution.GetDocument(document.Id)!;
+        }
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ConstructorParametersShouldMatchPropertyAndFieldNames.cs

@@ -0,0 +1,213 @@
+// 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.Diagnostics.CodeAnalysis;
+using System.Linq;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.NetCore.Analyzers.Runtime
+{
+    /// <summary>
+    /// CA1071: Constructor parameters should match property and field names
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+    public sealed class ConstructorParametersShouldMatchPropertyAndFieldNamesAnalyzer : DiagnosticAnalyzer
+    {
+        internal const string RuleId = "CA1071";
+
+        internal const string ReferencedFieldOrPropertyNameKey = "ReferencedPropertyOrFieldName";
+        internal const string DiagnosticReasonKey = "DiagnosticReason";
+
+        private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.ConstructorParametersShouldMatchPropertyOrFieldNamesTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
+
+        private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.ConstructorParameterShouldMatchPropertyOrFieldName), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
+        private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.ConstructorParametersShouldMatchPropertyOrFieldNamesDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
+
+        internal static DiagnosticDescriptor PropertyOrFieldNameRule = DiagnosticDescriptorHelper.Create(RuleId,
+                                                                             s_localizableTitle,
+                                                                             s_localizableMessage,
+                                                                             DiagnosticCategory.Design,
+                                                                             RuleLevel.BuildWarning,
+                                                                             description: s_localizableDescription,
+                                                                             isPortedFxCopRule: false,
+                                                                             isDataflowRule: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(PropertyOrFieldNameRule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.EnableConcurrentExecution();
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+            context.RegisterCompilationStartAction((context) =>
+            {
+                INamedTypeSymbol? jsonConstructorAttributeNamedSymbol = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextJsonSerializationJsonConstructorAttribute);
+                if (jsonConstructorAttributeNamedSymbol == null)
+                {
+                    return;
+                }
+
+                var paramAnalyzer = new ParameterAnalyzer(jsonConstructorAttributeNamedSymbol);
+
+                context.RegisterSymbolStartAction((context) =>
+                {
+                    var constructors = ((INamedTypeSymbol)context.Symbol).InstanceConstructors;
+
+                    foreach (var ctor in constructors)
+                    {
+                        if (paramAnalyzer.ShouldAnalyzeMethod(ctor))
+                        {
+                            context.RegisterOperationAction(
+                                context => ParameterAnalyzer.AnalyzeOperationAndReport(context),
+                                OperationKind.ParameterReference);
+                        }
+                    }
+                }, SymbolKind.NamedType);
+            });
+        }
+
+        internal enum ParameterDiagnosticReason
+        {
+            NameMismatch,
+            PropertyInappropriateVisibility,
+            FieldInappropriateVisibility,
+            UnreferencedParameter,
+        }
+
+        private sealed class ParameterAnalyzer
+        {
+            private readonly INamedTypeSymbol _jsonConstructorAttributeInfoType;
+
+            public ParameterAnalyzer(INamedTypeSymbol jsonConstructorAttributeInfoType)
+            {
+                _jsonConstructorAttributeInfoType = jsonConstructorAttributeInfoType;
+            }
+
+            public static void AnalyzeOperationAndReport(OperationAnalysisContext context)
+            {
+                var operation = (IParameterReferenceOperation)context.Operation;
+
+                IMemberReferenceOperation? memberReferenceOperation = TryGetMemberReferenceOperation(operation);
+                ISymbol? referencedSymbol = memberReferenceOperation?.GetReferencedMemberOrLocalOrParameter();
+
+                // TODO: convert "IsStatic" to a separate diagnostic
+                if (referencedSymbol == null || referencedSymbol.IsStatic)
+                {
+                    return;
+                }
+
+                IParameterSymbol param = operation.Parameter;
+
+                if (referencedSymbol is IFieldSymbol field)
+                {
+                    if (!IsParamMatchesReferencedMemberName(param, field))
+                    {
+                        ReportFieldDiagnostic(context, PropertyOrFieldNameRule, ParameterDiagnosticReason.NameMismatch, param, field);
+                    }
+
+                    if (!field.IsPublic())
+                    {
+                        ReportFieldDiagnostic(context, PropertyOrFieldNameRule, ParameterDiagnosticReason.FieldInappropriateVisibility, param, field);
+                    }
+                }
+                else if (referencedSymbol is IPropertySymbol prop)
+                {
+                    if (!IsParamMatchesReferencedMemberName(param, prop))
+                    {
+                        ReportPropertyDiagnostic(context, PropertyOrFieldNameRule, ParameterDiagnosticReason.NameMismatch, param, prop);
+                    }
+
+                    if (!prop.IsPublic())
+                    {
+                        ReportPropertyDiagnostic(context, PropertyOrFieldNameRule, ParameterDiagnosticReason.PropertyInappropriateVisibility, param, prop);
+                    }
+                }
+            }
+
+            public bool ShouldAnalyzeMethod(IMethodSymbol method)
+            {
+                // We only care about constructors with parameters.
+                if (method.Parameters.IsEmpty)
+                {
+                    return false;
+                }
+
+                // We only care about constructors that are marked with JsonConstructor attribute.
+                return this.IsJsonConstructor(method);
+            }
+
+            private static bool IsParamMatchesReferencedMemberName(IParameterSymbol param, ISymbol referencedMember)
+            {
+                if (param.Name.Length != referencedMember.Name.Length)
+                {
+                    return false;
+                }
+
+                var paramWords = WordParser.Parse(param.Name, WordParserOptions.SplitCompoundWords);
+                var memberWords = WordParser.Parse(referencedMember.Name, WordParserOptions.SplitCompoundWords);
+
+                return paramWords.SequenceEqual(memberWords, StringComparer.OrdinalIgnoreCase);
+            }
+
+            private bool IsJsonConstructor([NotNullWhen(returnValue: true)] IMethodSymbol? method)
+                => method.IsConstructor() &&
+                    method.HasAttribute(this._jsonConstructorAttributeInfoType);
+
+            private static IMemberReferenceOperation? TryGetMemberReferenceOperation(IParameterReferenceOperation paramOperation)
+            {
+                if (paramOperation.Parent is IAssignmentOperation assignmentOperation
+                    && assignmentOperation.Target is IMemberReferenceOperation assignmentTarget)
+                {
+                    return assignmentTarget;
+                }
+
+                if (paramOperation.Parent is ITupleOperation sourceTuple
+                    && sourceTuple.Parent is IConversionOperation conversion
+                    && conversion.Parent is IDeconstructionAssignmentOperation deconstruction
+                    && deconstruction.Target is ITupleOperation targetTuple)
+                {
+                    var paramIndexInTuple = sourceTuple.Elements.IndexOf(paramOperation);
+
+                    return targetTuple.Elements[paramIndexInTuple] as IMemberReferenceOperation;
+                }
+
+                return null;
+            }
+
+            private static void ReportFieldDiagnostic(OperationAnalysisContext context, DiagnosticDescriptor diagnosticDescriptor, ParameterDiagnosticReason reason, IParameterSymbol param, IFieldSymbol field)
+            {
+                var properties = ImmutableDictionary<string, string?>.Empty
+                    .SetItem(ReferencedFieldOrPropertyNameKey, field.Name)
+                    .SetItem(DiagnosticReasonKey, reason.ToString());
+
+                context.ReportDiagnostic(
+                    param.CreateDiagnostic(
+                        diagnosticDescriptor,
+                        properties,
+                        param.ContainingType.ToDisplayString(SymbolDisplayFormats.ShortSymbolDisplayFormat),
+                        param.Name,
+                        field.Name));
+            }
+
+            private static void ReportPropertyDiagnostic(OperationAnalysisContext context, DiagnosticDescriptor diagnosticDescriptor, ParameterDiagnosticReason reason, IParameterSymbol param, IPropertySymbol prop)
+            {
+                var properties = ImmutableDictionary<string, string?>.Empty
+                    .SetItem(ReferencedFieldOrPropertyNameKey, prop.Name)
+                    .SetItem(DiagnosticReasonKey, reason.ToString());
+
+                context.ReportDiagnostic(
+                    param.CreateDiagnostic(
+                        diagnosticDescriptor,
+                        properties,
+                        param.ContainingType.ToDisplayString(SymbolDisplayFormats.ShortSymbolDisplayFormat),
+                        param.Name,
+                        prop.Name));
+            }
+        }
+    }
+}