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

New Rule S4545: "DebuggerDisplayAttribute" strings should reference existing members #6728

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions analyzers/rspec/cs/S4545_c#.html
@@ -0,0 +1,23 @@
<p>The <code>DebuggerDisplayAttribute</code> is used to determine how an object is displayed in the debugger window.</p>
<p>The <code>DebuggerDisplayAttribute</code> constructor takes a single argument: the string to be displayed in the value column for instances of the
type. Any text within curly braces is evaluated as the name of a field, property, or method.</p>
<p>Naming a non-existent field, property or method between curly braces will result in a CS0103 error in the debug window when debugging objects.
Although there is no impact on the production code, providing a wrong value can lead to difficulties when debugging the application.</p>
<p>This rule raises an issue when text specified between curly braces refers to members that don’t exist in the current context.</p>
<h2>Noncompliant Code Example</h2>
<pre>
[DebuggerDisplay("Name: {Name}")] // Noncompliant - Name doesn't exist in this context
public class Person
{
public string FullName { get; private set; }
}
</pre>
<h2>Compliant Solution</h2>
<pre>
[DebuggerDisplay("Name: {FullName}")]
public class Person
{
public string FullName { get; private set; }
}
</pre>

15 changes: 15 additions & 0 deletions analyzers/rspec/cs/S4545_c#.json
@@ -0,0 +1,15 @@
{
"title": "\"DebuggerDisplayAttribute\" strings should reference existing members",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-4545",
"sqKey": "S4545",
"scope": "All",
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Expand Up @@ -240,6 +240,7 @@
"S4502",
"S4507",
"S4524",
"S4545",
"S4581",
"S4583",
"S4586",
Expand Down
25 changes: 25 additions & 0 deletions analyzers/rspec/vbnet/S4545_vb.net.html
@@ -0,0 +1,25 @@
<p>The <code>DebuggerDisplayAttribute</code> is used to determine how an object is displayed in the debugger window.</p>
<p>The <code>DebuggerDisplayAttribute</code> constructor takes a single argument: the string to be displayed in the value column for instances of the
type. Any text within curly braces is evaluated as the name of a field, property, or method.</p>
<p>Naming a non-existent field, property or method between curly braces will result in a BC30451 error in the debug window when debugging objects.
Although there is no impact on the production code, providing a wrong value can lead to difficulties when debugging the application.</p>
<p>This rule raises an issue when text specified between curly braces refers to members that don’t exist in the current context.</p>
<h2>Noncompliant Code Example</h2>
<pre>
&lt;DebuggerDisplay("Name: {Name}")&gt; ' Noncompliant - Name doesn't exist in this context
Public Class Person

Public Property FullName As String

End Class
</pre>
<h2>Compliant Solution</h2>
<pre>
&lt;DebuggerDisplay("Name: {FullName}")&gt;
Public Class Person

Public Property FullName As String

End Class
</pre>

15 changes: 15 additions & 0 deletions analyzers/rspec/vbnet/S4545_vb.net.json
@@ -0,0 +1,15 @@
{
"title": "\"DebuggerDisplayAttribute\" strings should reference existing members",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-4545",
"sqKey": "S4545",
"scope": "All",
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/vbnet/Sonar_way_profile.json
Expand Up @@ -107,6 +107,7 @@
"S4423",
"S4428",
"S4507",
"S4545",
"S4581",
"S4583",
"S4586",
Expand Down
Expand Up @@ -50,6 +50,9 @@ internal sealed class CSharpSyntaxFacade : SyntaxFacade<SyntaxKind>

public override bool IsNullLiteral(SyntaxNode node) => node.IsNullLiteral();

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
{
Expand Down
@@ -0,0 +1,35 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class DebuggerDisplayUsesExistingMembers : DebuggerDisplayUsesExistingMembersBase<AttributeSyntax, SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

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);
}
26 changes: 26 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Common/RegexConstants.cs
@@ -0,0 +1,26 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Common;

public static class RegexConstants
{
public static TimeSpan DefaultTimeout => TimeSpan.FromMilliseconds(100);
andrei-epure-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs
Expand Up @@ -32,6 +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 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);
Expand Down
@@ -0,0 +1,101 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;

namespace SonarAnalyzer.Rules;

public abstract class DebuggerDisplayUsesExistingMembersBase<TAttributeSyntax, TSyntaxKind> : SonarDiagnosticAnalyzer<TSyntaxKind>
andrei-epure-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
where TAttributeSyntax : SyntaxNode
where TSyntaxKind : struct
{
private const string DiagnosticId = "S4545";

private readonly Regex evaluatedExpressionRegex = new(@"\{(?<EvaluatedExpression>[^}]+)\}", RegexOptions.Multiline, RegexConstants.DefaultTimeout);

protected abstract SyntaxNode AttributeFormatString(TAttributeSyntax attribute);
protected abstract bool IsValidMemberName(string memberName);

protected override string MessageFormat => "'{0}' doesn't exist in this context.";

protected DebuggerDisplayUsesExistingMembersBase() : base(DiagnosticId) { }

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(Language.GeneratedCodeRecognizer,
c =>
{
var attribute = (TAttributeSyntax)c.Node;
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));
}
},
Language.SyntaxKind.Attribute);

private string FirstInvalidMemberName(SonarSyntaxNodeReportingContext context, string formatString, TAttributeSyntax attributeSyntax)
{
try
{
return (attributeSyntax.Parent?.Parent is { } targetSyntax
andrei-epure-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
&& 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
&& !allMembers.Contains(memberName))
{
return memberName;
}
}

return null;
}

string ExtractValidMemberName(string evaluatedExpression)
{
var sanitizedExpression = evaluatedExpression.Split(',')[0].Trim();
return IsValidMemberName(sanitizedExpression) ? sanitizedExpression : null;
}

static ITypeSymbol TypeContainingReferencedMembers(ISymbol symbol) =>
symbol is ITypeSymbol typeSymbol ? typeSymbol : symbol.ContainingType;
}
}
Expand Up @@ -50,6 +50,9 @@ internal sealed class VisualBasicSyntaxFacade : SyntaxFacade<SyntaxKind>

public override bool IsAnyKind(SyntaxTrivia trivia, params SyntaxKind[] syntaxKinds) => trivia.IsAnyKind(syntaxKinds);

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
{
Expand Down
@@ -0,0 +1,35 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.VisualBasic;

[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class DebuggerDisplayUsesExistingMembers : DebuggerDisplayUsesExistingMembersBase<AttributeSyntax, SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;

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);
}
Expand Up @@ -4469,7 +4469,7 @@ internal static class RuleTypeMappingCS
// ["S4542"],
// ["S4543"],
// ["S4544"],
// ["S4545"],
["S4545"] = "CODE_SMELL",
// ["S4546"],
// ["S4547"],
// ["S4548"],
Expand Down
Expand Up @@ -4469,7 +4469,7 @@ internal static class RuleTypeMappingVB
// ["S4542"],
// ["S4543"],
// ["S4544"],
// ["S4545"],
["S4545"] = "CODE_SMELL",
// ["S4546"],
// ["S4547"],
// ["S4548"],
Expand Down