Revert "Merge analyzer RCS1156 with RCS1113 (fix #650)" (fix #662)

dotnet · Apr 8, 2020 · f28a428 · f28a428
1 parent d2d0c8c
commit f28a428
Show file tree

Hide file tree

Showing 10 changed files with 239 additions and 167 deletions.
diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/BinaryExpressionCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/BinaryExpressionCodeFixProvider.cs
@@ -1,8 +1,8 @@
 // Copyright (c) Josef Pihrt. All rights reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Immutable;
 using System.Composition;
+using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
@@ -33,6 +33,7 @@ public sealed override ImmutableArray<string> FixableDiagnosticIds
                     DiagnosticIdentifiers.UseStringIsNullOrEmptyMethod,
                     DiagnosticIdentifiers.SimplifyCoalesceExpression,
                     DiagnosticIdentifiers.RemoveRedundantAsOperator,
+                    DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString,
                     DiagnosticIdentifiers.UnconstrainedTypeParameterCheckedForNull,
                     DiagnosticIdentifiers.ValueTypeObjectIsNeverEqualToNull,
                     DiagnosticIdentifiers.JoinStringExpressions,
@@ -91,7 +92,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
                         {
                             CodeAction codeAction = CodeAction.Create(
                                 "Use 'string.IsNullOrEmpty' method",
-                                ct => UseStringIsNullOrEmptyMethodAsync(document, binaryExpression, ct),
+                                cancellationToken => UseStringIsNullOrEmptyMethodAsync(document, binaryExpression, cancellationToken),
                                 GetEquivalenceKey(diagnostic));
 
                             context.RegisterCodeFix(codeAction, diagnostic);
@@ -122,6 +123,16 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
                                 cancellationToken => RemoveRedundantAsOperatorRefactoring.RefactorAsync(document, binaryExpression, cancellationToken),
                                 GetEquivalenceKey(diagnostic));
 
+                            context.RegisterCodeFix(codeAction, diagnostic);
+                            break;
+                        }
+                    case DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString:
+                        {
+                            CodeAction codeAction = CodeAction.Create(
+                                "Use string.Length",
+                                cancellationToken => UseStringLengthInsteadOfComparisonWithEmptyStringAsync(document, binaryExpression, cancellationToken),
+                                GetEquivalenceKey(diagnostic));
+
                             context.RegisterCodeFix(codeAction, diagnostic);
                             break;
                         }
@@ -260,50 +271,69 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             }
         }
 
-        private static async Task<Document> UseStringIsNullOrEmptyMethodAsync(
+        private static Task<Document> UseStringIsNullOrEmptyMethodAsync(
             Document document,
             BinaryExpressionSyntax binaryExpression,
             CancellationToken cancellationToken)
         {
-            if (binaryExpression.IsKind(SyntaxKind.EqualsExpression))
-            {
-                SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            NullCheckExpressionInfo nullCheck = SyntaxInfo.NullCheckExpressionInfo(binaryExpression.Left);
 
-                BinaryExpressionInfo binaryExpressionInfo = SyntaxInfo.BinaryExpressionInfo(binaryExpression);
+            ExpressionSyntax newNode = SimpleMemberInvocationExpression(
+                CSharpTypeFactory.StringType(),
+                IdentifierName("IsNullOrEmpty"),
+                Argument(nullCheck.Expression));
 
-                ExpressionSyntax expression = (CSharpUtility.IsEmptyStringExpression(binaryExpressionInfo.Left, semanticModel, cancellationToken))
-                    ? binaryExpressionInfo.Right
-                    : binaryExpressionInfo.Left;
+            if (nullCheck.IsCheckingNotNull)
+                newNode = LogicalNotExpression(newNode);
 
-                ExpressionSyntax newNode = SimpleMemberInvocationExpression(
-                    CSharpTypeFactory.StringType(),
-                    IdentifierName("IsNullOrEmpty"),
-                    Argument(expression));
+            newNode = newNode
+                .WithTriviaFrom(binaryExpression)
+                .WithFormatterAnnotation();
 
-                newNode = newNode
-                    .WithTriviaFrom(binaryExpression)
-                    .WithFormatterAnnotation();
+            return document.ReplaceNodeAsync(binaryExpression, newNode, cancellationToken);
+        }
 
-                return await document.ReplaceNodeAsync(binaryExpression, newNode, cancellationToken).ConfigureAwait(false);
+        private static async Task<Document> UseStringLengthInsteadOfComparisonWithEmptyStringAsync(
+            Document document,
+            BinaryExpressionSyntax binaryExpression,
+            CancellationToken cancellationToken)
+        {
+            SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+
+            ExpressionSyntax left = binaryExpression.Left;
+
+            ExpressionSyntax right = binaryExpression.Right;
+
+            BinaryExpressionSyntax newNode;
+
+            if (CSharpUtility.IsEmptyStringExpression(left, semanticModel, cancellationToken))
+            {
+                newNode = binaryExpression
+                    .WithLeft(NumericLiteralExpression(0))
+                    .WithRight(CreateConditionalAccess(right));
+            }
+            else if (CSharpUtility.IsEmptyStringExpression(right, semanticModel, cancellationToken))
+            {
+                newNode = binaryExpression
+                    .WithLeft(CreateConditionalAccess(left))
+                    .WithRight(NumericLiteralExpression(0));
             }
             else
             {
-                NullCheckExpressionInfo nullCheck = SyntaxInfo.NullCheckExpressionInfo(binaryExpression.Left);
-
-                ExpressionSyntax newNode = SimpleMemberInvocationExpression(
-                    CSharpTypeFactory.StringType(),
-                    IdentifierName("IsNullOrEmpty"),
-                    Argument(nullCheck.Expression));
+                Debug.Fail(binaryExpression.ToString());
+                return document;
+            }
 
-                if (nullCheck.IsCheckingNotNull)
-                    newNode = LogicalNotExpression(newNode);
+            newNode = newNode.WithTriviaFrom(binaryExpression).WithFormatterAnnotation();
 
-                newNode = newNode
-                    .WithTriviaFrom(binaryExpression)
-                    .WithFormatterAnnotation();
+            return await document.ReplaceNodeAsync(binaryExpression, newNode, cancellationToken).ConfigureAwait(false);
+        }
 
-                return await document.ReplaceNodeAsync(binaryExpression, newNode, cancellationToken).ConfigureAwait(false);
-            }
+        private static ConditionalAccessExpressionSyntax CreateConditionalAccess(ExpressionSyntax expression)
+        {
+            return ConditionalAccessExpression(
+                expression.Parenthesize(),
+                MemberBindingExpression(IdentifierName("Length")));
         }
     }
 }
diff --git a/src/Analyzers/Analyzers.xml b/src/Analyzers/Analyzers.xml
@@ -2510,23 +2510,9 @@ public static partial class Foo
 }]]></Before>
         <After><![CDATA[if (string.IsNullOrEmpty(s))
 {
-}]]></After>
-      </Sample>
-      <Sample>
-        <Before><![CDATA[if (s == "") // [|Id|]
-{
-}]]></Before>
-        <After><![CDATA[if (string.IsNullOrEmpty(s))
-{
 }]]></After>
       </Sample>
     </Samples>
-    <Links>
-      <Link>
-        <Url>https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1820</Url>
-        <Text>CA1820: Test for empty strings using string length</Text>
-      </Link>
-    </Links>
   </Analyzer>
   <Analyzer Identifier="RemoveRedundantDelegateCreation">
     <Id>RCS1114</Id>
@@ -3329,7 +3315,7 @@ Foo();]]></After>
       </Sample>
     </Samples>
   </Analyzer>
-  <Analyzer Identifier="UseStringLengthInsteadOfComparisonWithEmptyString" IsObsolete="true">
+  <Analyzer Identifier="UseStringLengthInsteadOfComparisonWithEmptyString">
     <Id>RCS1156</Id>
     <Title>Use string.Length instead of comparison with empty string.</Title>
     <MessageFormat>Use string.Length instead of comparison with empty string.</MessageFormat>
@@ -3348,6 +3334,12 @@ Foo();]]></After>
 }]]></After>
       </Sample>
     </Samples>
+    <Links>
+      <Link>
+        <Url>https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1820</Url>
+        <Text>CA1820: Test for empty strings using string length</Text>
+      </Link>
+    </Links>
   </Analyzer>
   <Analyzer Identifier="CompositeEnumValueContainsUndefinedFlag">
     <Id>RCS1157</Id>

diff --git a/src/Analyzers/CSharp/Analysis/UseStringIsNullOrEmptyMethodAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseStringIsNullOrEmptyMethodAnalyzer.cs
@@ -26,13 +26,10 @@ public override void Initialize(AnalysisContext context)
                 throw new ArgumentNullException(nameof(context));
 
             base.Initialize(context);
-            context.EnableConcurrentExecution();
 
             context.RegisterSyntaxNodeAction(AnalyzeBinaryExpression,
                 SyntaxKind.LogicalOrExpression,
                 SyntaxKind.LogicalAndExpression);
-
-            context.RegisterSyntaxNodeAction(AnalyzeEqualsExpression, SyntaxKind.EqualsExpression);
         }
 
         private static void AnalyzeBinaryExpression(SyntaxNodeAnalysisContext context)
@@ -127,37 +124,5 @@ private static bool SymbolEquals(
             return semanticModel.GetSymbol(expression1, cancellationToken)?
                 .Equals(semanticModel.GetSymbol(expression2, cancellationToken)) == true;
         }
-
-        private static void AnalyzeEqualsExpression(SyntaxNodeAnalysisContext context)
-        {
-            var equalsExpression = (BinaryExpressionSyntax)context.Node;
-
-            if (equalsExpression.ContainsDirectives)
-                return;
-
-            BinaryExpressionInfo equalsExpressionInfo = SyntaxInfo.BinaryExpressionInfo(equalsExpression);
-
-            if (!equalsExpressionInfo.Success)
-                return;
-
-            ExpressionSyntax left = equalsExpressionInfo.Left;
-            ExpressionSyntax right = equalsExpressionInfo.Right;
-
-            SemanticModel semanticModel = context.SemanticModel;
-            CancellationToken cancellationToken = context.CancellationToken;
-
-            if (CSharpUtility.IsEmptyStringExpression(left, semanticModel, cancellationToken))
-            {
-                if (CSharpUtility.IsStringExpression(right, semanticModel, cancellationToken))
-                {
-                    DiagnosticHelpers.ReportDiagnostic(context, DiagnosticDescriptors.UseStringIsNullOrEmptyMethod, equalsExpression);
-                }
-            }
-            else if (CSharpUtility.IsEmptyStringExpression(right, semanticModel, cancellationToken)
-                && CSharpUtility.IsStringExpression(left, semanticModel, cancellationToken))
-            {
-                DiagnosticHelpers.ReportDiagnostic(context, DiagnosticDescriptors.UseStringIsNullOrEmptyMethod, equalsExpression);
-            }
-        }
     }
 }
diff --git a/src/Analyzers/CSharp/Analysis/UseStringLengthInsteadOfComparisonWithEmptyStringAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseStringLengthInsteadOfComparisonWithEmptyStringAnalyzer.cs
@@ -0,0 +1,66 @@
+// Copyright (c) Josef Pihrt. All rights reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Immutable;
+using System.Threading;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Roslynator.CSharp.Syntax;
+
+namespace Roslynator.CSharp.Analysis
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class UseStringLengthInsteadOfComparisonWithEmptyStringAnalyzer : BaseDiagnosticAnalyzer
+    {
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+        {
+            get { return ImmutableArray.Create(DiagnosticDescriptors.UseStringLengthInsteadOfComparisonWithEmptyString); }
+        }
+
+        public override void Initialize(AnalysisContext context)
+        {
+            if (context == null)
+                throw new ArgumentNullException(nameof(context));
+
+            base.Initialize(context);
+
+            context.RegisterSyntaxNodeAction(AnalyzeEqualsExpression, SyntaxKind.EqualsExpression);
+        }
+
+        private static void AnalyzeEqualsExpression(SyntaxNodeAnalysisContext context)
+        {
+            var equalsExpression = (BinaryExpressionSyntax)context.Node;
+
+            if (equalsExpression.ContainsDirectives)
+                return;
+
+            BinaryExpressionInfo equalsExpressionInfo = SyntaxInfo.BinaryExpressionInfo(equalsExpression);
+
+            if (!equalsExpressionInfo.Success)
+                return;
+
+            ExpressionSyntax left = equalsExpressionInfo.Left;
+            ExpressionSyntax right = equalsExpressionInfo.Right;
+
+            SemanticModel semanticModel = context.SemanticModel;
+            CancellationToken cancellationToken = context.CancellationToken;
+
+            if (CSharpUtility.IsEmptyStringExpression(left, semanticModel, cancellationToken))
+            {
+                if (CSharpUtility.IsStringExpression(right, semanticModel, cancellationToken)
+                    && !equalsExpression.IsInExpressionTree(semanticModel, cancellationToken))
+                {
+                    DiagnosticHelpers.ReportDiagnostic(context, DiagnosticDescriptors.UseStringLengthInsteadOfComparisonWithEmptyString, equalsExpression);
+                }
+            }
+            else if (CSharpUtility.IsEmptyStringExpression(right, semanticModel, cancellationToken)
+                && CSharpUtility.IsStringExpression(left, semanticModel, cancellationToken)
+                && !equalsExpression.IsInExpressionTree(semanticModel, cancellationToken))
+            {
+                DiagnosticHelpers.ReportDiagnostic(context, DiagnosticDescriptors.UseStringLengthInsteadOfComparisonWithEmptyString, equalsExpression);
+            }
+        }
+    }
+}
diff --git a/src/Analyzers/CSharp/DiagnosticDescriptors.Deprecated.Generated.cs b/src/Analyzers/CSharp/DiagnosticDescriptors.Deprecated.Generated.cs
@@ -294,18 +294,6 @@ public static partial class DiagnosticDescriptors
             helpLinkUri:        DiagnosticIdentifiers.MemberTypeMustMatchOverriddenMemberType, 
             customTags:         Array.Empty<string>());
 
-        [Obsolete("", error: true)]
-        internal static readonly DiagnosticDescriptor UseStringLengthInsteadOfComparisonWithEmptyString = Factory.Create(
-            id:                 DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString, 
-            title:              "Use string.Length instead of comparison with empty string.", 
-            messageFormat:      "Use string.Length instead of comparison with empty string.", 
-            category:           DiagnosticCategories.Usage, 
-            defaultSeverity:    DiagnosticSeverity.Info, 
-            isEnabledByDefault: true, 
-            description:        null, 
-            helpLinkUri:        DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString, 
-            customTags:         Array.Empty<string>());
-
         [Obsolete("", error: true)]
         internal static readonly DiagnosticDescriptor OverridingMemberCannotChangeAccessModifiers = Factory.Create(
             id:                 DiagnosticIdentifiers.OverridingMemberCannotChangeAccessModifiers, 

diff --git a/src/Analyzers/CSharp/DiagnosticDescriptors.Generated.cs b/src/Analyzers/CSharp/DiagnosticDescriptors.Generated.cs
@@ -1625,6 +1625,18 @@ public static partial class DiagnosticDescriptors
             helpLinkUri:        DiagnosticIdentifiers.UseStringComparison, 
             customTags:         Array.Empty<string>());
 
+        /// <summary>RCS1156</summary>
+        public static readonly DiagnosticDescriptor UseStringLengthInsteadOfComparisonWithEmptyString = Factory.Create(
+            id:                 DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString, 
+            title:              "Use string.Length instead of comparison with empty string.", 
+            messageFormat:      "Use string.Length instead of comparison with empty string.", 
+            category:           DiagnosticCategories.Usage, 
+            defaultSeverity:    DiagnosticSeverity.Info, 
+            isEnabledByDefault: true, 
+            description:        null, 
+            helpLinkUri:        DiagnosticIdentifiers.UseStringLengthInsteadOfComparisonWithEmptyString, 
+            customTags:         Array.Empty<string>());
+
         /// <summary>RCS1157</summary>
         public static readonly DiagnosticDescriptor CompositeEnumValueContainsUndefinedFlag = Factory.Create(
             id:                 DiagnosticIdentifiers.CompositeEnumValueContainsUndefinedFlag, 

diff --git a/src/Analyzers/CSharp/DiagnosticIdentifiers.Deprecated.Generated.cs b/src/Analyzers/CSharp/DiagnosticIdentifiers.Deprecated.Generated.cs
@@ -55,8 +55,6 @@ public static partial class DiagnosticIdentifiers
         [Obsolete("", error: true)]
         public const string MemberTypeMustMatchOverriddenMemberType = "RCS1152";
         [Obsolete("", error: true)]
-        public const string UseStringLengthInsteadOfComparisonWithEmptyString = "RCS1156";
-        [Obsolete("", error: true)]
         public const string OverridingMemberCannotChangeAccessModifiers = "RCS1167";
         [Obsolete("", error: true)]
         public const string CallDebugFailInsteadOfDebugAssert = "RCS1178";

diff --git a/src/Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs b/src/Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs
@@ -139,6 +139,7 @@ public static partial class DiagnosticIdentifiers
         public const string AddEmptyLineAfterClosingBrace = "RCS1153";
         public const string SortEnumMembers = "RCS1154";
         public const string UseStringComparison = "RCS1155";
+        public const string UseStringLengthInsteadOfComparisonWithEmptyString = "RCS1156";
         public const string CompositeEnumValueContainsUndefinedFlag = "RCS1157";
         public const string StaticMemberInGenericTypeShouldUseTypeParameter = "RCS1158";
         public const string UseGenericEventHandler = "RCS1159";