diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantInheritanceList.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantInheritanceList.cs index 2a3e6090ddb..f641525dafd 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantInheritanceList.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantInheritanceList.cs @@ -18,10 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Globalization; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -29,6 +27,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using SonarAnalyzer.Common; +using SonarAnalyzer.Extensions; using SonarAnalyzer.Helpers; using StyleCop.Analyzers.Lightup; @@ -44,81 +43,53 @@ public sealed class RedundantInheritanceList : SonarDiagnosticAnalyzer private const string MessageAlreadyImplements = "'{0}' implements '{1}' so '{1}' can be removed from the inheritance list."; private const string MessageFormat = "{0}"; - private static readonly DiagnosticDescriptor Rule = - DiagnosticDescriptorBuilder.GetDescriptor(DiagnosticId, MessageFormat, RspecStrings.ResourceManager); + private static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorBuilder.GetDescriptor(DiagnosticId, MessageFormat, RspecStrings.ResourceManager); public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterSyntaxNodeActionInNonGenerated(CheckEnum, SyntaxKind.EnumDeclaration); - context.RegisterSyntaxNodeActionInNonGenerated(CheckInterface, SyntaxKind.InterfaceDeclaration); - context.RegisterSyntaxNodeActionInNonGenerated(CheckClassAndRecord, SyntaxKind.ClassDeclaration, SyntaxKindEx.RecordClassDeclaration); - } - - private static void CheckClassAndRecord(SyntaxNodeAnalysisContext context) - { - var typeDeclaration = (TypeDeclarationSyntax)context.Node; - if (context.ContainingSymbol.Kind != SymbolKind.NamedType - || IsBaseListNullOrEmpty(typeDeclaration)) + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterSyntaxNodeActionInNonGenerated(c => { - return; - } - + if (c.IsRedundantPositionalRecordContext() || c.Node is not BaseTypeDeclarationSyntax { BaseList: { Types: { Count: > 0 } } }) + { + return; + } + switch (c.Node) + { + case EnumDeclarationSyntax enumDeclaration: + ReportRedundantBaseType(c, enumDeclaration, KnownType.System_Int32, MessageEnum); + break; + case InterfaceDeclarationSyntax interfaceDeclaration: + ReportRedundantInterfaces(c, interfaceDeclaration); + break; + case TypeDeclarationSyntax nonInterfaceDeclaration: + ReportRedundantBaseType(c, nonInterfaceDeclaration, KnownType.System_Object, MessageObjectBase); + ReportRedundantInterfaces(c, nonInterfaceDeclaration); + break; + } + }, + SyntaxKind.EnumDeclaration, + SyntaxKind.InterfaceDeclaration, + SyntaxKind.ClassDeclaration, + SyntaxKind.StructDeclaration, + SyntaxKindEx.RecordClassDeclaration, + SyntaxKindEx.RecordStructDeclaration); + + private static void ReportRedundantBaseType(SyntaxNodeAnalysisContext context, BaseTypeDeclarationSyntax typeDeclaration, KnownType redundantType, string message) + { var baseTypeSyntax = typeDeclaration.BaseList.Types.First().Type; - if (!(context.SemanticModel.GetSymbolInfo(baseTypeSyntax).Symbol is ITypeSymbol baseTypeSymbol)) - { - return; - } - - if (baseTypeSymbol.Is(KnownType.System_Object)) + if (context.SemanticModel.GetSymbolInfo(baseTypeSyntax).Symbol is ITypeSymbol baseTypeSymbol + && baseTypeSymbol.Is(redundantType)) { var location = GetLocationWithToken(baseTypeSyntax, typeDeclaration.BaseList.Types); - context.ReportIssue( - Diagnostic.Create(Rule, location, - ImmutableDictionary.Empty.Add(RedundantIndexKey, "0"), - MessageObjectBase)); - } - - ReportRedundantInterfaces(context, typeDeclaration); - } - - private static void CheckInterface(SyntaxNodeAnalysisContext context) - { - var interfaceDeclaration = (InterfaceDeclarationSyntax)context.Node; - if (IsBaseListNullOrEmpty(interfaceDeclaration)) - { - return; + context.ReportIssue(Diagnostic.Create(Rule, location, DiagnosticsProperties(0), message)); } - - ReportRedundantInterfaces(context, interfaceDeclaration); - } - - private static void CheckEnum(SyntaxNodeAnalysisContext context) - { - var enumDeclaration = (EnumDeclarationSyntax)context.Node; - if (IsBaseListNullOrEmpty(enumDeclaration)) - { - return; - } - - var baseTypeSyntax = enumDeclaration.BaseList.Types.First().Type; - var baseTypeSymbol = context.SemanticModel.GetSymbolInfo(baseTypeSyntax).Symbol as ITypeSymbol; - if (!baseTypeSymbol.Is(KnownType.System_Int32)) - { - return; - } - - context.ReportIssue( - Diagnostic.Create(Rule, enumDeclaration.BaseList.GetLocation(), - ImmutableDictionary.Empty.Add(RedundantIndexKey, "0"), - MessageEnum)); } private static void ReportRedundantInterfaces(SyntaxNodeAnalysisContext context, BaseTypeDeclarationSyntax typeDeclaration) { var declaredType = context.SemanticModel.GetDeclaredSymbol(typeDeclaration); - if (declaredType == null) + if (declaredType is null) { return; } @@ -129,25 +100,17 @@ private static void ReportRedundantInterfaces(SyntaxNodeAnalysisContext context, for (var i = 0; i < baseList.Types.Count; i++) { var baseType = baseList.Types[i]; - if (!(context.SemanticModel.GetSymbolInfo(baseType.Type).Symbol is INamedTypeSymbol interfaceType) - || !interfaceType.IsInterface()) + if (context.SemanticModel.GetSymbolInfo(baseType.Type).Symbol is INamedTypeSymbol interfaceType + && interfaceType.IsInterface() + && CollidingDeclaration(declaredType, interfaceType, interfaceTypesWithAllInterfaces) is { } collidingDeclaration) { - continue; - } + var location = GetLocationWithToken(baseType.Type, baseList.Types); + var message = string.Format(MessageAlreadyImplements, + collidingDeclaration.ToMinimalDisplayString(context.SemanticModel, baseType.Type.SpanStart), + interfaceType.ToMinimalDisplayString(context.SemanticModel, baseType.Type.SpanStart)); - if (!TryGetCollidingDeclaration(declaredType, interfaceType, interfaceTypesWithAllInterfaces, out var collidingDeclaration)) - { - continue; + context.ReportIssue(Diagnostic.Create(Rule, location, DiagnosticsProperties(i), message)); } - - var location = GetLocationWithToken(baseType.Type, baseList.Types); - var message = string.Format(MessageAlreadyImplements, - collidingDeclaration.ToMinimalDisplayString(context.SemanticModel, baseType.Type.SpanStart), - interfaceType.ToMinimalDisplayString(context.SemanticModel, baseType.Type.SpanStart)); - - context.ReportIssue(Diagnostic.Create(Rule, location, - ImmutableDictionary.Empty.Add(RedundantIndexKey, i.ToString(CultureInfo.InvariantCulture)), - message)); } } @@ -156,40 +119,29 @@ private static MultiValueDictionary GetImple .Select(baseType => semanticModel.GetSymbolInfo(baseType.Type).Symbol as INamedTypeSymbol) .WhereNotNull() .Distinct() - .Select(symbol => new Tuple>(symbol, symbol.AllInterfaces)) - .ToMultiValueDictionary(kv => kv.Item1, kv => kv.Item2); + .ToMultiValueDictionary(x => x.AllInterfaces); - private static bool TryGetCollidingDeclaration(INamedTypeSymbol declaredType, - INamedTypeSymbol interfaceType, - MultiValueDictionary interfaceMappings, - out INamedTypeSymbol collidingDeclaration) + private static INamedTypeSymbol CollidingDeclaration(INamedTypeSymbol declaredType, INamedTypeSymbol interfaceType, MultiValueDictionary interfaceMappings) { - var collisionMapping = interfaceMappings - .Where(i => i.Key.IsInterface()) - .FirstOrDefault(v => v.Value.Contains(interfaceType)); - - if (collisionMapping.Key != null) + var collisionMapping = interfaceMappings.FirstOrDefault(x => x.Key.IsInterface() && x.Value.Contains(interfaceType)); + if (collisionMapping.Key is not null) { - collidingDeclaration = collisionMapping.Key; - return true; + return collisionMapping.Key; } - var baseClassMapping = interfaceMappings.FirstOrDefault(i => i.Key.IsClass()); - if (baseClassMapping.Key == null) + var baseClassMapping = interfaceMappings.FirstOrDefault(x => x.Key.IsClass()); + if (baseClassMapping.Key is null) { - collidingDeclaration = null; - return false; + return null; } - collidingDeclaration = baseClassMapping.Key; - return CanInterfaceBeRemovedBasedOnMembers(declaredType, interfaceType); + var canBeRemoved = CanInterfaceBeRemovedBasedOnMembers(declaredType, interfaceType); + return canBeRemoved ? baseClassMapping.Key : null; } private static bool CanInterfaceBeRemovedBasedOnMembers(INamedTypeSymbol declaredType, INamedTypeSymbol interfaceType) { - var allMembersOfInterface = interfaceType.AllInterfaces.Concat(new[] { interfaceType }) - .SelectMany(i => i.GetMembers()) - .ToList(); + var allMembersOfInterface = AllInterfacesAndSelf(interfaceType).SelectMany(x => x.GetMembers()).ToImmutableArray(); if (!allMembersOfInterface.Any()) { @@ -198,38 +150,29 @@ private static bool CanInterfaceBeRemovedBasedOnMembers(INamedTypeSymbol declare foreach (var interfaceMember in allMembersOfInterface) { - var classMember = declaredType.FindImplementationForInterfaceMember(interfaceMember); - if (classMember != null + if (declaredType.FindImplementationForInterfaceMember(interfaceMember) is { } classMember && (classMember.ContainingType.Equals(declaredType) - || !classMember.ContainingType.Interfaces.SelectMany(i => i.AllInterfaces.Concat(new[] { i })).Contains(interfaceType))) + || !classMember.ContainingType.Interfaces.Any(x => AllInterfacesAndSelf(x).Contains(interfaceType)))) { return false; } } return true; + + static IEnumerable AllInterfacesAndSelf(INamedTypeSymbol interfaceType) => + interfaceType.AllInterfaces.Concat(new[] { interfaceType }); } private static Location GetLocationWithToken(TypeSyntax type, SeparatedSyntaxList baseTypes) { - int start; - int end; - - if (baseTypes.Count == 1 - || baseTypes.First().Type != type) - { - start = type.GetFirstToken().GetPreviousToken().Span.Start; - end = type.Span.End; - } - else - { - start = type.SpanStart; - end = type.GetLastToken().GetNextToken().Span.End; - } + var span = baseTypes.Count == 1 || baseTypes.First().Type != type + ? TextSpan.FromBounds(type.GetFirstToken().GetPreviousToken().Span.Start, type.Span.End) + : TextSpan.FromBounds(type.SpanStart, type.GetLastToken().GetNextToken().Span.End); - return Location.Create(type.SyntaxTree, new TextSpan(start, end - start)); + return Location.Create(type.SyntaxTree, span); } - private static bool IsBaseListNullOrEmpty(BaseTypeDeclarationSyntax baseTypeDeclaration) => - baseTypeDeclaration.BaseList == null || !baseTypeDeclaration.BaseList.Types.Any(); + private static ImmutableDictionary DiagnosticsProperties(int redundantIndex) => + ImmutableDictionary.Create().Add(RedundantIndexKey, redundantIndex.ToString()); } } diff --git a/analyzers/src/SonarAnalyzer.Common/Common/MultiValueDictionary.cs b/analyzers/src/SonarAnalyzer.Common/Common/MultiValueDictionary.cs index 0db748ef273..88d820449f3 100644 --- a/analyzers/src/SonarAnalyzer.Common/Common/MultiValueDictionary.cs +++ b/analyzers/src/SonarAnalyzer.Common/Common/MultiValueDictionary.cs @@ -39,20 +39,16 @@ protected MultiValueDictionary(SerializationInfo info, StreamingContext context) } public static MultiValueDictionary Create() - where TUnderlying : ICollection, new() - { - return new MultiValueDictionary + where TUnderlying : ICollection, new() => + new MultiValueDictionary { UnderlyingCollectionFactory = () => new TUnderlying() }; - } private Func> UnderlyingCollectionFactory { get; set; } = () => new List(); - public void Add(TKey key, TValue value) - { + public void Add(TKey key, TValue value) => AddWithKey(key, value); - } public void AddWithKey(TKey key, TValue value) { @@ -88,11 +84,12 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont public static class MultiValueDictionaryExtensions { - public static MultiValueDictionary ToMultiValueDictionary( - this IEnumerable source, - Func keySelector, - Func> elementSelector) - where TSource : Tuple> + public static MultiValueDictionary ToMultiValueDictionary(this IEnumerable source, Func> elementSelector) => + source.ToMultiValueDictionary(x => x, elementSelector); + + public static MultiValueDictionary ToMultiValueDictionary(this IEnumerable source, + Func keySelector, + Func> elementSelector) { var dictionary = new MultiValueDictionary(); foreach (var item in source) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantInheritanceListTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantInheritanceListTest.cs index 7bf168c6774..59e323febbd 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantInheritanceListTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantInheritanceListTest.cs @@ -27,42 +27,43 @@ namespace SonarAnalyzer.UnitTest.Rules [TestClass] public class RedundantInheritanceListTest { + private readonly VerifierBuilder rule = new VerifierBuilder(); + private readonly VerifierBuilder codeFix = new VerifierBuilder().WithCodeFix(); + [TestMethod] public void RedundantInheritanceList() => - OldVerifier.VerifyAnalyzer(@"TestCases\RedundantInheritanceList.cs", new RedundantInheritanceList()); + rule.AddPaths("RedundantInheritanceList.cs").Verify(); #if NET + [TestMethod] public void RedundantInheritanceList_CSharp9() => - OldVerifier.VerifyAnalyzerFromCSharp9Library(@"TestCases\RedundantInheritanceList.CSharp9.cs", new RedundantInheritanceList()); + rule.AddPaths("RedundantInheritanceList.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); [TestMethod] public void RedundantInheritanceList_CSharp9_CodeFix() => - OldVerifier.VerifyCodeFix( - @"TestCases\RedundantInheritanceList.CSharp9.cs", - @"TestCases\RedundantInheritanceList.CSharp9.Fixed.cs", - new RedundantInheritanceList(), - ParseOptionsHelper.FromCSharp9); + codeFix.AddPaths("RedundantInheritanceList.CSharp9.cs") + .WithCodeFixedPaths("RedundantInheritanceList.CSharp9.Fixed.cs") + .WithOptions(ParseOptionsHelper.FromCSharp9) + .VerifyCodeFix(); [TestMethod] public void RedundantInheritanceList_CSharp10() => - OldVerifier.VerifyAnalyzerFromCSharp10Library(@"TestCases\RedundantInheritanceList.CSharp10.cs", new RedundantInheritanceList()); + rule.AddPaths("RedundantInheritanceList.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify(); [TestMethod] public void RedundantInheritanceList_CSharp10_CodeFix() => - OldVerifier.VerifyCodeFix( - @"TestCases\RedundantInheritanceList.CSharp10.cs", - @"TestCases\RedundantInheritanceList.CSharp10.Fixed.cs", - new RedundantInheritanceList(), - ParseOptionsHelper.FromCSharp10); + codeFix.AddPaths("RedundantInheritanceList.CSharp10.cs") + .WithCodeFixedPaths("RedundantInheritanceList.CSharp10.Fixed.cs") + .WithOptions(ParseOptionsHelper.FromCSharp10) + .VerifyCodeFix(); #endif [TestMethod] public void RedundantInheritanceList_CodeFix() => - OldVerifier.VerifyCodeFix( - @"TestCases\RedundantInheritanceList.cs", - @"TestCases\RedundantInheritanceList.Fixed.cs", - new RedundantInheritanceList()); + codeFix.AddPaths("RedundantInheritanceList.cs") + .WithCodeFixedPaths("RedundantInheritanceList.Fixed.cs") + .VerifyCodeFix(); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.Fixed.cs index 3832cdae62a..8a7cad475ba 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.Fixed.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.Fixed.cs @@ -1,11 +1,11 @@ using System; -record Record { } // Fixed +interface IBaseContract { } -record struct RedunantInterfaceImpl : IContract, IBaseContract { } // FN +interface IContract : IBaseContract { } -record struct RedunantInterfaceImplPositionalRecord(int SomeProperty) : IContract, IBaseContract { } // FN +record Record { } // Fixed -interface IContract : IBaseContract { } +record struct RedunantInterfaceImpl : IContract { } // Fixed -interface IBaseContract { } +record struct RedunantInterfaceImplPositionalRecord(int SomeProperty) : IContract { } // Fixed diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.cs index 7a9c26c9ec7..d611334b0ba 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.CSharp10.cs @@ -1,11 +1,11 @@ using System; -record Record : Object { } // Noncompliant +interface IBaseContract { } -record struct RedunantInterfaceImpl : IContract, IBaseContract { } // FN +interface IContract : IBaseContract { } -record struct RedunantInterfaceImplPositionalRecord(int SomeProperty) : IContract, IBaseContract { } // FN +record Record : Object { } // Noncompliant -interface IContract : IBaseContract { } +record struct RedunantInterfaceImpl : IContract, IBaseContract { } // Noncompliant -interface IBaseContract { } +record struct RedunantInterfaceImplPositionalRecord(int SomeProperty) : IContract, IBaseContract { } // Noncompliant diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.Fixed.cs index 47fc2cb232d..e80865423c4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.Fixed.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.Fixed.cs @@ -146,5 +146,5 @@ class X3 : X2, IMyInt // Compliant { } - struct RedunantInterfaceImpl : IA, IBase { } // FN + struct RedunantInterfaceImpl : IA { } // Fixed } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.cs index 2d1e9adab27..ba904a46ff2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantInheritanceList.cs @@ -158,5 +158,6 @@ class X3 : X2, IMyInt // Compliant { } - struct RedunantInterfaceImpl : IA, IBase { } // FN + struct RedunantInterfaceImpl : IA, IBase { } // Noncompliant {{'IA' implements 'IBase' so 'IBase' can be removed from the inheritance list.}} + // ^^^^^^^ }