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

reacheight · 2021-07-16T18:55:02Z

…tern analyzer and code fix

CyrusNajmabadi · 2021-07-17T08:13:10Z

src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpUseNotPatternDiagnosticAnalyzer.cs

+                    Operand: ParenthesizedExpressionSyntax
+                    {
+                        Expression: BinaryExpressionSyntax
+                        {
+                            Right: TypeSyntax
+                        } isExpression,
+                    },
+                } notIsExpression && isExpression.IsKind(SyntaxKind.IsExpression))


Suggested change

Operand: ParenthesizedExpressionSyntax

{

Expression: BinaryExpressionSyntax

{

Right: TypeSyntax

} isExpression,

},

} notIsExpression && isExpression.IsKind(SyntaxKind.IsExpression))

Operand: ParenthesizedExpressionSyntax { Expression: BinaryExpressionSyntax(SyntaxKind.IsExpression) },

})

I don't understand how Expression: BinaryExpressionSyntax(SyntaxKind.IsExpression) is supposed to work.

The only way to check if it's an is-expression using pattern matching I came up with is

Expression: BinaryExpressionSyntax { RawKind: (int)SyntaxKind.IsExpression }

but I'm not sure how it looks.

We should have a deconstruct member here to allow a positional pattern to match type and kind. I'm not in front of the computer, but I can try to find later.

Or maybe @alrz or @Youssef1313 know where it is?

roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxNodeExtensions.cs

Lines 24 to 27 in 5c707a8

public static void Deconstruct(this SyntaxNode node, out SyntaxKind kind)

{

kind = node.Kind();

}

Related PR: #53663

src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpUseNotPatternDiagnosticAnalyzer.cs

CyrusNajmabadi · 2021-07-17T08:14:11Z

src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpUseNotPatternCodeFixProvider.cs

+                {
+                    Right: TypeSyntax type
+                } binaryIsExpression => IsPatternExpression(binaryIsExpression.Left, UnaryPattern(Token(SyntaxKind.NotKeyword), TypePattern(type))),
+                _ => null


how does this case get hit?

It's required for successful CI build.

@reacheight You can use throw ExceptionUtilities.Unreachable; instead.

CyrusNajmabadi · 2021-07-17T08:14:28Z

src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpUseNotPatternCodeFixProvider.cs

+                BinaryExpressionSyntax
+                {
+                    Right: TypeSyntax type
+                } binaryIsExpression => IsPatternExpression(binaryIsExpression.Left, UnaryPattern(Token(SyntaxKind.NotKeyword), TypePattern(type))),


Suggested change

BinaryExpressionSyntax

{

Right: TypeSyntax type

} binaryIsExpression => IsPatternExpression(binaryIsExpression.Left, UnaryPattern(Token(SyntaxKind.NotKeyword), TypePattern(type))),

BinaryExpressionSyntax { Right: TypeSyntax type } binaryIsExpression

=> IsPatternExpression(binaryIsExpression.Left, UnaryPattern(Token(SyntaxKind.NotKeyword), TypePattern(type))),

CyrusNajmabadi · 2021-07-17T08:16:57Z

src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpUseNotPatternDiagnosticAnalyzer.cs

-            // Look for the form: !(x is Y y)
-            if (!(node is PrefixUnaryExpressionSyntax
+            // Look for the form: !(x is Y y) and !(x is const)
+            if (node is PrefixUnaryExpressionSyntax


i'm not a huge fan of all the redundancy here. i would just make this a simple first check that checks for a !(...) and bails otherwise. then we can just have the simple dedicated code that checks the inner expression.

CyrusNajmabadi

A few changes to make. Let us know if you'd like to do this. if you don't have time, we're happy to take over this PR for you. Thanks!

reacheight · 2021-07-17T08:44:19Z

Thanks! Yes, I'd like to get this done.

CyrusNajmabadi · 2021-07-17T16:21:49Z

Awesome!

CyrusNajmabadi · 2021-07-19T16:17:28Z

src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpUseNotPatternDiagnosticAnalyzer.cs

-                    },
-                } notExpression))
+                    Operand: ParenthesizedExpressionSyntax parenthesizedExpression
+                } notExpression)
            {


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

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

I use it in ReportDiagnostic call at the end.

I use it in ReportDiagnostic call at the end.

I think you can use node for that.

CyrusNajmabadi · 2021-07-19T16:18:37Z

src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpUseNotPatternCodeFixProvider.cs

+                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.Unreachable


Suggested change

_ => throw ExceptionUtilities.Unreachable

_ => throw ExceptionUtilities.UnexpectedValue(oldExpression)

CyrusNajmabadi

lgtm with the suggestions made.

CyrusNajmabadi · 2021-07-19T17:20:13Z

Awesome. Thanks!

reacheight added 2 commits July 16, 2021 21:46

Add binary is-expression and constant is-pattern support in UseNotPat…

65faabb

…tern analyzer and code fix

Fix indentation

c30c9cc

dotnet-issue-labeler bot added the Area-IDE label Jul 16, 2021

CyrusNajmabadi reviewed Jul 17, 2021

View reviewed changes

src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpUseNotPatternDiagnosticAnalyzer.cs Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 17, 2021

View reviewed changes

CyrusNajmabadi requested changes Jul 17, 2021

View reviewed changes

reacheight added 3 commits July 18, 2021 11:33

Refactoring and review fixes

3befdf0

Add default case for switch expression

d71ac40

Throw in unreachable switch case

0004b65

CyrusNajmabadi reviewed Jul 19, 2021

View reviewed changes

CyrusNajmabadi approved these changes Jul 19, 2021

View reviewed changes

reacheight added 2 commits July 19, 2021 19:45

Correct check for not-expression and better exception

6d5dd88

Remove unnecessary variable

854bc01

CyrusNajmabadi enabled auto-merge July 19, 2021 17:20

reacheight added 4 commits July 20, 2021 09:34

Merge branch 'main' into extend-use-not-pattern

5297328

Merge branch 'main' into extend-use-not-pattern

1ede65d

Merge branch 'main' into extend-use-not-pattern

28a18b4

Merge branch 'main' into extend-use-not-pattern

2e738e7

CyrusNajmabadi merged commit 8f8f95e into dotnet:main Jul 29, 2021

ghost added this to the Next milestone Jul 29, 2021

jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jul 29, 2021

dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

reacheight commented Jul 16, 2021

CyrusNajmabadi Jul 17, 2021

reacheight Jul 17, 2021

CyrusNajmabadi Jul 17, 2021

Youssef1313 Jul 17, 2021 •

edited

Loading

CyrusNajmabadi Jul 17, 2021

reacheight Jul 18, 2021

Youssef1313 Jul 18, 2021

CyrusNajmabadi Jul 17, 2021

CyrusNajmabadi Jul 17, 2021

CyrusNajmabadi left a comment

reacheight commented Jul 17, 2021

CyrusNajmabadi commented Jul 17, 2021

CyrusNajmabadi Jul 19, 2021

CyrusNajmabadi Jul 19, 2021

reacheight Jul 19, 2021

Youssef1313 Jul 19, 2021

CyrusNajmabadi Jul 19, 2021

CyrusNajmabadi left a comment

CyrusNajmabadi commented Jul 19, 2021

	public static void Deconstruct(this SyntaxNode node, out SyntaxKind kind)
	{
	kind = node.Kind();
	}

	_ => throw ExceptionUtilities.Unreachable
	_ => throw ExceptionUtilities.UnexpectedValue(oldExpression)

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

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

Conversation

reacheight commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Youssef1313 Jul 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

reacheight commented Jul 17, 2021

CyrusNajmabadi commented Jul 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Jul 19, 2021

Youssef1313 Jul 17, 2021 •

edited

Loading