Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDE0083: add support for binary is-expressions and constant is-patterns #54870

Merged
merged 11 commits into from
Jul 29, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp.UsePatternMatching
Expand Down Expand Up @@ -64,27 +65,34 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
if (!styleOption.Value)
return;

// Look for the form: !(x is Y y)
if (!(node is PrefixUnaryExpressionSyntax
// Look for the form: !(...)
if (node is not PrefixUnaryExpressionSyntax(SyntaxKind.LogicalNotExpression)
{
Operand: ParenthesizedExpressionSyntax
{
Expression: IsPatternExpressionSyntax
{
Pattern: DeclarationPatternSyntax,
} isPattern,
},
} notExpression))
Operand: ParenthesizedExpressionSyntax parenthesizedExpression
})
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also need to check that this is actually a ! expression. There are other prefix-unary expressions in the alngauge as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't look like you use 'notExpression'. can you remove if so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use it in ReportDiagnostic call at the end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use it in ReportDiagnostic call at the end.

I think you can use node for that.

return;
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}

// Put a diagnostic with the appropriate severity on `is` keyword.
var isKeywordLocation = parenthesizedExpression.Expression switch
{
// Look for the form: !(x is Y y) and !(x is const)
IsPatternExpressionSyntax { Pattern: DeclarationPatternSyntax or ConstantPatternSyntax } isPattern => isPattern.IsKeyword.GetLocation(),

// Look for the form: !(x is Y)
BinaryExpressionSyntax(SyntaxKind.IsExpression) { Right: TypeSyntax } isExpression => isExpression.OperatorToken.GetLocation(),

_ => null
};

if (isKeywordLocation is null)
return;

syntaxContext.ReportDiagnostic(DiagnosticHelper.Create(
Descriptor,
isPattern.IsKeyword.GetLocation(),
isKeywordLocation,
styleOption.Notification.Severity,
ImmutableArray.Create(notExpression.GetLocation()),
ImmutableArray.Create(node.GetLocation()),
properties: null));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,20 @@ private static void ProcessDiagnostic(

var notExpression = (PrefixUnaryExpressionSyntax)notExpressionLocation.FindNode(getInnermostNodeForTie: true, cancellationToken);
var parenthesizedExpression = (ParenthesizedExpressionSyntax)notExpression.Operand;
var isPattern = (IsPatternExpressionSyntax)parenthesizedExpression.Expression;
var oldExpression = parenthesizedExpression.Expression;

var updatedPattern = oldExpression switch
{
IsPatternExpressionSyntax isPattern => isPattern.WithPattern(UnaryPattern(Token(SyntaxKind.NotKeyword), isPattern.Pattern)),
BinaryExpressionSyntax { Right: TypeSyntax type } binaryIsExpression
=> IsPatternExpression(binaryIsExpression.Left, UnaryPattern(Token(SyntaxKind.NotKeyword), TypePattern(type))),
_ => throw ExceptionUtilities.UnexpectedValue(oldExpression)
};

var updatedPattern = isPattern.WithPattern(UnaryPattern(Token(SyntaxKind.NotKeyword), isPattern.Pattern));
editor.ReplaceNode(
notExpression,
updatedPattern.WithPrependedLeadingTrivia(notExpression.GetLeadingTrivia())
.WithAppendedTrailingTrivia(notExpression.GetTrailingTrivia()));

}

private class MyCodeAction : CustomCodeActions.DocumentChangeAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,66 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.UsePatternMatching

public partial class CSharpUseNotPatternTests
{
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNotPattern)]
[WorkItem(50690, "https://github.com/dotnet/roslyn/issues/50690")]
public async Task BinaryIsExpression()
{
await new VerifyCS.Test
{
TestCode =
@"class C
{
void M(object x)
{
if (!(x [|is|] string))
{
}
}
}",
FixedCode =
@"class C
{
void M(object x)
{
if (x is not string)
{
}
}
}",
LanguageVersion = LanguageVersion.CSharp9,
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNotPattern)]
[WorkItem(50690, "https://github.com/dotnet/roslyn/issues/50690")]
public async Task ConstantPattern()
{
await new VerifyCS.Test
{
TestCode =
@"class C
{
void M(object x)
{
if (!(x [|is|] null))
{
}
}
}",
FixedCode =
@"class C
{
void M(object x)
{
if (x is not null)
{
}
}
}",
LanguageVersion = LanguageVersion.CSharp9,
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNotPattern)]
[WorkItem(46699, "https://github.com/dotnet/roslyn/issues/46699")]
public async Task UseNotPattern()
Expand Down