From 4365779fe54547a53ae7fd10945ac750c63ad09d Mon Sep 17 00:00:00 2001 From: jnm2 Date: Sun, 29 Nov 2020 14:04:04 -0500 Subject: [PATCH] Use extra generic type parameters and apply C#-specific knowledge to all langs instead of using inheritance --- .../Analyzers/CSharpAnalyzers.projitems | 1 - .../SimplifyInterpolation/CSharpHelpers.cs | 21 --------- ...SimplifyInterpolationDiagnosticAnalyzer.cs | 5 +- ...arpSimplifyInterpolationCodeFixProvider.cs | 6 +-- ...SimplifyInterpolationDiagnosticAnalyzer.cs | 10 ++-- .../SimplifyInterpolation/Helpers.cs | 46 +++++++++++++------ ...actSimplifyInterpolationCodeFixProvider.cs | 12 ++--- ...SimplifyInterpolationDiagnosticAnalyzer.vb | 6 ++- ...sicSimplifyInterpolationCodeFixProvider.vb | 3 +- 9 files changed, 54 insertions(+), 56 deletions(-) delete mode 100644 src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpHelpers.cs diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index d99869203931f..3ac1b308bbb27 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -47,7 +47,6 @@ - diff --git a/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpHelpers.cs b/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpHelpers.cs deleted file mode 100644 index 08bfaf0a38490..0000000000000 --- a/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpHelpers.cs +++ /dev/null @@ -1,21 +0,0 @@ -// 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 CSharpHelpers : Helpers - { - protected override SyntaxNode GetPreservedInterpolationExpressionSyntax(IOperation operation) - { - return base.GetPreservedInterpolationExpressionSyntax(operation) switch - { - ConditionalExpressionSyntax { Parent: ParenthesizedExpressionSyntax parent } => parent, - var syntax => syntax, - }; - } - } -} diff --git a/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs index dc1eba07b6358..6484ab2763622 100644 --- a/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs @@ -4,7 +4,6 @@ #nullable disable -using Microsoft.CodeAnalysis.CSharp.Analyzers.SimplifyInterpolation; using Microsoft.CodeAnalysis.CSharp.EmbeddedLanguages.VirtualChars; using Microsoft.CodeAnalysis.CSharp.LanguageServices; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -17,14 +16,12 @@ namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation { [DiagnosticAnalyzer(LanguageNames.CSharp)] internal class CSharpSimplifyInterpolationDiagnosticAnalyzer : AbstractSimplifyInterpolationDiagnosticAnalyzer< - InterpolationSyntax, ExpressionSyntax> + InterpolationSyntax, ExpressionSyntax, ConditionalExpressionSyntax, ParenthesizedExpressionSyntax> { protected override IVirtualCharService GetVirtualCharService() => CSharpVirtualCharService.Instance; protected override ISyntaxFacts GetSyntaxFacts() => CSharpSyntaxFacts.Instance; - - protected override Helpers GetHelpers() => new CSharpHelpers(); } } diff --git a/src/Analyzers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs index 9f2d7e5f51ea8..bcf1edb3b4caa 100644 --- a/src/Analyzers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs @@ -8,7 +8,6 @@ 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; @@ -17,7 +16,8 @@ namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation [ExportCodeFixProvider(LanguageNames.CSharp), Shared] internal class CSharpSimplifyInterpolationCodeFixProvider : AbstractSimplifyInterpolationCodeFixProvider< InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax, - InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax> + InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax, + ConditionalExpressionSyntax, ParenthesizedExpressionSyntax> { [ImportingConstructor] [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")] @@ -25,8 +25,6 @@ public CSharpSimplifyInterpolationCodeFixProvider() { } - protected override Helpers GetHelpers() => new CSharpHelpers(); - protected override InterpolationSyntax WithExpression(InterpolationSyntax interpolation, ExpressionSyntax expression) => interpolation.WithExpression(expression); diff --git a/src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs index 5868280a3ed14..0f7cbe60530e3 100644 --- a/src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs @@ -15,9 +15,13 @@ namespace Microsoft.CodeAnalysis.SimplifyInterpolation { internal abstract class AbstractSimplifyInterpolationDiagnosticAnalyzer< TInterpolationSyntax, - TExpressionSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer + TExpressionSyntax, + TConditionalExpressionSyntax, + TParenthesizedExpressionSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TInterpolationSyntax : SyntaxNode where TExpressionSyntax : SyntaxNode + where TConditionalExpressionSyntax : TExpressionSyntax + where TParenthesizedExpressionSyntax : TExpressionSyntax { protected AbstractSimplifyInterpolationDiagnosticAnalyzer() : base(IDEDiagnosticIds.SimplifyInterpolationId, @@ -32,8 +36,6 @@ protected AbstractSimplifyInterpolationDiagnosticAnalyzer() protected abstract ISyntaxFacts GetSyntaxFacts(); - protected virtual Helpers GetHelpers() => new(); - public override DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SemanticSpanAnalysis; @@ -60,7 +62,7 @@ private void AnalyzeInterpolation(OperationAnalysisContext context) return; } - GetHelpers().UnwrapInterpolation( + Helpers.UnwrapInterpolation( GetVirtualCharService(), GetSyntaxFacts(), interpolation, out _, out var alignment, out _, out var formatString, out var unnecessaryLocations); diff --git a/src/Analyzers/Core/Analyzers/SimplifyInterpolation/Helpers.cs b/src/Analyzers/Core/Analyzers/SimplifyInterpolation/Helpers.cs index 1a586156fb7bf..7a81b6da5b00b 100644 --- a/src/Analyzers/Core/Analyzers/SimplifyInterpolation/Helpers.cs +++ b/src/Analyzers/Core/Analyzers/SimplifyInterpolation/Helpers.cs @@ -14,19 +14,28 @@ namespace Microsoft.CodeAnalysis.SimplifyInterpolation { - internal class Helpers + internal static class Helpers { - protected virtual SyntaxNode GetPreservedInterpolationExpressionSyntax(IOperation operation) + private static SyntaxNode GetPreservedInterpolationExpressionSyntax( + IOperation operation) + where TConditionalExpressionSyntax : SyntaxNode + where TParenthesizedExpressionSyntax : SyntaxNode { - return operation.Syntax; + return operation.Syntax switch + { + TConditionalExpressionSyntax { Parent: TParenthesizedExpressionSyntax parent } => parent, + var syntax => syntax, + }; } - public void UnwrapInterpolation( + public static void UnwrapInterpolation( IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IInterpolationOperation interpolation, out TExpressionSyntax? unwrapped, out TExpressionSyntax? alignment, out bool negate, out string? formatString, out ImmutableArray unnecessaryLocations) - where TInterpolationSyntax : SyntaxNode - where TExpressionSyntax : SyntaxNode + where TInterpolationSyntax : SyntaxNode + where TExpressionSyntax : SyntaxNode + where TConditionalExpressionSyntax : TExpressionSyntax + where TParenthesizedExpressionSyntax : TExpressionSyntax { alignment = null; negate = false; @@ -37,15 +46,17 @@ public void UnwrapInterpolation( var expression = Unwrap(interpolation.Expression); if (interpolation.Alignment == null) { - UnwrapAlignmentPadding(expression, out expression, out alignment, out negate, unnecessarySpans); + UnwrapAlignmentPadding( + expression, out expression, out alignment, out negate, unnecessarySpans); } if (interpolation.FormatString == null) { - UnwrapFormatString(virtualCharService, syntaxFacts, expression, out expression, out formatString, unnecessarySpans); + UnwrapFormatString( + virtualCharService, syntaxFacts, expression, out expression, out formatString, unnecessarySpans); } - unwrapped = GetPreservedInterpolationExpressionSyntax(expression) as TExpressionSyntax; + unwrapped = GetPreservedInterpolationExpressionSyntax(expression) as TExpressionSyntax; unnecessaryLocations = unnecessarySpans.OrderBy(t => t.Start) @@ -70,9 +81,11 @@ private static IOperation Unwrap(IOperation expression) } } - private void UnwrapFormatString( + private static void UnwrapFormatString( IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IOperation expression, out IOperation unwrapped, out string? formatString, List unnecessarySpans) + where TConditionalExpressionSyntax : SyntaxNode + where TParenthesizedExpressionSyntax : SyntaxNode { if (expression is IInvocationOperation { TargetMethod: { Name: nameof(ToString) } } invocation && HasNonImplicitInstance(invocation) && @@ -87,8 +100,9 @@ private void UnwrapFormatString( unwrapped = invocation.Instance; formatString = value; + var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax(unwrapped); unnecessarySpans.AddRange(invocation.Syntax.Span - .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance).FullSpan) + .Subtract(unwrappedSyntax.FullSpan) .Subtract(GetSpanWithinLiteralQuotes(virtualCharService, literal.Syntax.GetFirstToken()))); return; } @@ -107,8 +121,9 @@ private void UnwrapFormatString( unwrapped = invocation.Instance; formatString = ""; + var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax(unwrapped); unnecessarySpans.AddRange(invocation.Syntax.Span - .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance).FullSpan)); + .Subtract(unwrappedSyntax.FullSpan)); return; } } @@ -125,10 +140,12 @@ private static TextSpan GetSpanWithinLiteralQuotes(IVirtualCharService virtualCh : TextSpan.FromBounds(sequence.First().Span.Start, sequence.Last().Span.End); } - private void UnwrapAlignmentPadding( + private static void UnwrapAlignmentPadding( IOperation expression, out IOperation unwrapped, out TExpressionSyntax? alignment, out bool negate, List unnecessarySpans) where TExpressionSyntax : SyntaxNode + where TConditionalExpressionSyntax : TExpressionSyntax + where TParenthesizedExpressionSyntax : TExpressionSyntax { if (expression is IInvocationOperation invocation && HasNonImplicitInstance(invocation)) @@ -151,8 +168,9 @@ private void UnwrapAlignmentPadding( alignment = alignmentSyntax as TExpressionSyntax; negate = targetName == nameof(string.PadRight); + var unwrappedSyntax = GetPreservedInterpolationExpressionSyntax(unwrapped); unnecessarySpans.AddRange(invocation.Syntax.Span - .Subtract(GetPreservedInterpolationExpressionSyntax(invocation.Instance).FullSpan) + .Subtract(unwrappedSyntax.FullSpan) .Subtract(alignmentSyntax.FullSpan)); return; } diff --git a/src/Analyzers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs index ceb97dad6cd0f..1c214116cd044 100644 --- a/src/Analyzers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs @@ -23,20 +23,22 @@ internal abstract class AbstractSimplifyInterpolationCodeFixProvider< TExpressionSyntax, TInterpolationAlignmentClause, TInterpolationFormatClause, - TInterpolatedStringExpressionSyntax> : SyntaxEditorBasedCodeFixProvider + TInterpolatedStringExpressionSyntax, + TConditionalExpressionSyntax, + TParenthesizedExpressionSyntax> : 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 FixableDiagnosticIds { get; } = ImmutableArray.Create(IDEDiagnosticIds.SimplifyInterpolationId); internal override CodeFixCategory CodeFixCategory => CodeFixCategory.CodeStyle; - protected virtual Helpers GetHelpers() => new(); - protected abstract TInterpolationSyntax WithExpression(TInterpolationSyntax interpolation, TExpressionSyntax expression); protected abstract TInterpolationSyntax WithAlignmentClause(TInterpolationSyntax interpolation, TInterpolationAlignmentClause alignmentClause); protected abstract TInterpolationSyntax WithFormatClause(TInterpolationSyntax interpolation, TInterpolationFormatClause? formatClause); @@ -57,8 +59,6 @@ protected override async Task FixAllAsync( var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var generator = editor.Generator; var generatorInternal = document.GetRequiredLanguageService(); - var helpers = GetHelpers(); - foreach (var diagnostic in diagnostics) { var loc = diagnostic.AdditionalLocations[0]; @@ -66,7 +66,7 @@ protected override async Task FixAllAsync( if (interpolation?.Syntax is TInterpolationSyntax interpolationSyntax && interpolationSyntax.Parent is TInterpolatedStringExpressionSyntax interpolatedString) { - helpers.UnwrapInterpolation( + Helpers.UnwrapInterpolation( document.GetRequiredLanguageService(), document.GetRequiredLanguageService(), interpolation, out var unwrapped, out var alignment, out var negate, out var formatString, out _); diff --git a/src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb index d9db8dc8cb266..b88ee1f3db3ae 100644 --- a/src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb @@ -13,7 +13,11 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation Friend Class VisualBasicSimplifyInterpolationDiagnosticAnalyzer - Inherits AbstractSimplifyInterpolationDiagnosticAnalyzer(Of InterpolationSyntax, ExpressionSyntax) + Inherits AbstractSimplifyInterpolationDiagnosticAnalyzer(Of + InterpolationSyntax, + ExpressionSyntax, + TernaryConditionalExpressionSyntax, + ParenthesizedExpressionSyntax) Protected Overrides Function GetVirtualCharService() As IVirtualCharService Return VisualBasicVirtualCharService.Instance diff --git a/src/Analyzers/VisualBasic/CodeFixes/SimplifyInterpolation/VisualBasicSimplifyInterpolationCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/SimplifyInterpolation/VisualBasicSimplifyInterpolationCodeFixProvider.vb index 7fec35d7ca1e9..ebbdd5712afe9 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/SimplifyInterpolation/VisualBasicSimplifyInterpolationCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/SimplifyInterpolation/VisualBasicSimplifyInterpolationCodeFixProvider.vb @@ -13,7 +13,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation Friend Class VisualBasicSimplifyInterpolationCodeFixProvider Inherits AbstractSimplifyInterpolationCodeFixProvider(Of InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax, - InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax) + InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax, + TernaryConditionalExpressionSyntax, ParenthesizedExpressionSyntax)