Skip to content

Commit 35e2554

Browse files
bcollamorebcollamore
authored
Merge branch 'main' into copilot/fix-719
2 parents 64f0987 + d6625fe commit 35e2554

6 files changed

Lines changed: 218 additions & 3 deletions

File tree

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jobs:
1919
- name: Checkout repository
2020
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v3
2121
- name: Initialize CodeQL
22-
uses: github/codeql-action/init@4e828ff8d448a8a6e532957b1811f387a63867e8
22+
uses: github/codeql-action/init@51f77329afa6477de8c49fc9c7046c15b9a4e79d
2323
with:
2424
languages: ${{ matrix.language }}
2525
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -28,7 +28,7 @@ jobs:
2828
# Prefix the list here with "+" to use these queries and those in the config file.
2929
# Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
3030
# queries: security-extended,security-and-quality
31-
uses: github/codeql-action/autobuild@4e828ff8d448a8a6e532957b1811f387a63867e8
31+
uses: github/codeql-action/autobuild@51f77329afa6477de8c49fc9c7046c15b9a4e79d
3232

3333
- name: Perform CodeQL Analysis
34-
uses: github/codeql-action/analyze@4e828ff8d448a8a6e532957b1811f387a63867e8
34+
uses: github/codeql-action/analyze@51f77329afa6477de8c49fc9c7046c15b9a4e79d

‎Documentation/Diagnostics/PH2145.md‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# PH2145: Avoid string.Format in interpolated string
2+
3+
| Property | Value |
4+
|--|--|
5+
| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
6+
| Diagnostic ID | PH2145 |
7+
| Category | [Maintainability](../Maintainability.md) |
8+
| Analyzer | [AvoidStringFormatInInterpolatedStringAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidStringFormatInInterpolatedStringAnalyzer.cs)
9+
| CodeFix | No |
10+
| Severity | Info |
11+
| Enabled By Default | No |
12+
13+
## Introduction
14+
15+
Using `string.Format` within interpolated strings is redundant and can be simplified by using direct interpolation.
16+
17+
## How to solve
18+
19+
Replace `string.Format` calls inside interpolated strings with direct interpolation of the variables.
20+
21+
## Example
22+
23+
Code that triggers a diagnostic:
24+
``` cs
25+
class BadExample
26+
{
27+
public void BadMethod()
28+
{
29+
var firstName = "John";
30+
var lastName = "Doe";
31+
var result = $"Hello {string.Format("{0} {1}", firstName, lastName)}";
32+
33+
var name = "John";
34+
var age = 30;
35+
var message = $"User info: {string.Format("Name: {0}, Age: {1}", name, age)}";
36+
}
37+
}
38+
```
39+
40+
And the simplified replacement:
41+
``` cs
42+
class GoodExample
43+
{
44+
public void GoodMethod()
45+
{
46+
var firstName = "John";
47+
var lastName = "Doe";
48+
var result = $"Hello {firstName} {lastName}";
49+
50+
var name = "John";
51+
var age = 30;
52+
var message = $"User info: Name: {name}, Age: {age}";
53+
}
54+
}
55+
```
56+
57+
## Configuration
58+
59+
This analyzer is disabled by default. The general ways of [suppressing](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings) diagnostics apply.

‎Philips.CodeAnalysis.Common/DiagnosticId.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ public enum DiagnosticId
137137
AvoidEmptyRegions = 2141,
138138
AvoidCastToString = 2142,
139139
AvoidAssemblyGetEntryAssembly = 2143,
140+
AvoidStringFormatInInterpolatedString = 2145,
140141
AvoidVariableNamedUnderscore = 2147,
141142
}
142143
}

‎Philips.CodeAnalysis.Common/SingleDiagnosticAnalyzer{TNode,TSyntaxNodeAction}.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ protected virtual SyntaxKind GetSyntaxKind()
8787
nameof(InterfaceDeclarationSyntax) => SyntaxKind.InterfaceDeclaration,
8888
nameof(AccessorDeclarationSyntax) => SyntaxKind.SetAccessorDeclaration,
8989
nameof(CatchClauseSyntax) => SyntaxKind.CatchClause,
90+
nameof(InterpolatedStringExpressionSyntax) => SyntaxKind.InterpolatedStringExpression,
9091
_ => SyntaxKind.None,
9192
};
9293
}

‎Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidStringFormatInInterpolatedStringAnalyzer.cs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// © 2024 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
using Philips.CodeAnalysis.Common;
7+
8+
namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability
9+
{
10+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
11+
public class AvoidStringFormatInInterpolatedStringAnalyzer : SingleDiagnosticAnalyzer<InterpolatedStringExpressionSyntax, AvoidStringFormatInInterpolatedStringSyntaxNodeAction>
12+
{
13+
private const string Title = @"Avoid string.Format in interpolated string";
14+
private const string MessageFormat = @"Consider simplifying string.Format usage in interpolated string";
15+
private const string Description = @"Using string.Format within interpolated strings can be simplified by using direct interpolation";
16+
17+
public AvoidStringFormatInInterpolatedStringAnalyzer()
18+
: base(DiagnosticId.AvoidStringFormatInInterpolatedString, Title, MessageFormat, Description, Categories.Maintainability, isEnabled: false)
19+
{ }
20+
}
21+
22+
public class AvoidStringFormatInInterpolatedStringSyntaxNodeAction : SyntaxNodeAction<InterpolatedStringExpressionSyntax>
23+
{
24+
public override void Analyze()
25+
{
26+
// Look for interpolated string expressions
27+
foreach (InterpolatedStringContentSyntax content in Node.Contents)
28+
{
29+
if (content is InterpolationSyntax interpolation && ContainsStringFormatCall(interpolation.Expression))
30+
{
31+
Location location = interpolation.GetLocation();
32+
ReportDiagnostic(location);
33+
}
34+
}
35+
}
36+
37+
private bool ContainsStringFormatCall(ExpressionSyntax expression)
38+
{
39+
// Check if this is a direct string.Format call
40+
if (expression is InvocationExpressionSyntax invocation)
41+
{
42+
// Use semantic analysis to properly identify string.Format
43+
SymbolInfo symbolInfo = Context.SemanticModel.GetSymbolInfo(invocation);
44+
if (symbolInfo.Symbol is IMethodSymbol methodSymbol)
45+
{
46+
return methodSymbol.Name == "Format" &&
47+
methodSymbol.ContainingType?.SpecialType == SpecialType.System_String;
48+
}
49+
}
50+
51+
return false;
52+
}
53+
}
54+
}

‎Philips.CodeAnalysis.Test/Maintainability/Maintainability/AvoidStringFormatInInterpolatedStringAnalyzerTest.cs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// © 2024 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using System.Threading.Tasks;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
using Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability;
7+
using Philips.CodeAnalysis.Test.Helpers;
8+
using Philips.CodeAnalysis.Test.Verifiers;
9+
10+
namespace Philips.CodeAnalysis.Test.Maintainability.Maintainability
11+
{
12+
[TestClass]
13+
public class AvoidStringFormatInInterpolatedStringAnalyzerTest : DiagnosticVerifier
14+
{
15+
protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
16+
{
17+
return new AvoidStringFormatInInterpolatedStringAnalyzer();
18+
}
19+
20+
[TestMethod]
21+
[TestCategory(TestDefinitions.UnitTests)]
22+
public async Task StringFormatInInterpolatedStringTriggersWarning()
23+
{
24+
var code = @"
25+
using System;
26+
27+
class Test
28+
{
29+
public void Method()
30+
{
31+
var firstName = ""John"";
32+
var lastName = ""Doe"";
33+
var result = $""Hello {string.Format(""{0} {1}"", firstName, lastName)}"";
34+
}
35+
}
36+
";
37+
await VerifyDiagnostic(code).ConfigureAwait(false);
38+
}
39+
40+
[TestMethod]
41+
[TestCategory(TestDefinitions.UnitTests)]
42+
public async Task StringFormatWithMultipleArgumentsTriggersWarning()
43+
{
44+
var code = @"
45+
using System;
46+
47+
class Test
48+
{
49+
public void Method()
50+
{
51+
var name = ""John"";
52+
var age = 30;
53+
var result = $""User info: {string.Format(""Name: {0}, Age: {1}"", name, age)}"";
54+
}
55+
}
56+
";
57+
await VerifyDiagnostic(code).ConfigureAwait(false);
58+
}
59+
60+
[TestMethod]
61+
[TestCategory(TestDefinitions.UnitTests)]
62+
public async Task InterpolatedStringWithoutStringFormatIsOk()
63+
{
64+
var code = @"
65+
using System;
66+
67+
class Test
68+
{
69+
public void Method()
70+
{
71+
var firstName = ""John"";
72+
var lastName = ""Doe"";
73+
var result = $""Hello {firstName} {lastName}"";
74+
}
75+
}
76+
";
77+
await VerifySuccessfulCompilation(code).ConfigureAwait(false);
78+
}
79+
80+
[TestMethod]
81+
[TestCategory(TestDefinitions.UnitTests)]
82+
public async Task StringFormatOutsideInterpolatedStringIsOk()
83+
{
84+
var code = @"
85+
using System;
86+
87+
class Test
88+
{
89+
public void Method()
90+
{
91+
var firstName = ""John"";
92+
var lastName = ""Doe"";
93+
var result = string.Format(""Hello {0} {1}"", firstName, lastName);
94+
}
95+
}
96+
";
97+
await VerifySuccessfulCompilation(code).ConfigureAwait(false);
98+
}
99+
}
100+
}

0 commit comments

Comments
 (0)