Skip to content

Commit d022e89

Browse files
Copilotbcollamore
andauthored
fix: make PH2153 fixer less aggressive for standalone statements
Agent-Logs-Url: https://github.com/philips-software/roslyn-analyzers/sessions/438a660c-8a91-4654-b7e0-aab21311773f Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent 222eee1 commit d022e89

2 files changed

Lines changed: 80 additions & 5 deletions

File tree

Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidUnusedToStringCodeFixProvider.cs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,41 @@ protected override async Task<Document> ApplyFix(Document document, InvocationEx
2828
return document;
2929
}
3030

31-
// Case 1: Standalone expression statement (expr.ToString();) - remove the entire statement
31+
ExpressionSyntax receiver = memberAccess.Expression;
32+
33+
// Case 1: Standalone expression statement (expr.ToString();)
3234
if (node.Parent is ExpressionStatementSyntax expressionStatement)
3335
{
34-
rootNode = rootNode.RemoveNode(expressionStatement, SyntaxRemoveOptions.KeepDirectives);
36+
// Only remove the entire statement when the receiver is known to be side-effect free:
37+
// local variable, parameter, or 'this'.
38+
if (IsSideEffectFree(receiver))
39+
{
40+
rootNode = rootNode.RemoveNode(expressionStatement, SyntaxRemoveOptions.KeepDirectives);
41+
}
42+
else
43+
{
44+
// Keep the receiver expression as a standalone statement to preserve side effects.
45+
ExpressionStatementSyntax newStatement = expressionStatement
46+
.WithExpression(receiver.WithoutTrivia())
47+
.WithLeadingTrivia(expressionStatement.GetLeadingTrivia())
48+
.WithTrailingTrivia(expressionStatement.GetTrailingTrivia())
49+
.WithAdditionalAnnotations(Formatter.Annotation);
50+
rootNode = rootNode.ReplaceNode(expressionStatement, newStatement);
51+
}
52+
3553
return document.WithSyntaxRoot(rootNode);
3654
}
3755

3856
// Case 2: Assignment to discard (_ = expr.ToString()) - replace with just the expression
39-
ExpressionSyntax expression = memberAccess.Expression;
4057
SyntaxTriviaList trivia = node.GetLeadingTrivia();
41-
ExpressionSyntax newExpression = expression.WithLeadingTrivia(trivia).WithAdditionalAnnotations(Formatter.Annotation);
58+
ExpressionSyntax newExpression = receiver.WithLeadingTrivia(trivia).WithAdditionalAnnotations(Formatter.Annotation);
4259
rootNode = rootNode.ReplaceNode(node, newExpression);
4360
return document.WithSyntaxRoot(rootNode);
4461
}
62+
63+
private static bool IsSideEffectFree(ExpressionSyntax expression)
64+
{
65+
return expression is IdentifierNameSyntax or ThisExpressionSyntax;
66+
}
4567
}
4668
}

Philips.CodeAnalysis.Test/Maintainability/Maintainability/AvoidUnusedToStringAnalyzerTest.cs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,59 @@ private void SomeMethod(string value) { }
182182

183183
[TestMethod]
184184
[TestCategory(TestDefinitions.UnitTests)]
185-
public async Task FixStandaloneToStringRemovesEntireStatement()
185+
public async Task FixStandaloneLocalVariableToStringRemovesEntireStatement()
186+
{
187+
var test = @"
188+
class TestClass
189+
{
190+
public void TestMethod()
191+
{
192+
var node = ""test"";
193+
node.ToString();
194+
}
195+
}";
196+
197+
var fixedCode = @"
198+
class TestClass
199+
{
200+
public void TestMethod()
201+
{
202+
var node = ""test"";
203+
}
204+
}";
205+
206+
await VerifyDiagnostic(test).ConfigureAwait(false);
207+
await VerifyFix(test, fixedCode, shouldAllowNewCompilerDiagnostics: true).ConfigureAwait(false);
208+
}
209+
210+
[TestMethod]
211+
[TestCategory(TestDefinitions.UnitTests)]
212+
public async Task FixStandaloneThisToStringRemovesEntireStatement()
213+
{
214+
var test = @"
215+
class TestClass
216+
{
217+
public void TestMethod()
218+
{
219+
this.ToString();
220+
}
221+
}";
222+
223+
var fixedCode = @"
224+
class TestClass
225+
{
226+
public void TestMethod()
227+
{
228+
}
229+
}";
230+
231+
await VerifyDiagnostic(test).ConfigureAwait(false);
232+
await VerifyFix(test, fixedCode).ConfigureAwait(false);
233+
}
234+
235+
[TestMethod]
236+
[TestCategory(TestDefinitions.UnitTests)]
237+
public async Task FixStandaloneMethodCallToStringKeepsMethodCall()
186238
{
187239
var test = @"
188240
class TestClass
@@ -200,6 +252,7 @@ class TestClass
200252
public string GetNode() => ""test"";
201253
public void TestMethod()
202254
{
255+
GetNode();
203256
}
204257
}";
205258

0 commit comments

Comments
 (0)