Review update

SonarSource · Jun 6, 2024 · 2c735df · 2c735df
1 parent 4ebcc90
commit 2c735df
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 35 deletions.
diff --git a/....CSharp/Rules/ClassShouldNotBeAbstract.cs → ....CSharp/Rules/AbstractClassToInterface.cs b/....CSharp/Rules/ClassShouldNotBeAbstract.cs → ....CSharp/Rules/AbstractClassToInterface.cs
@@ -21,7 +21,7 @@
 namespace SonarAnalyzer.Rules.CSharp;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-public sealed class ClassShouldNotBeAbstract : SonarDiagnosticAnalyzer<SyntaxKind>
+public sealed class AbstractClassToInterface : SonarDiagnosticAnalyzer<SyntaxKind>
 {
     private const string DiagnosticId = "S1694";
 
@@ -30,7 +30,7 @@ public sealed class ClassShouldNotBeAbstract : SonarDiagnosticAnalyzer<SyntaxKin
     protected override string MessageFormat => "Convert this 'abstract' {0} to an interface.";
     protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
 
-    public ClassShouldNotBeAbstract() : base(DiagnosticId) { }
+    public AbstractClassToInterface() : base(DiagnosticId) { }
 
     protected override void Initialize(SonarAnalysisContext context) =>
         context.RegisterSymbolAction(
@@ -39,9 +39,9 @@ protected override void Initialize(SonarAnalysisContext context) =>
                 var symbol = (INamedTypeSymbol)c.Symbol;
                 if (symbol.IsClass()
                     && symbol.IsAbstract
-                    && !HasInheritedAbstractMembers(symbol)
+                    && symbol.BaseType.Is(KnownType.System_Object)
                     && !IsRecordWithParameters(symbol)
-                    && AbstractTypeShouldBeInterface(symbol))
+                    && AllMethodsAreAbstract(symbol))
                 {
                     foreach (var declaringSyntaxReference in symbol.DeclaringSyntaxReferences)
                     {
@@ -61,36 +61,18 @@ protected override void Initialize(SonarAnalysisContext context) =>
             },
             SymbolKind.NamedType);
 
-    private static bool HasInheritedAbstractMembers(INamedTypeSymbol symbol)
-    {
-        var baseTypes = symbol.BaseType.GetSelfAndBaseTypes().ToList();
-        var abstractMethods = baseTypes.SelectMany(GetAllAbstractMethods);
-        var baseTypesAndSelf = baseTypes.Concat(new[] { symbol }).ToList();
-        var overrideMethods = baseTypesAndSelf.SelectMany(GetAllOverrideMethods);
-        var overriddenMethods = overrideMethods.Select(x => x.OverriddenMethod);
-        var stillAbstractMethods = abstractMethods.Except(overriddenMethods);
-
-        return stillAbstractMethods.Any();
-    }
-
-    private static IEnumerable<IMethodSymbol> GetAllAbstractMethods(INamedTypeSymbol symbol) =>
-        GetAllMethods(symbol).Where(x => x.IsAbstract);
-
-    private static IEnumerable<IMethodSymbol> GetAllOverrideMethods(INamedTypeSymbol symbol) =>
-        GetAllMethods(symbol).Where(x => x.IsOverride);
-
     private static bool IsRecordWithParameters(ISymbol symbol) =>
         symbol.DeclaringSyntaxReferences.Any(x => x.GetSyntax() is { } node
                                                   && RecordDeclarationSyntaxWrapper.IsInstance(node)
                                                   && ((RecordDeclarationSyntaxWrapper)node).ParameterList is { } parameterList
                                                   && parameterList.Parameters.Count > 0);
 
-    private static bool AbstractTypeShouldBeInterface(INamedTypeSymbol symbol)
+    private static bool AllMethodsAreAbstract(INamedTypeSymbol symbol)
     {
         var methods = GetAllMethods(symbol);
-        return symbol.BaseType.Is(KnownType.System_Object) && methods.Any() && methods.All(x => x.IsAbstract);
+        return methods.Any() && methods.All(x => x.IsAbstract);
     }
 
-    private static IList<IMethodSymbol> GetAllMethods(INamedTypeSymbol symbol) =>
-        symbol.GetMembers().OfType<IMethodSymbol>().Where(x => !x.IsImplicitlyDeclared && !ConstructorKinds.Contains(x.MethodKind)).ToList();
+    private static IMethodSymbol[] GetAllMethods(INamedTypeSymbol symbol) =>
+        symbol.GetMembers().OfType<IMethodSymbol>().Where(x => !x.IsImplicitlyDeclared && !ConstructorKinds.Contains(x.MethodKind)).ToArray();
 }
diff --git a/analyzers/tests/SonarAnalyzer.Test/AnalysisContext/SonarAnalysisContextTest.cs b/analyzers/tests/SonarAnalyzer.Test/AnalysisContext/SonarAnalysisContextTest.cs
@@ -65,7 +65,7 @@ public partial class SonarAnalysisContextTest
             // S2953 - MAIN only
             new TestSetup("DisposeNotImplementingDispose.cs", new DisposeNotImplementingDispose()),
             // S1694 - MAIN only
-            new TestSetup("ClassShouldNotBeAbstract.cs", new ClassShouldNotBeAbstract()),
+            new TestSetup("ClassShouldNotBeAbstract.cs", new AbstractClassToInterface()),
         });
 
     [TestMethod]

diff --git a/...est/Rules/ClassShouldNotBeAbstractTest.cs → ...est/Rules/AbstractClassToInterfaceTest.cs b/...est/Rules/ClassShouldNotBeAbstractTest.cs → ...est/Rules/AbstractClassToInterfaceTest.cs
@@ -23,25 +23,25 @@
 namespace SonarAnalyzer.Test.Rules;
 
 [TestClass]
-public class ClassShouldNotBeAbstractTest
+public class AbstractClassToInterfaceTest
 {
-    private readonly VerifierBuilder builder = new VerifierBuilder<ClassShouldNotBeAbstract>();
+    private readonly VerifierBuilder builder = new VerifierBuilder<AbstractClassToInterface>();
 
     [TestMethod]
     public void ClassShouldNotBeAbstract() =>
-        builder.AddPaths("ClassShouldNotBeAbstract.cs").Verify();
+        builder.AddPaths("AbstractClassToInterface.cs").Verify();
 
 #if NET
 
     [TestMethod]
     public void ClassShouldNotBeAbstract_CSharp9() =>
-        builder.AddPaths("ClassShouldNotBeAbstract.CSharp9.cs")
+        builder.AddPaths("AbstractClassToInterface.CSharp9.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp9)
             .Verify();
 
     [TestMethod]
     public void ClassShouldNotBeAbstract_CSharp11() =>
-        builder.AddPaths("ClassShouldNotBeAbstract.CSharp11.cs")
+        builder.AddPaths("AbstractClassToInterface.CSharp11.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp11)
             .Verify();
 

diff --git a/...ases/ClassShouldNotBeAbstract.CSharp11.cs → ...ases/AbstractClassToInterface.CSharp11.cs b/...ases/ClassShouldNotBeAbstract.CSharp11.cs → ...ases/AbstractClassToInterface.CSharp11.cs
diff --git a/...Cases/ClassShouldNotBeAbstract.CSharp9.cs → ...Cases/AbstractClassToInterface.CSharp9.cs b/...Cases/ClassShouldNotBeAbstract.CSharp9.cs → ...Cases/AbstractClassToInterface.CSharp9.cs
diff --git a/...est/TestCases/ClassShouldNotBeAbstract.cs → ...est/TestCases/AbstractClassToInterface.cs b/...est/TestCases/ClassShouldNotBeAbstract.cs → ...est/TestCases/AbstractClassToInterface.cs
@@ -1,15 +1,24 @@
 using System;
 using System.Collections.Generic;
 
-public abstract partial class A
+public abstract partial class PartialMixed
 {
     public abstract void X();
 }
-public abstract partial class A
+public abstract partial class PartialMixed
 {
     public void Y() { }
 }
 
+public abstract partial class PartialAbstract // Noncompliant
+{
+    public abstract void X();
+}
+public abstract partial class PartialAbstract // Noncompliant
+{
+    public abstract void Y();
+}
+
 public abstract class Empty
 {
 }
@@ -67,7 +76,6 @@ public int getRed()
 
 public abstract class LampCompliant
 {
-
     private bool switchLamp = false;
 
     public abstract void glow();
@@ -105,3 +113,18 @@ public abstract class View3Derived : SomeUnknownType // Error [CS0246]
     public string Content3 { get; }
     public override int Content1 { get { return 1; } }
 }
+
+public abstract class WithConstructor   // Noncompliant
+{
+    public abstract void ToOverride();
+
+    static WithConstructor()
+    {
+        // Do something here
+    }
+
+    public WithConstructor()
+    {
+        // Do something here
+    }
+}