Merge pull request #6677 from dotnet/primaryConstructors

Do not fire 'make member static' when a method references a primary constructor parameter.
dotnet · Jun 8, 2023 · 2f6e15a · 2f6e15a
2 parents 2b6ab8d + 7b549bd
commit 2f6e15a
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 18 deletions.
diff --git a/...etAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/MarkMembersAsStatic.cs b/...etAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/MarkMembersAsStatic.cs
@@ -104,7 +104,7 @@ void OnOperationBlockStart(OperationBlockStartAnalysisContext context)
 
                     context.RegisterOperationAction(context =>
                     {
-                        if (((IInstanceReferenceOperation)context.Operation).ReferenceKind == InstanceReferenceKind.ContainingTypeInstance
+                        if (context.Operation is IInstanceReferenceOperation { ReferenceKind: InstanceReferenceKind.ContainingTypeInstance }
                             && (context.Operation.Parent is not IInvocationOperation invocation || !invocation.TargetMethod.Equals(methodSymbol, SymbolEqualityComparer.Default)))
                         {
                             isInstanceReferenced = true;
@@ -120,6 +120,17 @@ void OnOperationBlockStart(OperationBlockStartAnalysisContext context)
                         }
                     }, OperationKind.None);
 
+                    context.RegisterOperationAction(context =>
+                    {
+                        if (context.Operation is IParameterReferenceOperation { Parameter.ContainingSymbol: IMethodSymbol { MethodKind: MethodKind.Constructor } })
+                        {
+                            // we're referencing a parameter not from our actual method, but from a type constructor.
+                            // This must be a primary constructor scenario, and we're capturing the parameter here.  
+                            // This member cannot be made static.
+                            isInstanceReferenced = true;
+                        }
+                    }, OperationKind.ParameterReference);
+
                     context.RegisterOperationBlockEndAction(context =>
                     {
                         if (!isInstanceReferenced)

diff --git a/...s/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/MarkMembersAsStaticTests.cs b/...s/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/MarkMembersAsStaticTests.cs
@@ -1,19 +1,18 @@
-// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
 using System;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeQuality.CSharp.Analyzers.QualityGuidelines;
+using Microsoft.CodeQuality.VisualBasic.Analyzers.QualityGuidelines;
 using Test.Utilities;
 using Xunit;
-using VerifyCS = Test.Utilities.CSharpCodeFixVerifier<
-    Microsoft.CodeQuality.Analyzers.QualityGuidelines.MarkMembersAsStaticAnalyzer,
-    Microsoft.CodeQuality.CSharp.Analyzers.QualityGuidelines.CSharpMarkMembersAsStaticFixer>;
-using VerifyVB = Test.Utilities.VisualBasicCodeFixVerifier<
-    Microsoft.CodeQuality.Analyzers.QualityGuidelines.MarkMembersAsStaticAnalyzer,
-    Microsoft.CodeQuality.VisualBasic.Analyzers.QualityGuidelines.BasicMarkMembersAsStaticFixer>;
 
 namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines.UnitTests
 {
+    using VerifyCS = CSharpCodeFixVerifier<MarkMembersAsStaticAnalyzer, CSharpMarkMembersAsStaticFixer>;
+    using VerifyVB = VisualBasicCodeFixVerifier<MarkMembersAsStaticAnalyzer, BasicMarkMembersAsStaticFixer>;
+
     public class MarkMembersAsStaticTests
     {
         [Fact]
@@ -1473,13 +1472,13 @@ public Task RecursiveMethod_DiagnosticAsync()
 public class Test
 {
     public void [|Recursive|](string argument)
-	{
-		if (argument.Length > 1)
-		{
+    {
+        if (argument.Length > 1)
+        {
             Recursive(argument[1..]);
-		}
+        }
 
-		Console.WriteLine($""argument[-1]: {argument}"");
+        Console.WriteLine($""argument[-1]: {argument}"");
     }
 }",
                 FixedCode = @"
@@ -1488,17 +1487,47 @@ public class Test
 public class Test
 {
     public static void Recursive(string argument)
-	{
-		if (argument.Length > 1)
-		{
+    {
+        if (argument.Length > 1)
+        {
             Recursive(argument[1..]);
-		}
+        }
 
-		Console.WriteLine($""argument[-1]: {argument}"");
+        Console.WriteLine($""argument[-1]: {argument}"");
     }
 }",
                 LanguageVersion = LanguageVersion.CSharp8
             }.RunAsync();
         }
+
+        [Fact(Skip = "Need update of roslyn to parse primary constructors properly"), WorkItem(6573, "https://github.com/dotnet/roslyn-analyzers/issues/6573")]
+        public Task PrimaryConstructor()
+        {
+            return new VerifyCS.Test
+            {
+                TestCode = """
+                    using System;
+                    using System.Collections.Generic; 
+                    using System.Linq;
+
+                    public class Student(int id, string name, IEnumerable<decimal> grades)
+                    {
+                        public Student(int id, string name) : this(id, name, new List<decimal>()) { }
+                        public int Id => id;
+                        public string Name { get; set; } = name.Trim();
+
+                        // validate property
+                        public decimal GPA => grades.Any() ? grades.Average() : 4.0m;
+
+                        // validate method
+                        public decimal GetGPA() => grades.Any() ? grades.Average() : 4.0m;
+
+                        // validate indexer
+                        public int this[int x] => id;
+                    }
+                    """,
+                LanguageVersion = LanguageVersion.Preview
+            }.RunAsync();
+        }
     }
 }