Skip to content

Commit

Permalink
Do not report unused using/imports if not processing doc comments.
Browse files Browse the repository at this point in the history
Also silently process doc comments when parsing them so we can report unused usings.
Fixes dotnet#2773
  • Loading branch information
gafter committed Apr 26, 2018
1 parent 8f9ee61 commit 81f4249
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,7 @@ internal override void ReportUnusedImports(SyntaxTree filterTree, DiagnosticBag
cancellationToken.ThrowIfCancellationRequested();

SyntaxTree infoTree = info.Tree;
if (filterTree == null || filterTree == infoTree)
if ((filterTree == null || filterTree == infoTree) && infoTree.Options.DocumentationMode != DocumentationMode.None)
{
TextSpan infoSpan = info.Span;
if (!this.IsImportDirectiveUsed(infoTree, infoSpan.Start))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,6 @@ private bool TryProcessDocumentationCommentTriviaNodes(
{
Debug.Assert(!docCommentNodes.IsDefaultOrEmpty);

bool haveWriter = _writer != null;

bool processedDocComment = false; // Even if there are DocumentationCommentTriviaSyntax, we may not need to process any of them.

ArrayBuilder<CSharpSyntaxNode> includeElementNodesBuilder = null;
Expand All @@ -413,13 +411,6 @@ private bool TryProcessDocumentationCommentTriviaNodes(

bool reportDiagnosticsForCurrentTrivia = trivia.SyntaxTree.ReportDocumentationCommentDiagnostics();

// If we're writing XML or we need to report diagnostics (either in this particular piece of trivia,
// or concerning undocumented [type] parameters), then we need to process this trivia node.
if (!(haveWriter || reportDiagnosticsForCurrentTrivia || reportParameterOrTypeParameterDiagnostics))
{
continue;
}

if (!processedDocComment)
{
// Since we have to throw away all the parts if any part is bad, we need to write to an intermediate temp.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,8 @@ public void CrefCountsAsUse()
/// <see cref='Console'/>
public class C { }
";

// Not binding doc comments.
CreateCompilation(source).VerifyDiagnostics(
// (2,1): info CS8019: Unnecessary using directive.
// using System;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using System;"));
// Not reporting doc comment diagnostics. It is still a use.
CreateCompilation(source).VerifyDiagnostics();

// Binding doc comments.
CreateCompilationWithMscorlib40AndDocumentationComments(source).VerifyDiagnostics();
Expand Down Expand Up @@ -430,5 +426,34 @@ partial void M(ref int x) { }
//Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using System.Runtime.InteropServices;").WithLocation(1, 1)
);
}

[Fact, WorkItem(2773, "https://github.com/dotnet/roslyn/issues/2773")]
public void UsageInDocComment()
{
var source = @"using X;
/// <summary/>
public class Program
{
/// <summary>
/// <see cref=""Q""/>
/// </summary>
static void Main(string[] args)
{
}
}
namespace X
{
/// <summary/>
public class Q { }
}
";
foreach (DocumentationMode documentationMode in Enum.GetValues(typeof(DocumentationMode)))
{
var compilation = CreateCompilation(source, parseOptions: TestOptions.Regular.WithDocumentationMode(documentationMode));
compilation.VerifyDiagnostics();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4369,17 +4369,17 @@ public partial class D { }
public partial class E { }
";

var tree1 = Parse(source1, options: TestOptions.RegularWithDocumentationComments.WithLanguageVersion(LanguageVersion.Latest));
var tree2 = Parse(source2, options: TestOptions.Regular.WithLanguageVersion(LanguageVersion.Latest));
var tree1 = Parse(source1, options: TestOptions.Regular.WithDocumentationMode(DocumentationMode.Diagnose).WithLanguageVersion(LanguageVersion.Latest));
var tree2 = Parse(source2, options: TestOptions.Regular.WithDocumentationMode(DocumentationMode.None).WithLanguageVersion(LanguageVersion.Latest));

// This scenario does not exist in dev11, but the diagnostics seem reasonable.
CreateCompilation(new[] { tree1, tree2 }).VerifyDiagnostics(
// (5,22): warning CS1591: Missing XML comment for publicly visible type or member 'D'
// public partial class D { }
Diagnostic(ErrorCode.WRN_MissingXMLComment, "D").WithArguments("D"),
Diagnostic(ErrorCode.WRN_MissingXMLComment, "D").WithArguments("D").WithLocation(5, 22),
// (7,22): warning CS1591: Missing XML comment for publicly visible type or member 'E'
// public partial class E { }
Diagnostic(ErrorCode.WRN_MissingXMLComment, "E").WithArguments("E"));
Diagnostic(ErrorCode.WRN_MissingXMLComment, "E").WithArguments("E").WithLocation(7, 22));
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/Test/Utilities/CSharp/TestOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ namespace Microsoft.CodeAnalysis.CSharp.Test.Utilities
{
public static class TestOptions
{
// Disable documentation comments by default so that we don't need to
// Disable diagnosing documentation comments by default so that we don't need to
// document every public member of every test input.
public static readonly CSharpParseOptions Regular = new CSharpParseOptions(kind: SourceCodeKind.Regular, documentationMode: DocumentationMode.None).WithLanguageVersion(LanguageVersion.Latest);
public static readonly CSharpParseOptions Regular = new CSharpParseOptions(kind: SourceCodeKind.Regular, documentationMode: DocumentationMode.Parse).WithLanguageVersion(LanguageVersion.Latest);
public static readonly CSharpParseOptions Script = Regular.WithKind(SourceCodeKind.Script);
public static readonly CSharpParseOptions Regular6 = Regular.WithLanguageVersion(LanguageVersion.CSharp6);
public static readonly CSharpParseOptions Regular7 = Regular.WithLanguageVersion(LanguageVersion.CSharp7);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1585,7 +1585,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
cancellationToken.ThrowIfCancellationRequested()

Dim infoTree As SyntaxTree = info.Tree
If filterTree Is Nothing OrElse filterTree Is infoTree Then
If (filterTree Is Nothing OrElse filterTree Is infoTree) AndAlso infoTree.Options.DocumentationMode <> DocumentationMode.None Then
Dim clauseSpans = info.ClauseSpans
Dim numClauseSpans = clauseSpans.Length

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,19 +281,17 @@ Imports System
''' <see cref='Console'/>
Class A
End Class
]]></file>
]]></file>
</compilation>

' Without doc comments.
CreateCompilationWithMscorlib40(source, parseOptions:=New VisualBasicParseOptions(documentationMode:=DocumentationMode.None)).AssertTheseDiagnostics(
<errors>
BC50001: Unused import statement.
Imports System
~~~~~~~~~~~~~~
</errors>, suppressInfos:=False)
CreateCompilationWithMscorlib40(source, parseOptions:=New VisualBasicParseOptions(documentationMode:=DocumentationMode.None)).AssertNoDiagnostics(suppressInfos:=False)

' With doc comments parsed.
CreateCompilationWithMscorlib40(source, parseOptions:=New VisualBasicParseOptions(documentationMode:=DocumentationMode.Parse)).AssertNoDiagnostics(suppressInfos:=False)

' With doc comments.
CreateCompilationWithMscorlib40(source, parseOptions:=New VisualBasicParseOptions(documentationMode:=DocumentationMode.Diagnose)).AssertTheseDiagnostics(<errors></errors>, suppressInfos:=False)
' With doc comments diagnosed.
CreateCompilationWithMscorlib40(source, parseOptions:=New VisualBasicParseOptions(documentationMode:=DocumentationMode.Diagnose)).AssertNoDiagnostics(suppressInfos:=False)
End Sub

<Fact>
Expand Down Expand Up @@ -366,5 +364,36 @@ End Class
Dim diagnostics = model.GetDiagnostics()
AssertTheseDiagnostics(diagnostics, <errors></errors>)
End Sub

<Fact, WorkItem(2773, "https://github.com/dotnet/roslyn/issues/2773")>
Public Sub UsageInDocComment()
Dim source =
<compilation>
<file name="a.vb"><![CDATA[
Imports X

''' <summary/>
Public Class Program

''' <summary>
''' <see cref="Q"/>
''' </summary>
Public Sub Main()
End Sub

End Class

Namespace Global.X
''' <summary/>
Public Class Q
End Class
End Namespace
]]></file>
</compilation>
For Each documentationMode As DocumentationMode In [Enum].GetValues(GetType(DocumentationMode))
Dim compilation = CreateCompilationWithMscorlib40(source, parseOptions:=TestOptions.Regular.WithDocumentationMode(documentationMode))
compilation.AssertNoDiagnostics(suppressInfos:=False)
Next
End Sub
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -1307,19 +1307,23 @@ public static void Main()
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)]
public async Task TestReferenceInCref()
{
// parsing doc comments as simple trivia; System is unnecessary
await TestInRegularAndScriptAsync(
// parsing doc comments as simple trivia; we don't know System is unnecessary
await TestMissingAsync(
@"[|using System;
/// <summary><see cref=""String"" /></summary>
class C
{
}|]",
@"/// <summary><see cref=""String"" /></summary>
}|]", new TestParameters(Options.Regular.WithDocumentationMode(DocumentationMode.None)));

// fully parsing doc comments; System is necessary
await TestMissingAsync(
@"[|using System;
/// <summary><see cref=""String"" /></summary>
class C
{
}");
}|]", new TestParameters(Options.Regular.WithDocumentationMode(DocumentationMode.Parse)));

// fully parsing doc comments; System is necessary
// fully parsing and diagnosing doc comments; System is necessary
await TestMissingAsync(
@"[|using System;
/// <summary><see cref=""String"" /></summary>
Expand Down

0 comments on commit 81f4249

Please sign in to comment.