Review 3

SonarSource · Feb 13, 2023 · 2088b9f · 2088b9f
1 parent c0cf33e
commit 2088b9f
Show file tree

Hide file tree

Showing 13 changed files with 116 additions and 83 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs
@@ -50,8 +50,8 @@ internal sealed class CSharpSyntaxFacade : SyntaxFacade<SyntaxKind>
 
     public override bool IsNullLiteral(SyntaxNode node) => node.IsNullLiteral();
 
-    public override bool IsKnownAttribute(SyntaxNode attribute, KnownType knownAttribute, SemanticModel semanticModel) =>
-        AttributeSyntaxExtensions.IsKnownType(Cast<AttributeSyntax>(attribute), knownAttribute, semanticModel);
+    public override bool IsKnownAttributeType(SyntaxNode attribute, KnownType knownType, SemanticModel model) =>
+        AttributeSyntaxExtensions.IsKnownType(Cast<AttributeSyntax>(attribute), knownType, model);
 
     public override IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node) =>
         node switch

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/DebuggerDisplayUsesExistingMembers.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/DebuggerDisplayUsesExistingMembers.cs
@@ -25,10 +25,11 @@ public sealed class DebuggerDisplayUsesExistingMembers : DebuggerDisplayUsesExis
 {
     protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
 
-    protected override SyntaxNode GetAttributeFormatString(AttributeSyntax attribute) =>
+    protected override SyntaxNode AttributeFormatString(AttributeSyntax attribute) =>
         attribute.ArgumentList.Arguments.FirstOrDefault() is { Expression: LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } formatString }
             ? formatString
             : null;
 
-    protected override bool IsValidMemberName(string memberName) => SyntaxFacts.IsValidIdentifier(memberName);
+    protected override bool IsValidMemberName(string memberName) =>
+        SyntaxFacts.IsValidIdentifier(memberName);
 }
diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs
@@ -32,7 +32,7 @@ public abstract class SyntaxFacade<TSyntaxKind>
     public abstract bool IsAnyKind(SyntaxNode node, ISet<TSyntaxKind> syntaxKinds);
     public abstract bool IsAnyKind(SyntaxNode node, params TSyntaxKind[] syntaxKinds);
     public abstract bool IsAnyKind(SyntaxTrivia trivia, params TSyntaxKind[] syntaxKinds);
-    public abstract bool IsKnownAttribute(SyntaxNode attribute, KnownType knownAttribute, SemanticModel semanticModel);
+    public abstract bool IsKnownAttributeType(SyntaxNode attribute, KnownType knownType, SemanticModel model);
     public abstract IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node);
     public abstract ImmutableArray<SyntaxNode> AssignmentTargets(SyntaxNode assignment);
     public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment);

diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/DebuggerDisplayUsesExistingMembersBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/DebuggerDisplayUsesExistingMembersBase.cs
@@ -19,6 +19,7 @@
  */
 
 using System.Text.RegularExpressions;
+using Microsoft.CodeAnalysis;
 
 namespace SonarAnalyzer.Rules;
 
@@ -28,10 +29,9 @@ public abstract class DebuggerDisplayUsesExistingMembersBase<TAttributeSyntax, T
 {
     private const string DiagnosticId = "S4545";
 
-    private readonly Regex noQuotesModifierExpressionRegex = new(@",\s*nq\s*$", RegexOptions.None, RegexConstants.DefaultTimeout);
-    private readonly Regex evaluatedExpressionRegex = new(@"\{(?<EvaluatedExpression>[^\}]+)\}", RegexOptions.Multiline, RegexConstants.DefaultTimeout);
+    private readonly Regex evaluatedExpressionRegex = new(@"\{(?<EvaluatedExpression>[^}]+)\}", RegexOptions.Multiline, RegexConstants.DefaultTimeout);
 
-    protected abstract SyntaxNode GetAttributeFormatString(TAttributeSyntax attribute);
+    protected abstract SyntaxNode AttributeFormatString(TAttributeSyntax attribute);
     protected abstract bool IsValidMemberName(string memberName);
 
     protected override string MessageFormat => "'{0}' doesn't exist in this context.";
@@ -43,9 +43,10 @@ public abstract class DebuggerDisplayUsesExistingMembersBase<TAttributeSyntax, T
             c =>
             {
                 var attribute = (TAttributeSyntax)c.Node;
-                if (Language.Syntax.IsKnownAttribute(attribute, KnownType.System_Diagnostics_DebuggerDisplayAttribute, c.SemanticModel)
-                    && GetAttributeFormatString(attribute) is { } formatString
-                    && FirstInvalidMemberName(c, formatString.GetFirstToken().ValueText, attribute) is { } firstInvalidMember)
+                if (Language.Syntax.IsKnownAttributeType(attribute, KnownType.System_Diagnostics_DebuggerDisplayAttribute, c.SemanticModel)
+                    && AttributeFormatString(attribute) is { } formatString
+                    && Language.Syntax.StringValue(formatString, c.SemanticModel) is { } formatStringText
+                    && FirstInvalidMemberName(c, formatStringText, attribute) is { } firstInvalidMember)
                 {
                     c.ReportIssue(Diagnostic.Create(Rule, formatString.GetLocation(), firstInvalidMember));
                 }
@@ -56,34 +57,45 @@ private string FirstInvalidMemberName(SonarSyntaxNodeReportingContext context, s
     {
         try
         {
+            return (attributeSyntax.Parent?.Parent is { } targetSyntax
+                && context.SemanticModel.GetDeclaredSymbol(targetSyntax) is { } targetSymbol
+                && TypeContainingReferencedMembers(targetSymbol) is { } typeSymbol)
+                    ? FirstInvalidMemberName(typeSymbol)
+                    : null;
+        }
+        catch (RegexMatchTimeoutException)
+        {
+            return null;
+        }
+
+        string FirstInvalidMemberName(ITypeSymbol typeSymbol)
+        {
+            var allMembers = typeSymbol
+                .GetSelfAndBaseTypes()
+                .SelectMany(x => x.GetMembers())
+                .Select(x => x.Name)
+                .ToHashSet(Language.NameComparer);
+
             foreach (Match match in evaluatedExpressionRegex.Matches(formatString))
             {
                 if (match.Groups["EvaluatedExpression"] is { Success: true, Value: var evaluatedExpression }
                     && ExtractValidMemberName(evaluatedExpression) is { } memberName
-                    && GetAttributeTarget(attributeSyntax) is { } targetSyntax
-                    && context.SemanticModel.GetDeclaredSymbol(targetSyntax) is { } targetSymbol
-                    && GetTypeContainingReferencedMembers(targetSymbol) is { } typeSymbol
-                    && typeSymbol.GetSelfAndBaseTypes().SelectMany(x => x.GetMembers()).All(x => Language.NameComparer.Compare(x.Name, memberName) != 0))
+                    && !allMembers.Contains(memberName))
                 {
                     return memberName;
                 }
             }
 
             return null;
         }
-        catch (RegexMatchTimeoutException)
-        {
-            return null;
-        }
 
         string ExtractValidMemberName(string evaluatedExpression)
         {
-            var sanitizedExpression = noQuotesModifierExpressionRegex.Replace(evaluatedExpression, string.Empty).Trim();
+            var sanitizedExpression = evaluatedExpression.Split(',')[0].Trim();
             return IsValidMemberName(sanitizedExpression) ? sanitizedExpression : null;
         }
 
-        static SyntaxNode GetAttributeTarget(TAttributeSyntax attributeSyntax) => attributeSyntax.Parent?.Parent;
-
-        static ITypeSymbol GetTypeContainingReferencedMembers(ISymbol symbol) => symbol is ITypeSymbol typeSymbol ? typeSymbol : symbol.ContainingType;
+        static ITypeSymbol TypeContainingReferencedMembers(ISymbol symbol) =>
+            symbol is ITypeSymbol typeSymbol ? typeSymbol : symbol.ContainingType;
     }
 }
diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs
@@ -50,8 +50,8 @@ internal sealed class VisualBasicSyntaxFacade : SyntaxFacade<SyntaxKind>
 
     public override bool IsAnyKind(SyntaxTrivia trivia, params SyntaxKind[] syntaxKinds) => trivia.IsAnyKind(syntaxKinds);
 
-    public override bool IsKnownAttribute(SyntaxNode attribute, KnownType knownAttribute, SemanticModel semanticModel) =>
-        AttributeSyntaxExtensions.IsKnownType(Cast<AttributeSyntax>(attribute), knownAttribute, semanticModel);
+    public override bool IsKnownAttributeType(SyntaxNode attribute, KnownType knownType, SemanticModel model) =>
+        AttributeSyntaxExtensions.IsKnownType(Cast<AttributeSyntax>(attribute), knownType, model);
 
     public override IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node) =>
         node switch

diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/DebuggerDisplayUsesExistingMembers.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/DebuggerDisplayUsesExistingMembers.cs
@@ -25,10 +25,11 @@ public sealed class DebuggerDisplayUsesExistingMembers : DebuggerDisplayUsesExis
 {
     protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
 
-    protected override SyntaxNode GetAttributeFormatString(AttributeSyntax attribute) =>
+    protected override SyntaxNode AttributeFormatString(AttributeSyntax attribute) =>
         attribute.ArgumentList.Arguments.FirstOrDefault() is SimpleArgumentSyntax { Expression: LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } formatString }
             ? formatString
             : null;
 
-    protected override bool IsValidMemberName(string memberName) => SyntaxFacts.IsValidIdentifier(memberName);
+    protected override bool IsValidMemberName(string memberName) =>
+        SyntaxFacts.IsValidIdentifier(memberName);
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/DebuggerDisplayUsesExistingMembersTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/DebuggerDisplayUsesExistingMembersTest.cs
@@ -41,12 +41,6 @@ public class DebuggerDisplayUsesExistingMembersTest
 
 #if NET
 
-    [TestMethod]
-    public void DebuggerDisplayUsesExistingMembers_CSharp9() =>
-        builderCS.AddPaths("DebuggerDisplayUsesExistingMembers.CSharp9.cs")
-            .WithOptions(ParseOptionsHelper.FromCSharp9)
-            .Verify();
-
     [TestMethod]
     public void DebuggerDisplayUsesExistingMembers_CSharp10() =>
         builderCS.AddPaths("DebuggerDisplayUsesExistingMembers.CSharp10.cs")

diff --git a/...ers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp10.cs b/...ers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp10.cs
@@ -6,6 +6,9 @@ public record SomeRecord(int RecordProperty)
 {
     [DebuggerDisplay("{RecordProperty}")] public record struct RecordStruct1(int RecordStructProperty);       // Noncompliant
     [DebuggerDisplay("{RecordStructProperty}")] public record struct RecordStruct2(int RecordStructProperty); // Compliant, RecordStructProperty is a property
+
+    [DebuggerDisplay("{RecordProperty}")] public record NestedRecord1(int NestedRecordProperty);       // Noncompliant
+    [DebuggerDisplay("{NestedRecordProperty}")] public record NestedRecord2(int NestedRecordProperty); // Compliant, NestedRecordProperty is a property
 }
 
 [DebuggerDisplay("{RecordProperty1} bla bla {RecordProperty2}")]
@@ -26,11 +29,22 @@ public class NestedClass2
     }
 }
 
-public class SupportConstantInterpolatedStrings
+public class ConstantInterpolatedStrings
 {
     [DebuggerDisplay($"{{{nameof(SomeProperty)}}}")]
     [DebuggerDisplay($"{{{nameof(NotAProperty)}}}")] // FN: constant interpolated strings not supported
     public int SomeProperty => 1;
 
     public class NotAProperty { }
 }
+
+public interface DefaultInterfaceImplementations
+{
+    [DebuggerDisplay("{OtherProperty}")]
+    [DebuggerDisplay("{OtherPropertyImplemented}")]
+    [DebuggerDisplay("{Nonexistent}")]               // Noncompliant
+    int WithNonexistentProperty => 1;
+
+    string OtherProperty { get; }
+    string OtherPropertyImplemented => "Something";
+}
diff --git a/...ers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp11.cs b/...ers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp11.cs
@@ -1,6 +1,6 @@
 using System.Diagnostics;
 
-class SupportRawStringLiterals
+class RawStringLiterals
 {
     int SomeProperty => 1;
     int SomeField = 2;

diff --git a/...zers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp8.cs b/...zers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp8.cs
@@ -1,7 +1,7 @@
 using System;
 using System.Diagnostics;
 
-public class SupportAccessModifiers
+public class AccessModifiers
 {
     public class BaseClass
     {

diff --git a/...zers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp9.cs b/...zers/tests/SonarAnalyzer.UnitTest/TestCases/DebuggerDisplayUsesExistingMembers.CSharp9.cs