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.

Sign up for GitHub

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

Jump to bottom

Fix fading span for unnecessary using #59276

Merged
merged 6 commits into from
Feb 7, 2022
Merged

Fix fading span for unnecessary using #59276

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f63cde
Fix fading span for unnecessary using
CyrusNajmabadi Feb 4, 2022
0938dad
Merge remote-tracking branch 'upstream/main' into importFadingSpan
CyrusNajmabadi Feb 4, 2022
7046e13
Refine
CyrusNajmabadi Feb 4, 2022
cf12db0
Revert
CyrusNajmabadi Feb 4, 2022
1199752
Merge branch 'refLambdaParam' into importFadingSpan
CyrusNajmabadi Feb 4, 2022
d66c454
Merge remote-tracking branch 'upstream/main' into importFadingSpan
CyrusNajmabadi Feb 5, 2022
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
4 changes: 4 additions & 0 deletions ...rp/Analyzers/RemoveUnnecessaryImports/CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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
41 changes: 37 additions & 4 deletions .../Analyzers/RemoveUnnecessaryImports/AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ protected AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer()
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 +152,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 +173,40 @@ 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.

{
(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 = TextSpan.FromBounds(node.Span.Start, node.FullSpan.End);
}
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, node.FullSpan.End);
}
else
{
// Return the last span, and start a new one
yield return previous.Value.textSpan;
textSpan = TextSpan.FromBounds(node.Span.Start, node.FullSpan.End);
}
}

previous = (node, textSpan);
}

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

// Create one diagnostic for each unnecessary span that will be classified as Unnecessary
private static IEnumerable<Diagnostic> CreateClassificationDiagnostics(
Expand Down
11 changes: 4 additions & 7 deletions ...alyzers/RemoveUnnecessaryImports/VisualBasicRemoveUnnecessaryImportsDiagnosticAnalyzer.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
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
149 changes: 126 additions & 23 deletions src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp;
Expand All @@ -16,7 +15,6 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics;
using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.QuickInfo;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Extensions;
using Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles;
Expand All @@ -41,7 +39,7 @@ public class ErrorSquiggleProducerTests
public async Task ErrorTagGeneratedForError()
{
var spans = await GetTagSpansAsync("class C {");
Assert.Equal(1, spans.Count());
Assert.Equal(1, spans.Length);

var firstSpan = spans.First();
Assert.Equal(PredefinedErrorTypeNames.SyntaxError, firstSpan.Tag.ErrorType);
Expand All @@ -51,7 +49,7 @@ public async Task ErrorTagGeneratedForError()
public async Task ErrorTagGeneratedForWarning()
{
var spans = await GetTagSpansAsync("class C { long x = 5l; }");
Assert.Equal(1, spans.Count());
Assert.Equal(1, spans.Length);
Assert.Equal(PredefinedErrorTypeNames.Warning, spans.First().Tag.ErrorType);
}

Expand All @@ -77,7 +75,7 @@ void Test()
using var workspace = TestWorkspace.Create(workspaceXml);
var spans = (await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetDiagnosticsAndErrorSpans(workspace)).Item2;

Assert.Equal(1, spans.Count());
Assert.Equal(1, spans.Length);
Assert.Equal(PredefinedErrorTypeNames.SyntaxError, spans.First().Tag.ErrorType);
}

Expand Down Expand Up @@ -150,7 +148,7 @@ void Test()
Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType);
ToolTipAssert.EqualContent(expectedToolTip, first.Tag.ToolTipContent);
Assert.Equal(40, first.Span.Start);
Assert.Equal(25, first.Span.Length);
Assert.Equal(27, first.Span.Length);

expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
Expand All @@ -163,7 +161,7 @@ void Test()
Assert.Equal(PredefinedErrorTypeNames.Suggestion, second.Tag.ErrorType);
ToolTipAssert.EqualContent(expectedToolTip, second.Tag.ToolTipContent);
Assert.Equal(82, second.Span.Start);
Assert.Equal(60, second.Span.Length);
Assert.Equal(62, second.Span.Length);

expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
Expand Down Expand Up @@ -196,7 +194,7 @@ void Test()
public async Task ErrorDoesNotCrashPastEOF()
{
var spans = await GetTagSpansAsync("class C { int x =");
Assert.Equal(3, spans.Count());
Assert.Equal(3, spans.Length);
}

[WpfFact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)]
Expand All @@ -206,7 +204,7 @@ public async Task SemanticErrorReported()

var spans = await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetDiagnosticsAndErrorSpans(workspace);

Assert.Equal(1, spans.Item2.Count());
Assert.Equal(1, spans.Item2.Length);

var firstDiagnostic = spans.Item1.First();
var firstSpan = spans.Item2.First();
Expand Down Expand Up @@ -294,14 +292,14 @@ class Test
var document = workspace.Documents.First();

var updateArgs = DiagnosticsUpdatedArgs.DiagnosticsCreated(
new object(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id,
ImmutableArray.Create(
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 0)),
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 1))));
new object(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id,
ImmutableArray.Create(
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 0)),
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 1))));

var spans = await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetErrorsFromUpdateSource(workspace, updateArgs);

Assert.Equal(1, spans.Count());
Assert.Equal(1, spans.Count);
var first = spans.First();

Assert.Equal(1, first.Span.Span.Length);
Expand All @@ -325,26 +323,131 @@ class Test
var document = workspace.Documents.First();

var updateArgs = DiagnosticsUpdatedArgs.DiagnosticsCreated(
new LiveId(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id,
ImmutableArray.Create(
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 0)),
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 1))));
new LiveId(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id,
ImmutableArray.Create(
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 0)),
TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.CreateDiagnosticData(document, new TextSpan(0, 1))));

var spans = await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetErrorsFromUpdateSource(workspace, updateArgs);

Assert.Equal(2, spans.Count());
Assert.Equal(2, spans.Count);
var first = spans.First();
var second = spans.Last();

Assert.Equal(1, first.Span.Span.Length);
Assert.Equal(1, second.Span.Span.Length);
}

[WpfFact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)]
[WorkItem(1322, "https://github.com/dotnet/roslyn/issues/1322")]
public async Task FadingSpanNoComment()
{
var workspaceXml =
@"<Workspace>
<Project Language=""C#"" CommonReferences=""true"">
<Document FilePath = ""Test.cs"" >
using System.Collections;

class Program
{
}
</Document>
</Project>
</Workspace>";

using var workspace = TestWorkspace.Create(workspaceXml, composition: SquiggleUtilities.CompositionWithSolutionCrawler);
var options = new Dictionary<OptionKey2, object>();
var language = workspace.Projects.Single().Language;

workspace.ApplyOptions(options);

var analyzerMap = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>
{
{
LanguageNames.CSharp,
ImmutableArray.Create<DiagnosticAnalyzer>(new CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer())
}
};

var diagnosticsAndSpans = await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetDiagnosticsAndErrorSpans(workspace, analyzerMap);

var spans = diagnosticsAndSpans.Item1
.Zip(diagnosticsAndSpans.Item2, (diagnostic, span) => (diagnostic, span))
.OrderBy(s => s.span.Span.Span.Start).ToImmutableArray();

Assert.Equal(1, spans.Length);
var first = spans[0].span;

var expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
new ClassifiedTextElement(
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005", QuickInfoHyperLink.TestAccessor.CreateNavigationAction(new Uri("https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005", UriKind.Absolute)), "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005"),
new ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"),
new ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "),
new ClassifiedTextRun(ClassificationTypeNames.Text, CSharpAnalyzersResources.Using_directive_is_unnecessary)));

Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType);
ToolTipAssert.EqualContent(expectedToolTip, first.Tag.ToolTipContent);
Assert.Equal(2, first.Span.Start);
Assert.Equal(27, first.Span.Length);
}

[WpfFact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)]
[WorkItem(1322, "https://github.com/dotnet/roslyn/issues/1322")]
public async Task FadingSpanWithComment()
{
var workspaceXml =
@"<Workspace>
<Project Language=""C#"" CommonReferences=""true"">
<Document FilePath = ""Test.cs"" >
using System.Collections; // Goo

class Program
{
}
</Document>
</Project>
</Workspace>";

using var workspace = TestWorkspace.Create(workspaceXml, composition: SquiggleUtilities.CompositionWithSolutionCrawler);
var options = new Dictionary<OptionKey2, object>();
var language = workspace.Projects.Single().Language;

workspace.ApplyOptions(options);

var analyzerMap = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>
{
{
LanguageNames.CSharp,
ImmutableArray.Create<DiagnosticAnalyzer>(new CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer())
}
};

var diagnosticsAndSpans = await TestDiagnosticTagProducer<DiagnosticsSquiggleTaggerProvider, IErrorTag>.GetDiagnosticsAndErrorSpans(workspace, analyzerMap);

var spans = diagnosticsAndSpans.Item1
.Zip(diagnosticsAndSpans.Item2, (diagnostic, span) => (diagnostic, span))
.OrderBy(s => s.span.Span.Span.Start).ToImmutableArray();

Assert.Equal(1, spans.Length);
var first = spans[0].span;

var expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
new ClassifiedTextElement(
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005", QuickInfoHyperLink.TestAccessor.CreateNavigationAction(new Uri("https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005", UriKind.Absolute)), "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005"),
new ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"),
new ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "),
new ClassifiedTextRun(ClassificationTypeNames.Text, CSharpAnalyzersResources.Using_directive_is_unnecessary)));

Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType);
ToolTipAssert.EqualContent(expectedToolTip, first.Tag.ToolTipContent);
Assert.Equal(2, first.Span.Start);
Assert.Equal(34, first.Span.Length);
}

private class LiveId : ISupportLiveUpdate
{
public LiveId()
{
}
}

private static async Task<ImmutableArray<ITagSpan<IErrorTag>>> GetTagSpansAsync(string content)
Expand All @@ -355,7 +458,7 @@ private static async Task<ImmutableArray<ITagSpan<IErrorTag>>> GetTagSpansAsync(

private sealed class ReportOnClassWithLink : DiagnosticAnalyzer
{
public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
public static readonly DiagnosticDescriptor Rule = new(
"id",
"title",
"messageFormat",
Expand Down
29 changes: 29 additions & 0 deletions src/EditorFeatures/CSharpTest2/Recommendations/RefKeywordRecommenderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,5 +977,34 @@ await VerifyKeywordAsync(
@"namespace N;
$$");
}

Copy link
Member Author

Choose a reason for hiding this comment

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

oops. part of another fix. will break out.

Copy link
Member

Choose a reason for hiding this comment

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

The using span changes look good. Let me know when the PR is fixed up and I'll give it a final look over.

[Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
[WorkItem(58906, "https://github.com/dotnet/roslyn/issues/58906")]
public async Task TestInPotentialLambdaParamListParsedAsCastOnDifferentLines()
{
await VerifyKeywordAsync(
@"class C
{
static void Main(string[] args)
{
var f = ($$)
Main(null);
}
}");
}

[Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
[WorkItem(58906, "https://github.com/dotnet/roslyn/issues/58906")]
public async Task TestNotInPotentialLambdaParamListParsedAsCastOnSameLine()
{
await VerifyAbsenceAsync(
@"class C
{
static void Main(string[] args)
{
var f = ($$)Main(null);
}
}");
}
}
}
Loading