Skip to content
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 @@ -347,5 +347,18 @@ public static bool Contains<T>(this IEnumerable<T> sequence, Func<T, bool> predi
{
return sequence.Any(predicate);
}

public static bool Contains(this IEnumerable<string> sequence, string s)
{
foreach (var item in sequence)
{
if (item == s)
{
return true;
}
}

return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Diagnostics.RemoveUnnecessaryImports;
using Microsoft.CodeAnalysis.CSharp.Diagnostics.SimplifyTypeNames;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.VisualStudio.Text.Adornments;
Expand Down Expand Up @@ -61,6 +65,48 @@ void Test()
}
}

[Fact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)]
public void SuggestionTagsForUnnecessaryCode()
{
var workspaceXml =
@"<Workspace>
<Project Language=""C#"" CommonReferences=""true"">
<Document FilePath = ""Test.cs"" >
using System.Collections; // Unused using.
class Program
{
void Test()
{
System.Int32 x = 2; // Simplify type name.
x += 1;
}
}
</Document>
</Project>
</Workspace>";

using (var workspace = TestWorkspaceFactory.CreateWorkspace(workspaceXml))
{
var analyzerMap = ImmutableDictionary.CreateBuilder<string, ImmutableArray<DiagnosticAnalyzer>>();
analyzerMap.Add(LanguageNames.CSharp,
ImmutableArray.Create<DiagnosticAnalyzer>(
new CSharpSimplifyTypeNamesDiagnosticAnalyzer(),
new CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer()));
var spans = GetErrorSpans(workspace, analyzerMap.ToImmutable());

spans = spans.OrderBy(s => s.Span.Span.Start);

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

Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType);
Assert.Equal(CSharpFeaturesResources.RemoveUnnecessaryUsingsDiagnosticTitle, first.Tag.ToolTipContent);
Assert.Equal(PredefinedErrorTypeNames.Suggestion, second.Tag.ErrorType);
Assert.Equal(WorkspacesResources.NameCanBeSimplified, second.Tag.ToolTipContent);
}
}

[Fact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)]
public void ErrorDoesNotCrashPastEOF()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ protected override int MinimumLength

protected override bool ShouldInclude(DiagnosticData diagnostic)
{
var isUnnecessary = (diagnostic.Severity == DiagnosticSeverity.Hidden && diagnostic.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary));

return
(diagnostic.Severity == DiagnosticSeverity.Warning || diagnostic.Severity == DiagnosticSeverity.Error) &&
(diagnostic.Severity == DiagnosticSeverity.Warning || diagnostic.Severity == DiagnosticSeverity.Error || isUnnecessary) &&
!string.IsNullOrWhiteSpace(diagnostic.Message);
}

Expand Down Expand Up @@ -89,7 +91,19 @@ private static string GetErrorTypeFromDiagnosticSeverity(DiagnosticData diagnost
case DiagnosticSeverity.Warning:
return PredefinedErrorTypeNames.Warning;
case DiagnosticSeverity.Info:
return null;
case DiagnosticSeverity.Hidden:
if (diagnostic.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary))
{
// This ensures that we have an 'invisible' squiggle (which will in turn
// display Quick Info on mouse hover) for the hidden diagnostics that we
// report for 'Remove Unnecessary Usings' and 'Simplify Type Name'. The
// presence of Quick Info pane for such squiggles allows allows platform
// to display Light Bulb for the corresponding fixes (per their current
// design platform can only display light bulb if Quick Info pane is present).
return PredefinedErrorTypeNames.Suggestion;
}

return null;
default:
return PredefinedErrorTypeNames.OtherError;
Expand Down
16 changes: 13 additions & 3 deletions src/EditorFeatures/Test/Squiggles/AbstractSquiggleProducerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles
{
public abstract class AbstractSquiggleProducerTests
{
protected static IEnumerable<ITagSpan<IErrorTag>> GetErrorSpans(TestWorkspace workspace)
protected static IEnumerable<ITagSpan<IErrorTag>> GetErrorSpans(TestWorkspace workspace, ImmutableDictionary<string, ImmutableArray<DiagnosticAnalyzer>> analyzerMap = null)
{
var registrationService = workspace.Services.GetService<ISolutionCrawlerRegistrationService>();
registrationService.Register(workspace);
Expand All @@ -29,8 +29,18 @@ protected static IEnumerable<ITagSpan<IErrorTag>> GetErrorSpans(TestWorkspace wo
() => diagnosticWaiter, new FeatureMetadata(new Dictionary<string, object>() { { "FeatureName", FeatureAttribute.DiagnosticService } })));

var optionsService = workspace.Services.GetService<IOptionService>();
var compilerAnalyzersMap = DiagnosticExtensions.GetCompilerDiagnosticAnalyzersMap();
var analyzerService = new DiagnosticAnalyzerService(compilerAnalyzersMap);

DiagnosticAnalyzerService analyzerService = null;
if (analyzerMap == null || analyzerMap.Count == 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could write analyzerMap?.Count > 0 and flip then/else branches.

{
var compilerAnalyzersMap = DiagnosticExtensions.GetCompilerDiagnosticAnalyzersMap();
analyzerService = new DiagnosticAnalyzerService(compilerAnalyzersMap);
}
else
{
analyzerService = new DiagnosticAnalyzerService(analyzerMap);
}

var diagnosticService = new DiagnosticService(SpecializedCollections.SingletonEnumerable<IDiagnosticUpdateSource>(analyzerService), diagnosticListeners);

var document = workspace.Documents.First();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.VisualBasic.CodeFixes.SimplifyTypeNames
Imports Microsoft.CodeAnalysis.VisualBasic.Diagnostics.RemoveUnnecessaryImports
Imports Microsoft.VisualStudio.Text.Adornments
Imports Microsoft.VisualStudio.Text.Tagging

Expand All @@ -16,6 +20,12 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Squiggles
End Using
End Function

Private Function ProduceSquiggles(analyzerMap As ImmutableDictionary(Of String, ImmutableArray(Of DiagnosticAnalyzer)), ParamArray lines As String()) As IEnumerable(Of ITagSpan(Of IErrorTag))
Using workspace = VisualBasicWorkspaceFactory.CreateWorkspaceFromLines(lines)
Return GetErrorSpans(workspace, analyzerMap)
End Using
End Function

<Fact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)>
Public Sub ErrorTagGeneratedForSimpleError()
' Make sure we have errors from the tree
Expand Down Expand Up @@ -57,5 +67,34 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Squiggles
Assert.Equal(PredefinedErrorTypeNames.SyntaxError, firstSpan.Tag.ErrorType)
Assert.Contains("Bar", DirectCast(firstSpan.Tag.ToolTipContent, String), StringComparison.Ordinal)
End Sub

<Fact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)>
Public Sub SuggestionTagsForUnnecessaryCode()

Dim analyzerMap = ImmutableDictionary.CreateBuilder(Of String, ImmutableArray(Of DiagnosticAnalyzer))
analyzerMap.Add(LanguageNames.VisualBasic,
ImmutableArray.Create(Of DiagnosticAnalyzer)(
New VisualBasicSimplifyTypeNamesDiagnosticAnalyzer(),
New VisualBasicRemoveUnnecessaryImportsDiagnosticAnalyzer()))

Dim spans = ProduceSquiggles(analyzerMap.ToImmutable(),
"Imports System.Collections ' Unused import.
Class C1
Sub Foo()
Dim x as System.Int32 = 2 ' Simplify type name.
x = x + 1
End Sub
End Class")
spans = spans.OrderBy(Function(s) s.Span.Span.Start)

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

Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType)
Assert.Equal(VBFeaturesResources.RemoveUnnecessaryImportsDiagnosticTitle, first.Tag.ToolTipContent)
Assert.Equal(PredefinedErrorTypeNames.Suggestion, second.Tag.ErrorType)
Assert.Equal(WorkspacesResources.NameCanBeSimplified, second.Tag.ToolTipContent)
End Sub
End Class
End Namespace
9 changes: 9 additions & 0 deletions src/Features/CSharp/CSharpFeaturesResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Features/CSharp/CSharpFeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,7 @@
<data name="IntoClause" xml:space="preserve">
<value>into clause</value>
</data>
<data name="RemoveUnnecessaryUsingsDiagnosticTitle" xml:space="preserve">
<value>Using directive is unnecessary.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ namespace Microsoft.CodeAnalysis.CSharp.Diagnostics.RemoveUnnecessaryImports
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal sealed class CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer : RemoveUnnecessaryImportsDiagnosticAnalyzerBase
{
private static readonly LocalizableString s_TitleAndMessageFormat =
new LocalizableResourceString(nameof(CSharpFeaturesResources.RemoveUnnecessaryUsingsDiagnosticTitle), CSharpFeaturesResources.ResourceManager, typeof(CSharpFeaturesResources));

protected override LocalizableString GetTitleAndMessageFormatForClassificationIdDescriptor()
{
return s_TitleAndMessageFormat;
}

protected override IEnumerable<SyntaxNode> GetUnnecessaryImports(SemanticModel semanticModel, SyntaxNode root, CancellationToken cancellationToken = default(CancellationToken))
{
return CSharpRemoveUnnecessaryImportsService.GetUnnecessaryImports(semanticModel, root, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,48 @@ internal abstract class RemoveUnnecessaryImportsDiagnosticAnalyzerBase : Diagnos
// NOTE: This is a trigger diagnostic, which doesn't show up in the ruleset editor and hence doesn't need a conventional IDE Diagnostic ID string.
internal const string DiagnosticFixableId = "RemoveUnnecessaryImportsFixable";

private static LocalizableString s_localizableMessageAndTitle = new LocalizableResourceString(nameof(WorkspacesResources.RemoveUnnecessaryImportsOrUsings), WorkspacesResources.ResourceManager, typeof(WorkspacesResources));

private static readonly DiagnosticDescriptor s_classificationIdDescriptor = new DiagnosticDescriptor(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId,
s_localizableMessageAndTitle,
s_localizableMessageAndTitle,
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
customTags: DiagnosticCustomTags.Unnecessary);

// The NotConfigurable custom tag ensures that user can't turn this diagnostic into a warning / error via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be nice to keep the descriptor here so that both of the descriptors are together instead of being spread across three files. Instead of the abstract GetClassificationIdDescriptor, you could simply have an abstract GetMessage(). That way if we make changes to the descriptor we can make it in one place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure will change that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

// ruleset editor or solution explorer. Setting messageFormat to empty string ensures that we won't display
// this diagnostic in the preview pane header.
private static readonly DiagnosticDescriptor s_fixableIdDescriptor = new DiagnosticDescriptor(DiagnosticFixableId,
title: "", messageFormat: "", category: "",
defaultSeverity: DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTags.NotConfigurable);
private static readonly DiagnosticDescriptor s_fixableIdDescriptor =
new DiagnosticDescriptor(DiagnosticFixableId,
title: "", messageFormat: "", category: "",
defaultSeverity: DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTags.NotConfigurable);

protected abstract LocalizableString GetTitleAndMessageFormatForClassificationIdDescriptor();

private static readonly ImmutableArray<DiagnosticDescriptor> s_descriptors = ImmutableArray.Create(s_fixableIdDescriptor, s_classificationIdDescriptor);
private DiagnosticDescriptor _classificationIdDescriptor;
private DiagnosticDescriptor GetClassificationIdDescriptor()
{
if (_classificationIdDescriptor == null)
{
var titleAndMessageFormat = GetTitleAndMessageFormatForClassificationIdDescriptor();
_classificationIdDescriptor =
new DiagnosticDescriptor(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId,
titleAndMessageFormat,
titleAndMessageFormat,
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
customTags: DiagnosticCustomTags.Unnecessary);
}

return _classificationIdDescriptor;
}

private ImmutableArray<DiagnosticDescriptor> _descriptors;
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
get
{
return s_descriptors;
if (_descriptors == null)
{
_descriptors = ImmutableArray.Create(s_fixableIdDescriptor, GetClassificationIdDescriptor());
}

return _descriptors;
}
}

Expand Down Expand Up @@ -85,7 +101,7 @@ protected virtual Func<SyntaxNode, SyntaxToken> GetLastTokenDelegateForContiguou
continue;
}

yield return Diagnostic.Create(s_classificationIdDescriptor, tree.GetLocation(span));
yield return Diagnostic.Create(GetClassificationIdDescriptor(), tree.GetLocation(span));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Diagnostics.RemoveUnnecessaryImport
<DiagnosticAnalyzer(LanguageNames.VisualBasic)>
Friend NotInheritable Class VisualBasicRemoveUnnecessaryImportsDiagnosticAnalyzer
Inherits RemoveUnnecessaryImportsDiagnosticAnalyzerBase
Private Shared ReadOnly s_TitleAndMessageFormat As LocalizableString =
New LocalizableResourceString(NameOf(VBFeaturesResources.RemoveUnnecessaryImportsDiagnosticTitle), VBFeaturesResources.ResourceManager, GetType(VBFeaturesResources.VBFeaturesResources))

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

Protected Overrides Function GetUnnecessaryImports(semanticModel As SemanticModel, root As SyntaxNode, Optional cancellationToken As CancellationToken = Nothing) As IEnumerable(Of SyntaxNode)
Return VisualBasicRemoveUnnecessaryImportsService.GetUnnecessaryImports(semanticModel, root, cancellationToken)
Expand Down
9 changes: 9 additions & 0 deletions src/Features/VisualBasic/VBFeaturesResources.Designer.vb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Features/VisualBasic/VBFeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1048,4 +1048,7 @@ Sub(&lt;parameterList&gt;) &lt;statement&gt;</value>
<data name="UncommentTheFollowingLineIfFinalizeIsOverridden" xml:space="preserve">
<value>TODO: uncomment the following line if Finalize() is overridden above.</value>
</data>
<data name="RemoveUnnecessaryImportsDiagnosticTitle" xml:space="preserve">
<value>Imports statement is unnecessary.</value>
</data>
</root>
Loading