WiseTechGlobal / WTG.Analyzers

Analyzers from WiseTech Global to enforce our styles, behaviours, and prevent common mistakes.
Other
15 stars 3 forks source link

Try to avoid CS0201 when collapsing away and/or assignments. #192

Closed brian-reichle closed 1 year ago

brian-reichle commented 1 year ago

fixes #191

When collapsing an and/or assignment to just the LHS, we mark it as "weak". When visiting an ExpressionStatement, we can detect the weak expression and return the "discardable" empty statement. There is already logic to dissolve the discardable empty statement if it happens to land in a BlockStatement.

yaakov-h commented 1 year ago

What happens to trivia, e.g. foo |= false; // a comment?

brian-reichle commented 1 year ago

When we replace a node with another node (possibly a nested node) we typically copy across the leading and trailing trivia. This means that the outer trivia is retained but inner trivia is lost.

foo |= /* stuff */ false; // a comment -> ; // a comment

If that empty statement is dissolved into a BlockStatement, we simply delete it, so the trivia is lost.

{
    // Comment A
    foo |= /* stuff */ false; // Comment B
    // Comment C
}

would become:

{
    // Comment C
}

It's probably safe to say that // Comment B should be removed. Whether or not // Comment A should be removed is probably dependent on what the comment is saying. I don't think there is any approach we can take that will be correct in all cases. We want it to be as close to correct as we can get in as many cases as we can, but people should really be reviewing the changes made and correcting it as necessary.

brian-reichle commented 1 year ago

Perhaps it would make sense to prepend the leading trivia of the removed statement onto the leading trivia of the following statement, with an elastic end-of-line between them.

yaakov-h commented 1 year ago

except for whitespace trivia, I guess?