From aa06131305f783f1806a56394d16bd777691a73f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 13 Aug 2020 13:37:15 -0700 Subject: [PATCH] Use C# 'not' pattern when available. --- .../CSharpAsAndNullCheckCodeFixProvider.cs | 46 ++++++++++++++----- .../CSharpAsAndNullCheckTests.cs | 15 +++--- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpAsAndNullCheckCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpAsAndNullCheckCodeFixProvider.cs index 9e980c41fcb..00f54d28ab3 100644 --- a/src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpAsAndNullCheckCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UsePatternMatching/CSharpAsAndNullCheckCodeFixProvider.cs @@ -43,20 +43,23 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) return Task.CompletedTask; } - protected override Task FixAllAsync( + protected override async Task FixAllAsync( Document document, ImmutableArray diagnostics, SyntaxEditor editor, CancellationToken cancellationToken) { using var _1 = PooledHashSet.GetInstance(out var declaratorLocations); using var _2 = PooledHashSet.GetInstance(out var statementParentScopes); + var tree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); + var languageVersion = ((CSharpParseOptions)tree.Options).LanguageVersion; + foreach (var diagnostic in diagnostics) { cancellationToken.ThrowIfCancellationRequested(); if (declaratorLocations.Add(diagnostic.AdditionalLocations[0])) { - AddEdits(editor, diagnostic, RemoveStatement, cancellationToken); + AddEdits(editor, diagnostic, languageVersion, RemoveStatement, cancellationToken); } } @@ -71,7 +74,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) }); } - return Task.CompletedTask; + return; void RemoveStatement(StatementSyntax statement) { @@ -86,6 +89,7 @@ void RemoveStatement(StatementSyntax statement) private static void AddEdits( SyntaxEditor editor, Diagnostic diagnostic, + LanguageVersion languageVersion, Action removeStatement, CancellationToken cancellationToken) { @@ -106,15 +110,8 @@ void RemoveStatement(StatementSyntax statement) var declarationPattern = SyntaxFactory.DeclarationPattern( ((TypeSyntax)asExpression.Right).WithoutTrivia().WithTrailingTrivia(SyntaxFactory.ElasticMarker), SyntaxFactory.SingleVariableDesignation(newIdentifier)); - ExpressionSyntax isExpression = SyntaxFactory.IsPatternExpression(asExpression.Left, declarationPattern); - // We should negate the is-expression if we have something like "x == null" or "x is null" - if (comparison.IsKind(SyntaxKind.EqualsExpression, SyntaxKind.IsPatternExpression)) - { - isExpression = SyntaxFactory.PrefixUnaryExpression( - SyntaxKind.LogicalNotExpression, - isExpression.Parenthesize()); - } + var condition = GetCondition(languageVersion, comparison, asExpression, declarationPattern); if (declarator.Parent is VariableDeclarationSyntax declaration && declaration.Parent is LocalDeclarationStatementSyntax localDeclaration && @@ -134,7 +131,32 @@ void RemoveStatement(StatementSyntax statement) editor.RemoveNode(declarator, SyntaxRemoveOptions.KeepUnbalancedDirectives); } - editor.ReplaceNode(comparison, isExpression.WithTriviaFrom(comparison)); + editor.ReplaceNode(comparison, condition.WithTriviaFrom(comparison)); + } + + private static ExpressionSyntax GetCondition( + LanguageVersion languageVersion, + ExpressionSyntax comparison, + BinaryExpressionSyntax asExpression, + DeclarationPatternSyntax declarationPattern) + { + var isPatternExpression = SyntaxFactory.IsPatternExpression(asExpression.Left, declarationPattern); + + // We should negate the is-expression if we have something like "x == null" or "x is null" + if (!comparison.IsKind(SyntaxKind.EqualsExpression, SyntaxKind.IsPatternExpression)) + return isPatternExpression; + +#if !CODE_STYLE + if (languageVersion >= LanguageVersion.CSharp9) + { + // In C# 9 and higher, convert to `x is not string s`. + return isPatternExpression.WithPattern( + SyntaxFactory.UnaryPattern(SyntaxFactory.Token(SyntaxKind.NotKeyword), isPatternExpression.Pattern)); + } +#endif + + // In C# 8 and lower, convert to `!(x is string s)` + return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, isPatternExpression.Parenthesize()); } private class MyCodeAction : CustomCodeActions.DocumentChangeAction diff --git a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpAsAndNullCheckTests.cs b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpAsAndNullCheckTests.cs index 2e497f46803..131dc677569 100644 --- a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpAsAndNullCheckTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpAsAndNullCheckTests.cs @@ -36,11 +36,14 @@ internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProvider [InlineData("(x = o as string) == null", "!(o is string x)")] [InlineData("null == (x = o as string)", "!(o is string x)")] [InlineData("(x = o as string) is null", "!(o is string x)")] - public async Task InlineTypeCheck1(string input, string output) +#if !CODE_STYLE + [InlineData("x == null", "o is not string x", LanguageVersion.CSharp9)] +#endif + public async Task InlineTypeCheck1(string input, string output, LanguageVersion version = LanguageVersion.CSharp8) { - await TestStatement($"if ({input}) {{ }}", $"if ({output}) {{ }}"); - await TestStatement($"var y = {input};", $"var y = {output};"); - await TestStatement($"return {input};", $"return {output};"); + await TestStatement($"if ({input}) {{ }}", $"if ({output}) {{ }}", version); + await TestStatement($"var y = {input};", $"var y = {output};", version); + await TestStatement($"return {input};", $"return {output};", version); } [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTypeCheck)] @@ -52,7 +55,7 @@ public async Task InlineTypeCheck1(string input, string output) public async Task InlineTypeCheck2(string input, string output) => await TestStatement($"while ({input}) {{ }}", $"while ({output}) {{ }}"); - private async Task TestStatement(string input, string output) + private async Task TestStatement(string input, string output, LanguageVersion version = LanguageVersion.CSharp8) { await TestInRegularAndScript1Async( $@"class C @@ -69,7 +72,7 @@ void M(object o) {{ {output} }} -}}"); +}}", new TestParameters(CSharpParseOptions.Default.WithLanguageVersion(version))); } [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTypeCheck)] -- GitLab