Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S1939: Add support for record struct #5606

Merged
merged 28 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cc1533b
Add support for structs
martin-strecker-sonarsource Apr 27, 2022
e26dc2e
Simplify ToMultiValueDictionary
martin-strecker-sonarsource Apr 27, 2022
370d793
Fix tests
martin-strecker-sonarsource Apr 27, 2022
6710158
Whitespace
martin-strecker-sonarsource Apr 27, 2022
e31595a
Extract DiagnosticsProperties
martin-strecker-sonarsource Apr 27, 2022
a693338
Formatting
martin-strecker-sonarsource Apr 27, 2022
237ed05
WIP: Introduce Lookup from Dictionary<TKey, IEnumerable<TValue>> like…
martin-strecker-sonarsource Apr 27, 2022
34f1fe6
Use ILookup instead of MultiValueDictionary
martin-strecker-sonarsource Apr 27, 2022
4436479
Add overload for ToLookup.
martin-strecker-sonarsource Apr 27, 2022
3d53444
Refactor code duplication.
martin-strecker-sonarsource Apr 28, 2022
c0ad4cf
Clean-up
martin-strecker-sonarsource Apr 28, 2022
6b2b68b
Simplify switch statement and throw on unsupported syntax nodes.
martin-strecker-sonarsource May 2, 2022
ee550b2
Apply suggestions from code review
martin-strecker-sonarsource May 3, 2022
b6a19e9
new test harness
martin-strecker-sonarsource May 3, 2022
18f7f4c
fix compiler error after suggestion
martin-strecker-sonarsource May 3, 2022
f8cbf6f
Merge ifs
martin-strecker-sonarsource May 3, 2022
320ded8
merge ifs
martin-strecker-sonarsource May 3, 2022
aa95de3
refactor GetLocationWithToken
martin-strecker-sonarsource May 3, 2022
f30f670
Change signature of CollidingDeclaration
martin-strecker-sonarsource May 3, 2022
7e4c15f
Remove Lookup impl and use MultiValueDict
martin-strecker-sonarsource May 3, 2022
abe8628
Use ToImmutableArray
martin-strecker-sonarsource May 3, 2022
ac7e015
Rmove unused usings
martin-strecker-sonarsource May 3, 2022
42839aa
verify codefixes by calling VerifyCodeFix
martin-strecker-sonarsource May 3, 2022
9fb11c2
Formatting
martin-strecker-sonarsource May 5, 2022
127d8ac
Remove code duplication
martin-strecker-sonarsource May 5, 2022
65a883a
Formatting
martin-strecker-sonarsource May 5, 2022
7a27e74
Remove named parameter
martin-strecker-sonarsource May 5, 2022
af4c205
Don't use the builder for the immutable dict just yet
martin-strecker-sonarsource May 5, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 65 additions & 122 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantInheritanceList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,16 @@
* 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;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Text;
using SonarAnalyzer.Common;
using SonarAnalyzer.Extensions;
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand All @@ -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<DiagnosticDescriptor> 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) =>
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
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<string, string>.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<string, string>.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;
}
Expand All @@ -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<string, string>.Empty.Add(RedundantIndexKey, i.ToString(CultureInfo.InvariantCulture)),
message));
}
}

Expand All @@ -156,40 +119,29 @@ private static MultiValueDictionary<INamedTypeSymbol, INamedTypeSymbol> GetImple
.Select(baseType => semanticModel.GetSymbolInfo(baseType.Type).Symbol as INamedTypeSymbol)
.WhereNotNull()
.Distinct()
.Select(symbol => new Tuple<INamedTypeSymbol, ICollection<INamedTypeSymbol>>(symbol, symbol.AllInterfaces))
.ToMultiValueDictionary(kv => kv.Item1, kv => kv.Item2);
.ToMultiValueDictionary(x => x.AllInterfaces);

private static bool TryGetCollidingDeclaration(INamedTypeSymbol declaredType,
INamedTypeSymbol interfaceType,
MultiValueDictionary<INamedTypeSymbol, INamedTypeSymbol> interfaceMappings,
out INamedTypeSymbol collidingDeclaration)
private static INamedTypeSymbol CollidingDeclaration(INamedTypeSymbol declaredType, INamedTypeSymbol interfaceType, MultiValueDictionary<INamedTypeSymbol, INamedTypeSymbol> 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())
{
Expand All @@ -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<INamedTypeSymbol> AllInterfacesAndSelf(INamedTypeSymbol interfaceType) =>
interfaceType.AllInterfaces.Concat(new[] { interfaceType });
}

private static Location GetLocationWithToken(TypeSyntax type, SeparatedSyntaxList<BaseTypeSyntax> 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<string, string> DiagnosticsProperties(int redundantIndex) =>
ImmutableDictionary.Create<string, string>().Add(RedundantIndexKey, redundantIndex.ToString());
}
}
21 changes: 9 additions & 12 deletions analyzers/src/SonarAnalyzer.Common/Common/MultiValueDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,16 @@ protected MultiValueDictionary(SerializationInfo info, StreamingContext context)
}

public static MultiValueDictionary<TKey, TValue> Create<TUnderlying>()
where TUnderlying : ICollection<TValue>, new()
{
return new MultiValueDictionary<TKey, TValue>
where TUnderlying : ICollection<TValue>, new() =>
new MultiValueDictionary<TKey, TValue>
{
UnderlyingCollectionFactory = () => new TUnderlying()
};
}

private Func<ICollection<TValue>> UnderlyingCollectionFactory { get; set; } = () => new List<TValue>();

public void Add(TKey key, TValue value)
{
public void Add(TKey key, TValue value) =>
AddWithKey(key, value);
}

public void AddWithKey(TKey key, TValue value)
{
Expand Down Expand Up @@ -88,11 +84,12 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont

public static class MultiValueDictionaryExtensions
{
public static MultiValueDictionary<TKey, TElement> ToMultiValueDictionary<TSource, TKey, TElement>(
this IEnumerable<TSource> source,
Func<TSource, TKey> keySelector,
Func<TSource, ICollection<TElement>> elementSelector)
where TSource : Tuple<TKey, ICollection<TElement>>
public static MultiValueDictionary<TSource, TElement> ToMultiValueDictionary<TSource, TElement>(this IEnumerable<TSource> source, Func<TSource, ICollection<TElement>> elementSelector) =>
source.ToMultiValueDictionary(x => x, elementSelector);

public static MultiValueDictionary<TKey, TElement> ToMultiValueDictionary<TSource, TKey, TElement>(this IEnumerable<TSource> source,
Func<TSource, TKey> keySelector,
Func<TSource, ICollection<TElement>> elementSelector)
{
var dictionary = new MultiValueDictionary<TKey, TElement>();
foreach (var item in source)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,42 +27,43 @@ namespace SonarAnalyzer.UnitTest.Rules
[TestClass]
public class RedundantInheritanceListTest
{
private readonly VerifierBuilder rule = new VerifierBuilder<RedundantInheritanceList>();
private readonly VerifierBuilder codeFix = new VerifierBuilder<RedundantInheritanceList>().WithCodeFix<RedundantInheritanceListCodeFix>();

[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<RedundantInheritanceListCodeFix>(
@"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<RedundantInheritanceListCodeFix>(
@"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<RedundantInheritanceListCodeFix>(
@"TestCases\RedundantInheritanceList.cs",
@"TestCases\RedundantInheritanceList.Fixed.cs",
new RedundantInheritanceList());
codeFix.AddPaths("RedundantInheritanceList.cs")
.WithCodeFixedPaths("RedundantInheritanceList.Fixed.cs")
.VerifyCodeFix();
}
}
Original file line number Diff line number Diff line change
@@ -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
Loading