'Remove unnecessary cast' adds unnecessary parentheses

[Neme12]

Inside a switch case:

switch (true)
{
    case (bool)default(bool):
}

switch (true)
{
    case (default(bool)):
}

inside a when clause:

switch (true)
{
    case true when (bool)default(bool):
}

switch (true)
{
    case true when (default(bool)):
}

inside an is pattern expression:

if (true is (bool)default(bool)) ;

if (true is (default(bool))) ;

Tagging @CyrusNajmabadi who's working on a code fix specifically to remove unnecessary parentheses. The problem is inside ParenthesizedExpressionSyntaxExtensions.CanRemoveParentheses. The first two cases should be trivial to fix but that's not the case inside is, where associativity plays a role. For example it's important to add tests so that parentheses are not removed here:

if (true is (true == true)) ;

[CyrusNajmabadi]

The first two cases should be trivial to fix but that's not the case inside is, where associativity plays a role. For example it's important to add tests so that parentheses are not removed here:

This should already be checked in my PR. precedence is different here, so we would consider those parens necessary.

[CyrusNajmabadi]

Tagging @CyrusNajmabadi who's working on a code fix specifically to remove unnecessary parentheses. The problem is inside ParenthesizedExpressionSyntaxExtensions.CanRemoveParentheses. The first two cases should be trivial to fix

I can do this if you'd like. But i don't want to step on your toes if you wanted to take this.

[Neme12]

I was going to go after this but then realized you might have fixed the issue inside is to take care of precedence since you changed a lot in that method. I'd prefer if you took care of this and added tests for these cases to tests for your analyzer and not to 'Remove unnecessary cast', since you can cover all of the cases in there.

[Neme12]

So if I was going to do this I would potentially have to duplicate some of your logic and step on your toes.

[Neme12]

A fix for this will break a few tests I added though: #25538 (comment) that currently expect these extra parens. Depending on which PR gets merged first, soebody will have to take care of that later.

But as I sad I'd be happy if you fixed this because your PR has a lot more to do with unnecessary parentheses than mine does.

[CyrusNajmabadi]

No prob. I'll make the change. Whoever goes in second will just do the fixup.

[CyrusNajmabadi]

@Neme12 Have made the changes here: #21316 Want to take a look?

[Neme12]

@CyrusNajmabadi My PR was merged so you might want to fix some of those tests in your branch. Thanks for fixing this.

[CyrusNajmabadi]

Thanks!

[Neme12]

This is fixed in 15.8 preview 2. @CyrusNajmabadi I think you fixed this with #26523 right?
@jinujoseph This can be closed