Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fading span for unnecessary using #59276

Merged
merged 6 commits into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageServices;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.RemoveUnnecessaryImports;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -27,6 +29,9 @@ internal sealed class CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer :
protected override LocalizableString GetTitleAndMessageFormatForClassificationIdDescriptor()
=> s_TitleAndMessageFormat;

protected override ISyntaxFacts SyntaxFacts
=> CSharpSyntaxFacts.Instance;

// C# has no need to do any merging of using statements. Only VB needs to
// merge import clauses to an import statement if it all the import clauses
// are unnecessary.
Expand All @@ -39,6 +44,10 @@ protected override IUnnecessaryImportsProvider UnnecessaryImportsProvider
protected override bool IsRegularCommentOrDocComment(SyntaxTrivia trivia)
=> trivia.IsRegularComment() || trivia.IsDocComment();

protected override SyntaxToken? TryGetLastToken(SyntaxNode node)
// No special behavior needed for C#
=> null;

protected override IEnumerable<TextSpan> GetFixableDiagnosticSpans(
IEnumerable<SyntaxNode> nodes, SyntaxTree tree, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.RemoveUnnecessaryImports;
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Testing;
using Roslyn.Test.Utilities;
using Xunit;
using VerifyCS = Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions.CSharpCodeFixVerifier<
Microsoft.CodeAnalysis.CSharp.RemoveUnnecessaryImports.CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer,
Microsoft.CodeAnalysis.CSharp.RemoveUnnecessaryImports.CSharpRemoveUnnecessaryImportsCodeFixProvider>;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnnecessaryImports
{
using VerifyCS = CSharpCodeFixVerifier<
CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer,
CSharpRemoveUnnecessaryImportsCodeFixProvider>;

public class RemoveUnnecessaryImportsTests
{
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)]
Expand Down Expand Up @@ -785,7 +788,7 @@ static void Main(string[] args)
public async Task TestComments8718()
{
await VerifyCS.VerifyCodeFixAsync(
@"[|using Goo; {|IDE0005:using System.Collections.Generic;|} /*comment*/ using Goo2;|]
@"[|using Goo; {|IDE0005:using System.Collections.Generic; /*comment*/|} using Goo2;|]

class Program
{
Expand Down Expand Up @@ -843,7 +846,7 @@ public async Task TestComments()
await VerifyCS.VerifyCodeFixAsync(
@"//c1
/*c2*/
[|{|IDE0005:using/*c3*/ System/*c4*/;|}|] //c5
{|IDE0005:[|using/*c3*/ System/*c4*/;|] //c5|}
//c6

class Program
Expand Down Expand Up @@ -1133,7 +1136,7 @@ static class Goo
public async Task TestRemoveTrailingComment()
{
await VerifyCS.VerifyCodeFixAsync(
@"[|{|IDE0005:using System.Collections.Generic;|}|] // comment
@"{|IDE0005:[|using System.Collections.Generic;|] // comment|}

class Program
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.LanguageServices;

#if CODE_STYLE
using OptionSet = Microsoft.CodeAnalysis.Diagnostics.AnalyzerConfigOptions;
Expand Down Expand Up @@ -93,11 +94,14 @@ protected AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer()
#pragma warning restore RS0030 // Do not used banned APIs
}

protected abstract ISyntaxFacts SyntaxFacts { get; }
protected abstract LocalizableString GetTitleAndMessageFormatForClassificationIdDescriptor();
protected abstract ImmutableArray<SyntaxNode> MergeImports(ImmutableArray<SyntaxNode> unnecessaryImports);
protected abstract bool IsRegularCommentOrDocComment(SyntaxTrivia trivia);
protected abstract IUnnecessaryImportsProvider UnnecessaryImportsProvider { get; }

protected abstract SyntaxToken? TryGetLastToken(SyntaxNode node);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
get
Expand Down Expand Up @@ -150,8 +154,7 @@ private void AnalyzeSemanticModel(SemanticModelAnalysisContext context)
descriptor = fadeOut ? _unnecessaryClassificationIdDescriptor : _classificationIdDescriptor;
}

var getLastTokenFunc = GetLastTokenDelegateForContiguousSpans();
var contiguousSpans = unnecessaryImports.GetContiguousSpans(getLastTokenFunc);
var contiguousSpans = GetContiguousSpans(unnecessaryImports);
var diagnostics =
CreateClassificationDiagnostics(contiguousSpans, tree, descriptor, cancellationToken).Concat(
CreateFixableDiagnostics(unnecessaryImports, tree, cancellationToken));
Expand All @@ -172,8 +175,56 @@ static bool ShouldFade(AnalyzerOptions options, SyntaxTree tree, string language
}
}

protected virtual Func<SyntaxNode, SyntaxToken>? GetLastTokenDelegateForContiguousSpans()
=> null;
private IEnumerable<TextSpan> GetContiguousSpans(ImmutableArray<SyntaxNode> nodes)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a move from an extension method to the single feature that used it.

{
var syntaxFacts = this.SyntaxFacts;
(SyntaxNode node, TextSpan textSpan)? previous = null;

// Sort the nodes in source location order.
foreach (var node in nodes.OrderBy(n => n.SpanStart))
{
TextSpan textSpan;
var nodeEnd = GetEnd(node);
if (previous == null)
{
textSpan = TextSpan.FromBounds(node.Span.Start, nodeEnd);
}
else
{
var lastToken = TryGetLastToken(previous.Value.node) ?? previous.Value.node.GetLastToken();
if (lastToken.GetNextToken(includeDirectives: true) == node.GetFirstToken())
{
// Expand the span
textSpan = TextSpan.FromBounds(previous.Value.textSpan.Start, nodeEnd);
}
else
{
// Return the last span, and start a new one
yield return previous.Value.textSpan;
textSpan = TextSpan.FromBounds(node.Span.Start, nodeEnd);
}
}

previous = (node, textSpan);
}

if (previous.HasValue)
yield return previous.Value.textSpan;

yield break;

int GetEnd(SyntaxNode node)
{
var end = node.Span.End;
foreach (var trivia in node.GetTrailingTrivia())
{
if (syntaxFacts.IsRegularComment(trivia))
end = trivia.Span.End;
}

return end;
}
}

// Create one diagnostic for each unnecessary span that will be classified as Unnecessary
private static IEnumerable<Diagnostic> CreateClassificationDiagnostics(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ Imports System.Collections.Immutable
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.LanguageServices
Imports Microsoft.CodeAnalysis.PooledObjects
Imports Microsoft.CodeAnalysis.RemoveUnnecessaryImports
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.LanguageServices
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports
Expand All @@ -19,15 +21,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports
Private Shared ReadOnly s_TitleAndMessageFormat As LocalizableString =
New LocalizableResourceString(NameOf(VisualBasicAnalyzersResources.Imports_statement_is_unnecessary), VisualBasicAnalyzersResources.ResourceManager, GetType(VisualBasicAnalyzersResources))

Protected Overrides ReadOnly Property SyntaxFacts As ISyntaxFacts = VisualBasicSyntaxFacts.Instance

Protected Overrides Function GetTitleAndMessageFormatForClassificationIdDescriptor() As LocalizableString
Return s_TitleAndMessageFormat
End Function

Protected Overrides ReadOnly Property UnnecessaryImportsProvider As IUnnecessaryImportsProvider
Get
Return VisualBasicUnnecessaryImportsProvider.Instance
End Get
End Property
Protected Overrides ReadOnly Property UnnecessaryImportsProvider As IUnnecessaryImportsProvider = VisualBasicUnnecessaryImportsProvider.Instance

Protected Overrides Function IsRegularCommentOrDocComment(trivia As SyntaxTrivia) As Boolean
Return trivia.IsRegularOrDocComment()
Expand Down Expand Up @@ -60,13 +60,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports
Return SpecializedCollections.SingletonEnumerable(tree.GetCompilationUnitRoot().Imports.GetContainedSpan())
End Function

Protected Overrides Function GetLastTokenDelegateForContiguousSpans() As Func(Of SyntaxNode, SyntaxToken)
Return Function(n)
Dim lastToken = n.GetLastToken()
Return If(lastToken.GetNextToken().Kind = SyntaxKind.CommaToken,
lastToken.GetNextToken(),
lastToken)
End Function
Protected Overrides Function TryGetLastToken(node As SyntaxNode) As SyntaxToken?
Dim lastToken = node.GetLastToken()
Dim nextToken = lastToken.GetNextToken()
Return If(nextToken.Kind = SyntaxKind.CommaToken, nextToken, lastToken)
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ protected override bool IsValidContext(int position, CSharpSyntaxContext context
{
var syntaxTree = context.SyntaxTree;

// TODO(cyrusn): lambda/anon methods can have out/ref parameters
return
context.TargetToken.IsTypeParameterVarianceContext() ||
IsOutParameterModifierContext(position, context) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,44 +326,6 @@ public static TextSpan GetContainedSpan(this IEnumerable<SyntaxNode> nodes)
return fullSpan;
}

public static IEnumerable<TextSpan> GetContiguousSpans(
this IEnumerable<SyntaxNode> nodes, Func<SyntaxNode, SyntaxToken>? getLastToken = null)
{
(SyntaxNode node, TextSpan textSpan)? previous = null;

// Sort the nodes in source location order.
foreach (var node in nodes.OrderBy(n => n.SpanStart))
{
TextSpan textSpan;
if (previous == null)
{
textSpan = node.Span;
}
else
{
var lastToken = getLastToken?.Invoke(previous.Value.node) ?? previous.Value.node.GetLastToken();
if (lastToken.GetNextToken(includeDirectives: true) == node.GetFirstToken())
{
// Expand the span
textSpan = TextSpan.FromBounds(previous.Value.textSpan.Start, node.Span.End);
}
else
{
// Return the last span, and start a new one
yield return previous.Value.textSpan;
textSpan = node.Span;
}
}

previous = (node, textSpan);
}

if (previous.HasValue)
{
yield return previous.Value.textSpan;
}
}

public static bool OverlapsHiddenPosition(this SyntaxNode node, CancellationToken cancellationToken)
=> node.OverlapsHiddenPosition(node.Span, cancellationToken);

Expand Down