From 2c735df430070ef402456049f55a7c006e4fb3da Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Thu, 6 Jun 2024 10:52:00 +0200 Subject: [PATCH] Review update --- ...bstract.cs => AbstractClassToInterface.cs} | 34 +++++-------------- .../SonarAnalysisContextTest.cs | 2 +- ...est.cs => AbstractClassToInterfaceTest.cs} | 10 +++--- ...s => AbstractClassToInterface.CSharp11.cs} | 0 ...cs => AbstractClassToInterface.CSharp9.cs} | 0 ...bstract.cs => AbstractClassToInterface.cs} | 29 ++++++++++++++-- 6 files changed, 40 insertions(+), 35 deletions(-) rename analyzers/src/SonarAnalyzer.CSharp/Rules/{ClassShouldNotBeAbstract.cs => AbstractClassToInterface.cs} (68%) rename analyzers/tests/SonarAnalyzer.Test/Rules/{ClassShouldNotBeAbstractTest.cs => AbstractClassToInterfaceTest.cs} (84%) rename analyzers/tests/SonarAnalyzer.Test/TestCases/{ClassShouldNotBeAbstract.CSharp11.cs => AbstractClassToInterface.CSharp11.cs} (100%) rename analyzers/tests/SonarAnalyzer.Test/TestCases/{ClassShouldNotBeAbstract.CSharp9.cs => AbstractClassToInterface.CSharp9.cs} (100%) rename analyzers/tests/SonarAnalyzer.Test/TestCases/{ClassShouldNotBeAbstract.cs => AbstractClassToInterface.cs} (78%) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeAbstract.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AbstractClassToInterface.cs similarity index 68% rename from analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeAbstract.cs rename to analyzers/src/SonarAnalyzer.CSharp/Rules/AbstractClassToInterface.cs index 039c8ecb13b..6b4ce9d8a56 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeAbstract.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AbstractClassToInterface.cs @@ -21,7 +21,7 @@ namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] -public sealed class ClassShouldNotBeAbstract : SonarDiagnosticAnalyzer +public sealed class AbstractClassToInterface : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S1694"; @@ -30,7 +30,7 @@ public sealed class ClassShouldNotBeAbstract : SonarDiagnosticAnalyzer "Convert this 'abstract' {0} to an interface."; protected override ILanguageFacade 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 GetAllAbstractMethods(INamedTypeSymbol symbol) => - GetAllMethods(symbol).Where(x => x.IsAbstract); - - private static IEnumerable 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 GetAllMethods(INamedTypeSymbol symbol) => - symbol.GetMembers().OfType().Where(x => !x.IsImplicitlyDeclared && !ConstructorKinds.Contains(x.MethodKind)).ToList(); + private static IMethodSymbol[] GetAllMethods(INamedTypeSymbol symbol) => + symbol.GetMembers().OfType().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 index 3327c327942..70a7bf0b721 100644 --- 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/analyzers/tests/SonarAnalyzer.Test/Rules/ClassShouldNotBeAbstractTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/AbstractClassToInterfaceTest.cs similarity index 84% rename from analyzers/tests/SonarAnalyzer.Test/Rules/ClassShouldNotBeAbstractTest.cs rename to analyzers/tests/SonarAnalyzer.Test/Rules/AbstractClassToInterfaceTest.cs index 0b9b0f60bfb..2f4851ef871 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/ClassShouldNotBeAbstractTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/AbstractClassToInterfaceTest.cs @@ -23,25 +23,25 @@ namespace SonarAnalyzer.Test.Rules; [TestClass] -public class ClassShouldNotBeAbstractTest +public class AbstractClassToInterfaceTest { - private readonly VerifierBuilder builder = new VerifierBuilder(); + private readonly VerifierBuilder builder = new VerifierBuilder(); [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/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.CSharp11.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.CSharp11.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.CSharp11.cs rename to analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.CSharp11.cs diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.CSharp9.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.CSharp9.cs rename to analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.CSharp9.cs diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.cs similarity index 78% rename from analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.cs rename to analyzers/tests/SonarAnalyzer.Test/TestCases/AbstractClassToInterface.cs index a0e0866856b..af46ffc7251 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeAbstract.cs +++ b/analyzers/tests/SonarAnalyzer.Test/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 + } +}