CR feedback

src/EditorFeatures/CSharpTest/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzerTests.cs

@@ -49,14 +49,15 @@ class C
         [InlineData("i is C c")]
         [InlineData("!(i > 0)")]
         [InlineData("o != null")]
+        [InlineData("!(o is C c)")]
         [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsUsePatternCombinators)]
         public async Task TestMissingOnExpression(string expression)
         {
             await TestMissingAsync(s_initialMarkup.Replace("EXPRESSION", expression));
         }
 
-        [InlineData("!(o is C c)", "o is not C c")]
         [InlineData("!(o is C)", "o is not C")]
+        [InlineData("!(o is C _)", "o is not C _")]
         [InlineData("i == 1 || 2 == i", "i is 1 or 2")]
         [InlineData("i != 1 || 2 != i", "i is not 1 or not 2")]
         [InlineData("i != 1 && 2 != i", "i is not (1 or 2)")]

src/Features/CSharp/Portable/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzer.cs

@@ -4,8 +4,10 @@
 
 #nullable enable
 
+using System.Linq;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
+using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 
@@ -57,18 +59,18 @@ protected override void InitializeWorker(AnalysisContext context)
                 ArrowExpressionClauseSyntax n => n.Expression,
                 AssignmentExpressionSyntax n => n.Right,
                 LambdaExpressionSyntax n => n.ExpressionBody,
-                ArgumentSyntax n when n.GetRefKind() == RefKind.None => n.Expression,
+                ArgumentSyntax n when n.RefKindKeyword.IsMissing => n.Expression,
                 _ => null,
             };
 
         private void AnalyzeNode(SyntaxNodeAnalysisContext context)
         {
             // TODO need an option for user to disable the feature
-
-            // TODO need to check language version >= C# 9.0
-
             var parentNode = context.Node;
 
+            if (!((CSharpParseOptions)parentNode.SyntaxTree.Options).LanguageVersion.IsCSharp9OrAbove())
+                return;
+
             var expression = GetExpression(parentNode);
             if (expression is null)
                 return;
@@ -84,6 +86,9 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context)
             if (IsTrivial(pattern))
                 return;
 
+            if (HasIllegalPatternVariables(pattern))
+                return;
+
             context.ReportDiagnostic(DiagnosticHelper.Create(
                 Descriptor,
                 location: expression.GetLocation(),
@@ -93,6 +98,26 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context)
                 messageArgs: null));
         }
 
+        private static bool HasIllegalPatternVariables(AnalyzedPattern pattern, bool permitDesignations = true)
+        {
+            switch (pattern)
+            {
+                case Not p:
+                    return HasIllegalPatternVariables(p.Pattern, permitDesignations: false);
+                case Binary p:
+                    if (p.IsDisjunctive)
+                        permitDesignations = false;
+                    return HasIllegalPatternVariables(p.Left, permitDesignations) ||
+                           HasIllegalPatternVariables(p.Right, permitDesignations);
+                case Source p when !permitDesignations:
+                    return p.PatternSyntax.DescendantNodes()
+                        .OfType<SingleVariableDesignationSyntax>()
+                        .Any(variable => !variable.Identifier.IsMissing);
+                default:
+                    return false;
+            }
+        }
+
         private static bool IsTrivial(AnalyzedPattern pattern)
             => pattern switch
             {