IDE0054: Fix compound assignment increment in expression context #54324
Conversation
src/Analyzers/CSharp/Tests/UseCompoundAssignment/UseCompoundAssignmentTests.cs
Show resolved
Hide resolved
| return Increment((TExpressionSyntax)leftOfAssign.WithoutTrailingTrivia(), postfix: syntaxFacts.IsStatement(currentAssignment.Parent)); | ||
| } | ||
|
|
||
| if (diagnostic.Properties.ContainsKey(UseCompoundAssignmentUtilities.Decrement)) | ||
| { | ||
| return Decrement((TExpressionSyntax)leftOfAssign); | ||
| return Decrement((TExpressionSyntax)leftOfAssign.WithoutTrailingTrivia(), postfix: syntaxFacts.IsStatement(currentAssignment.Parent)); |
There was a problem hiding this comment.
syntaxFacts.IsStatement seems legit to address #53969 (comment), but maybe there is a better way to detect
if the result of the assignment is used
There is also a lot of repetition, but I don't see a simple way to avoid that.
There was a problem hiding this comment.
What about switch (x = x + 1)? The assignment parent is a statement, but it should be prefix. Am I missing something?
There was a problem hiding this comment.
Yes, you are right. Maybe IsSimpleAssignmentStatement is better.
There was a problem hiding this comment.
I'm not sure. But things I'd make sure they are not broken:
switch(...)return ...yield return ...while (...)var x = ...var x = (...)bool_expr ? ... : ...MethodCall(...)
There was a problem hiding this comment.
Instead of doing syntax analysis here, could we do that by data flow analysis?
| { | ||
| return SyntaxFactory.PostfixUnaryExpression(SyntaxKind.PostIncrementExpression, left); | ||
| } | ||
| protected override ExpressionSyntax Increment(ExpressionSyntax left, bool postfix) |
There was a problem hiding this comment.
nit: I'd call the parameter "operand" instead of left.
| @@ -1087,11 +1087,85 @@ void M() | |||
| { | |||
| void M() | |||
| { | |||
| for (int i = 0; i < 10; i++) | |||
| for (int i = 0; i < 10; ++i) | |||
There was a problem hiding this comment.
would prefer this not change.
|
Are you ok with me pushing a couple of changes here? Tahnks! |
| /// <param name="startValue">C#: null | VB: 0</param> | ||
| /// <param name="endCondition">C#: i < 10 | VB: 10</param> | ||
| /// <param name="increments">C#: i++ | VB: Step 1</param> | ||
| void GetPartsOfForStatement(SyntaxNode? node, out SyntaxNode? variableDeclaration, out SyntaxNode? startValue, out SyntaxNode? endCondition, out ImmutableArray<SyntaxNode> increments); |
There was a problem hiding this comment.
I thought the for loops of VB and C# are close enough for a "GetPartsOf" attempt, but they turned out to be quite different. Maybe it's better to create something like IsPartOfForLoopIncrementor in AbstractUseCompoundAssignmentCodeFixProvider or ISyntaxFacts.
Sure. |
|
thanks! |
ghost
added this to the 
jinujoseph
added
the
Community
Fix #53969