Merge pull request #49717 from jnm2/simplify_interpolation_vb_alignment

'Simplify interpolation' should not be suggested in VB for non-literal alignments
dotnet · Apr 19, 2021 · 497a472 · 497a472
2 parents d7236c6 + 543371b
commit 497a472
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 80 deletions.
diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems
@@ -52,6 +52,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)RemoveUnreachableCode\CSharpRemoveUnreachableCodeDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)RemoveUnreachableCode\RemoveUnreachableCodeHelpers.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyBooleanExpression\CSharpSimplifyConditionalDiagnosticAnalyzer.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\CSharpSimplifyInterpolationHelpers.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\CSharpSimplifyInterpolationDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyLinqExpression\CSharpSimplifyLinqExpressionDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseAutoProperty\CSharpUseAutoPropertyAnalyzer.cs" />

diff --git a/...s/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs b/...s/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs
@@ -4,6 +4,7 @@
 
 #nullable disable
 
+using Microsoft.CodeAnalysis.CSharp.Analyzers.SimplifyInterpolation;
 using Microsoft.CodeAnalysis.CSharp.EmbeddedLanguages.VirtualChars;
 using Microsoft.CodeAnalysis.CSharp.LanguageServices;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -16,12 +17,14 @@ namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
     internal class CSharpSimplifyInterpolationDiagnosticAnalyzer : AbstractSimplifyInterpolationDiagnosticAnalyzer<
-        InterpolationSyntax, ExpressionSyntax, ConditionalExpressionSyntax, ParenthesizedExpressionSyntax>
+        InterpolationSyntax, ExpressionSyntax>
     {
         protected override IVirtualCharService GetVirtualCharService()
             => CSharpVirtualCharService.Instance;
 
         protected override ISyntaxFacts GetSyntaxFacts()
             => CSharpSyntaxFacts.Instance;
+
+        protected override AbstractSimplifyInterpolationHelpers GetHelpers() => CSharpSimplifyInterpolationHelpers.Instance;
     }
 }
diff --git a/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationHelpers.cs b/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationHelpers.cs
@@ -0,0 +1,27 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.SimplifyInterpolation;
+
+namespace Microsoft.CodeAnalysis.CSharp.Analyzers.SimplifyInterpolation
+{
+    internal sealed class CSharpSimplifyInterpolationHelpers : AbstractSimplifyInterpolationHelpers
+    {
+        public static CSharpSimplifyInterpolationHelpers Instance { get; } = new();
+
+        private CSharpSimplifyInterpolationHelpers() { }
+
+        protected override bool PermitNonLiteralAlignmentComponents => true;
+
+        protected override SyntaxNode GetPreservedInterpolationExpressionSyntax(IOperation operation)
+        {
+            return operation.Syntax switch
+            {
+                ConditionalExpressionSyntax { Parent: ParenthesizedExpressionSyntax parent } => parent,
+                var syntax => syntax,
+            };
+        }
+    }
+}
diff --git a/...zers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs b/...zers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs
@@ -8,6 +8,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Text;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Analyzers.SimplifyInterpolation;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.SimplifyInterpolation;
 
@@ -16,15 +17,16 @@ namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
     [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.SimplifyInterpolation), Shared]
     internal class CSharpSimplifyInterpolationCodeFixProvider : AbstractSimplifyInterpolationCodeFixProvider<
         InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax,
-        InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax,
-        ConditionalExpressionSyntax, ParenthesizedExpressionSyntax>
+        InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax>
     {
         [ImportingConstructor]
         [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
         public CSharpSimplifyInterpolationCodeFixProvider()
         {
         }
 
+        protected override AbstractSimplifyInterpolationHelpers GetHelpers() => CSharpSimplifyInterpolationHelpers.Instance;
+
         protected override InterpolationSyntax WithExpression(InterpolationSyntax interpolation, ExpressionSyntax expression)
             => interpolation.WithExpression(expression);
 

diff --git a/src/Analyzers/Core/Analyzers/Analyzers.projitems b/src/Analyzers/Core/Analyzers/Analyzers.projitems
@@ -64,7 +64,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyBooleanExpression\AbstractSimplifyConditionalDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyBooleanExpression\SimplifyBooleanExpressionConstants.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\AbstractSimplifyInterpolationDiagnosticAnalyzer.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\Helpers.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\AbstractSimplifyInterpolationHelpers.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyLinqExpression\AbstractSimplifyLinqExpressionDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseAutoProperty\AbstractUseAutoPropertyAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCoalesceExpression\AbstractUseCoalesceExpressionDiagnosticAnalyzer.cs" />

diff --git a/...s/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs b/...s/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs
@@ -15,13 +15,9 @@ namespace Microsoft.CodeAnalysis.SimplifyInterpolation
 {
     internal abstract class AbstractSimplifyInterpolationDiagnosticAnalyzer<
         TInterpolationSyntax,
-        TExpressionSyntax,
-        TConditionalExpressionSyntax,
-        TParenthesizedExpressionSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer
+        TExpressionSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer
         where TInterpolationSyntax : SyntaxNode
         where TExpressionSyntax : SyntaxNode
-        where TConditionalExpressionSyntax : TExpressionSyntax
-        where TParenthesizedExpressionSyntax : TExpressionSyntax
     {
         protected AbstractSimplifyInterpolationDiagnosticAnalyzer()
            : base(IDEDiagnosticIds.SimplifyInterpolationId,
@@ -37,6 +33,8 @@ protected AbstractSimplifyInterpolationDiagnosticAnalyzer()
 
         protected abstract ISyntaxFacts GetSyntaxFacts();
 
+        protected abstract AbstractSimplifyInterpolationHelpers GetHelpers();
+
         public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
             => DiagnosticAnalyzerCategory.SemanticSpanAnalysis;
 
@@ -63,7 +61,7 @@ private void AnalyzeInterpolation(OperationAnalysisContext context)
                 return;
             }
 
-            Helpers.UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax, TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
+            GetHelpers().UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
                 GetVirtualCharService(), GetSyntaxFacts(), interpolation, out _, out var alignment, out _,
                 out var formatString, out var unnecessaryLocations);
 

diff --git a/...nalyzers/SimplifyInterpolation/Helpers.cs → ...n/AbstractSimplifyInterpolationHelpers.cs b/...nalyzers/SimplifyInterpolation/Helpers.cs → ...n/AbstractSimplifyInterpolationHelpers.cs
@@ -17,28 +17,18 @@
 
 namespace Microsoft.CodeAnalysis.SimplifyInterpolation
 {
-    internal static class Helpers
+    internal abstract class AbstractSimplifyInterpolationHelpers
     {
-        private static SyntaxNode GetPreservedInterpolationExpressionSyntax<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
-            IOperation operation)
-            where TConditionalExpressionSyntax : SyntaxNode
-            where TParenthesizedExpressionSyntax : SyntaxNode
-        {
-            return operation.Syntax switch
-            {
-                TConditionalExpressionSyntax { Parent: TParenthesizedExpressionSyntax parent } => parent,
-                var syntax => syntax,
-            };
-        }
+        protected abstract bool PermitNonLiteralAlignmentComponents { get; }
 
-        public static void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax, TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
+        protected abstract SyntaxNode GetPreservedInterpolationExpressionSyntax(IOperation operation);
+
+        public void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
             IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IInterpolationOperation interpolation,
             out TExpressionSyntax? unwrapped, out TExpressionSyntax? alignment, out bool negate,
             out string? formatString, out ImmutableArray<Location> unnecessaryLocations)
-            where TInterpolationSyntax : SyntaxNode
-            where TExpressionSyntax : SyntaxNode
-            where TConditionalExpressionSyntax : TExpressionSyntax
-            where TParenthesizedExpressionSyntax : TExpressionSyntax
+                where TInterpolationSyntax : SyntaxNode
+                where TExpressionSyntax : SyntaxNode
         {
             alignment = null;
             negate = false;
@@ -49,17 +39,15 @@ public static void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax,
             var expression = Unwrap(interpolation.Expression);
             if (interpolation.Alignment == null)
             {
-                UnwrapAlignmentPadding<TExpressionSyntax, TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
-                    expression, out expression, out alignment, out negate, unnecessarySpans);
+                UnwrapAlignmentPadding(expression, out expression, out alignment, out negate, unnecessarySpans);
             }
 
             if (interpolation.FormatString == null)
             {
-                UnwrapFormatString<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
-                    virtualCharService, syntaxFacts, expression, out expression, out formatString, unnecessarySpans);
+                UnwrapFormatString(virtualCharService, syntaxFacts, expression, out expression, out formatString, unnecessarySpans);
             }
 
-            unwrapped = GetPreservedInterpolationExpressionSyntax<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(expression) as TExpressionSyntax;
+            unwrapped = GetPreservedInterpolationExpressionSyntax(expression) as TExpressionSyntax;
 
             unnecessaryLocations =
                 unnecessarySpans.OrderBy(t => t.Start)
@@ -88,11 +76,9 @@ public static void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax,
             }
         }
 
-        private static void UnwrapFormatString<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
+        private void UnwrapFormatString(
             IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IOperation expression, out IOperation unwrapped,
             out string? formatString, List<TextSpan> unnecessarySpans)
-            where TConditionalExpressionSyntax : SyntaxNode
-            where TParenthesizedExpressionSyntax : SyntaxNode
         {
             Contract.ThrowIfNull(expression.SemanticModel);
 
@@ -111,9 +97,8 @@ private static void UnwrapFormatString<TConditionalExpressionSyntax, TParenthesi
                         unwrapped = invocation.Instance;
                         formatString = value;
 
-                        var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(unwrapped);
                         unnecessarySpans.AddRange(invocation.Syntax.Span
-                            .Subtract(unwrappedSyntax.FullSpan)
+                            .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance).FullSpan)
                             .Subtract(GetSpanWithinLiteralQuotes(virtualCharService, literal.Syntax.GetFirstToken())));
                         return;
                     }
@@ -127,9 +112,8 @@ private static void UnwrapFormatString<TConditionalExpressionSyntax, TParenthesi
                     unwrapped = invocation.Instance;
                     formatString = "";
 
-                    var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(unwrapped);
                     unnecessarySpans.AddRange(invocation.Syntax.Span
-                        .Subtract(unwrappedSyntax.FullSpan));
+                        .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance).FullSpan));
                     return;
                 }
             }
@@ -215,12 +199,10 @@ private static TextSpan GetSpanWithinLiteralQuotes(IVirtualCharService virtualCh
                 : TextSpan.FromBounds(sequence.First().Span.Start, sequence.Last().Span.End);
         }
 
-        private static void UnwrapAlignmentPadding<TExpressionSyntax, TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
+        private void UnwrapAlignmentPadding<TExpressionSyntax>(
             IOperation expression, out IOperation unwrapped,
             out TExpressionSyntax? alignment, out bool negate, List<TextSpan> unnecessarySpans)
             where TExpressionSyntax : SyntaxNode
-            where TConditionalExpressionSyntax : TExpressionSyntax
-            where TParenthesizedExpressionSyntax : TExpressionSyntax
         {
             if (expression is IInvocationOperation invocation &&
                 HasNonImplicitInstance(invocation))
@@ -235,17 +217,19 @@ private static void UnwrapAlignmentPadding<TExpressionSyntax, TConditionalExpres
                             IsSpaceChar(invocation.Arguments[1]))
                         {
                             var alignmentOp = invocation.Arguments[0].Value;
-                            if (alignmentOp != null && alignmentOp.ConstantValue.HasValue)
+
+                            if (PermitNonLiteralAlignmentComponents
+                                ? alignmentOp is { ConstantValue: { HasValue: true } }
+                                : alignmentOp is { Kind: OperationKind.Literal })
                             {
                                 var alignmentSyntax = alignmentOp.Syntax;
 
                                 unwrapped = invocation.Instance!;
                                 alignment = alignmentSyntax as TExpressionSyntax;
                                 negate = targetName == nameof(string.PadRight);
 
-                                var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax<TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(unwrapped);
                                 unnecessarySpans.AddRange(invocation.Syntax.Span
-                                    .Subtract(unwrappedSyntax.FullSpan)
+                                    .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance!).FullSpan)
                                     .Subtract(alignmentSyntax.FullSpan));
                                 return;
                             }

diff --git a/...zers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs b/...zers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs
@@ -23,22 +23,20 @@ internal abstract class AbstractSimplifyInterpolationCodeFixProvider<
         TExpressionSyntax,
         TInterpolationAlignmentClause,
         TInterpolationFormatClause,
-        TInterpolatedStringExpressionSyntax,
-        TConditionalExpressionSyntax,
-        TParenthesizedExpressionSyntax> : SyntaxEditorBasedCodeFixProvider
+        TInterpolatedStringExpressionSyntax> : SyntaxEditorBasedCodeFixProvider
         where TInterpolationSyntax : SyntaxNode
         where TExpressionSyntax : SyntaxNode
         where TInterpolationAlignmentClause : SyntaxNode
         where TInterpolationFormatClause : SyntaxNode
         where TInterpolatedStringExpressionSyntax : TExpressionSyntax
-        where TConditionalExpressionSyntax : TExpressionSyntax
-        where TParenthesizedExpressionSyntax : TExpressionSyntax
     {
         public override ImmutableArray<string> FixableDiagnosticIds { get; } =
             ImmutableArray.Create(IDEDiagnosticIds.SimplifyInterpolationId);
 
         internal override CodeFixCategory CodeFixCategory => CodeFixCategory.CodeStyle;
 
+        protected abstract AbstractSimplifyInterpolationHelpers GetHelpers();
+
         protected abstract TInterpolationSyntax WithExpression(TInterpolationSyntax interpolation, TExpressionSyntax expression);
         protected abstract TInterpolationSyntax WithAlignmentClause(TInterpolationSyntax interpolation, TInterpolationAlignmentClause alignmentClause);
         protected abstract TInterpolationSyntax WithFormatClause(TInterpolationSyntax interpolation, TInterpolationFormatClause? formatClause);
@@ -59,17 +57,19 @@ protected override async Task FixAllAsync(
             var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
             var generator = editor.Generator;
             var generatorInternal = document.GetRequiredLanguageService<SyntaxGeneratorInternal>();
+            var helpers = GetHelpers();
+
             foreach (var diagnostic in diagnostics)
             {
                 var loc = diagnostic.AdditionalLocations[0];
                 var interpolation = semanticModel.GetOperation(loc.FindNode(getInnermostNodeForTie: true, cancellationToken), cancellationToken) as IInterpolationOperation;
                 if (interpolation?.Syntax is TInterpolationSyntax interpolationSyntax &&
                     interpolationSyntax.Parent is TInterpolatedStringExpressionSyntax interpolatedString)
                 {
-                    Helpers.UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax, TConditionalExpressionSyntax, TParenthesizedExpressionSyntax>(
+                    helpers.UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
                         document.GetRequiredLanguageService<IVirtualCharLanguageService>(),
-                        document.GetRequiredLanguageService<ISyntaxFactsService>(), interpolation, out var unwrapped,
-                        out var alignment, out var negate, out var formatString, out _);
+                        document.GetRequiredLanguageService<ISyntaxFactsService>(),
+                        interpolation, out var unwrapped, out var alignment, out var negate, out var formatString, out _);
 
                     if (unwrapped == null)
                         continue;

diff --git a/...sic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb b/...sic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb
@@ -13,11 +13,11 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
     <DiagnosticAnalyzer(LanguageNames.VisualBasic)>
     Friend Class VisualBasicSimplifyInterpolationDiagnosticAnalyzer
-        Inherits AbstractSimplifyInterpolationDiagnosticAnalyzer(Of
-            InterpolationSyntax,
-            ExpressionSyntax,
-            TernaryConditionalExpressionSyntax,
-            ParenthesizedExpressionSyntax)
+        Inherits AbstractSimplifyInterpolationDiagnosticAnalyzer(Of InterpolationSyntax, ExpressionSyntax)
+
+        Protected Overrides Function GetHelpers() As AbstractSimplifyInterpolationHelpers
+            Return VisualBasicSimplifyInterpolationHelpers.Instance
+        End Function
 
         Protected Overrides Function GetVirtualCharService() As IVirtualCharService
             Return VisualBasicVirtualCharService.Instance

diff --git a/...rs/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationHelpers.vb b/...rs/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationHelpers.vb
@@ -0,0 +1,22 @@
+' Licensed to the .NET Foundation under one or more agreements.
+' The .NET Foundation licenses this file to you under the MIT license.
+' See the LICENSE file in the project root for more information.
+
+Imports Microsoft.CodeAnalysis.SimplifyInterpolation
+
+Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
+    Friend NotInheritable Class VisualBasicSimplifyInterpolationHelpers
+        Inherits AbstractSimplifyInterpolationHelpers
+
+        Public Shared ReadOnly Property Instance As New VisualBasicSimplifyInterpolationHelpers
+
+        Private Sub New()
+        End Sub
+
+        Protected Overrides ReadOnly Property PermitNonLiteralAlignmentComponents As Boolean = False
+
+        Protected Overrides Function GetPreservedInterpolationExpressionSyntax(operation As IOperation) As SyntaxNode
+            Return operation.Syntax
+        End Function
+    End Class
+End Namespace