From f8284571b8e7331f710d36cd77821e5d47c074dd Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 15:24:14 -0700 Subject: [PATCH 1/7] Don't cleanup nodes that have syntax errors and multiline string literals in them. --- .../CaseCorrectionServiceTests.vb | 49 ++++++++++++- .../CodeCleanup/CSharpCodeCleanerService.cs | 5 ++ .../CodeCleanup/AbstractCodeCleanerService.cs | 27 +++++-- .../CodeCleanup/ICodeCleanerService.cs | 4 +- .../VisualBasicCodeCleanerService.vb | 72 ++++++++++++++++--- 5 files changed, 138 insertions(+), 19 deletions(-) diff --git a/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb b/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb index 85a094b645379..786715bed6327 100644 --- a/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb +++ b/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb @@ -1,9 +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 System.Threading Imports System.Xml.Linq Imports Microsoft.CodeAnalysis Imports Microsoft.CodeAnalysis.CaseCorrection +Imports Microsoft.CodeAnalysis.CodeCleanup +Imports Microsoft.CodeAnalysis.CodeCleanup.Providers Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces @@ -12,10 +15,8 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CaseCorrecting Private Async Function TestAsync(input As XElement, expected As XElement, Optional interProject As Boolean = False) As Tasks.Task If (interProject) Then Await TestAsync(input, expected.NormalizedValue) - Await TestAsync(input, expected.NormalizedValue) Else Await TestAsync(input.NormalizedValue, expected.NormalizedValue) - Await TestAsync(input.NormalizedValue, expected.NormalizedValue) End If End Function @@ -31,7 +32,11 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CaseCorrecting Dim document = workspace.CurrentSolution.GetDocument(hostDocument.Id) Dim span = (Await document.GetSyntaxRootAsync()).FullSpan - Dim newDocument = Await CaseCorrector.CaseCorrectAsync(document, span, CancellationToken.None) + Dim service = document.GetLanguageService(Of ICodeCleanerService) + Dim newDocument = Await service.CleanupAsync( + document, ImmutableArray.Create(span), + ImmutableArray.Create(Of ICodeCleanupProvider)(New CaseCorrectionCodeCleanupProvider())) + newDocument.Project.Solution.Workspace.ApplyDocumentChanges(newDocument, CancellationToken.None) Dim actual = buffer.CurrentSnapshot.GetText() @@ -1169,6 +1174,44 @@ End Module Module M Dim s = NameOf(M) End Module + + + Await TestAsync(input, expected) + End Function + + + + Public Async Function AvoidNodesWithSyntaxErrorsAndStringLiterals() As Task + Dim input = +Class C + Private Sub Test() + Dim sql As New System.Text.StringBuilder + + sql.AppendLine(" SELECT *") + sql.AppendLine(" , Table.Column1" + sql.AppendLine(", Table.Column2 + sql.AppendLine(", Table.Column3") + sql.AppendLine(" , Table.Column4 AS ColumnAlias") + sql.AppendLine(" , Table.Column5 AS ColumnAlias2") + sql.AppendLine("FROM Table('Parameter')") + End Sub +End Class + + + Dim expected = +Class C + Private Sub Test() + Dim sql As New System.Text.StringBuilder + + sql.AppendLine(" SELECT *") + sql.AppendLine(" , Table.Column1" + sql.AppendLine(", Table.Column2 + sql.AppendLine(", Table.Column3") + sql.AppendLine(" , Table.Column4 AS ColumnAlias") + sql.AppendLine(" , Table.Column5 AS ColumnAlias2") + sql.AppendLine("FROM Table('Parameter')") + End Sub +End Class Await TestAsync(input, expected) diff --git a/src/Workspaces/CSharp/Portable/CodeCleanup/CSharpCodeCleanerService.cs b/src/Workspaces/CSharp/Portable/CodeCleanup/CSharpCodeCleanerService.cs index adb17c9e623bd..5cc8ce49edc77 100644 --- a/src/Workspaces/CSharp/Portable/CodeCleanup/CSharpCodeCleanerService.cs +++ b/src/Workspaces/CSharp/Portable/CodeCleanup/CSharpCodeCleanerService.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Immutable; using Microsoft.CodeAnalysis.CodeCleanup; using Microsoft.CodeAnalysis.CodeCleanup.Providers; +using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.CSharp.CodeCleanup { @@ -14,5 +16,8 @@ internal class CSharpCodeCleanerService : AbstractCodeCleanerService public override ImmutableArray GetDefaultProviders() => s_defaultProviders; + + protected override ImmutableArray GetSpansToAvoid(SyntaxNode root) + => ImmutableArray.Empty; } } \ No newline at end of file diff --git a/src/Workspaces/Core/Portable/CodeCleanup/AbstractCodeCleanerService.cs b/src/Workspaces/Core/Portable/CodeCleanup/AbstractCodeCleanerService.cs index 787ff864300e3..3edf7a960f20d 100644 --- a/src/Workspaces/Core/Portable/CodeCleanup/AbstractCodeCleanerService.cs +++ b/src/Workspaces/Core/Portable/CodeCleanup/AbstractCodeCleanerService.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.CodeCleanup.Providers; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.LanguageServices; +using Microsoft.CodeAnalysis.Shared; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; @@ -19,13 +20,12 @@ namespace Microsoft.CodeAnalysis.CodeCleanup internal abstract class AbstractCodeCleanerService : ICodeCleanerService { public abstract ImmutableArray GetDefaultProviders(); + protected abstract ImmutableArray GetSpansToAvoid(SyntaxNode root); public async Task CleanupAsync(Document document, ImmutableArray spans, ImmutableArray providers, CancellationToken cancellationToken) { using (Logger.LogBlock(FunctionId.CodeCleanup_CleanupAsync, cancellationToken)) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - // If there is no span to format... if (!spans.Any()) { @@ -35,6 +35,8 @@ public async Task CleanupAsync(Document document, ImmutableArray CleanupAsync(Document document, ImmutableArray ImmutableArray.Create(n.FullSpan), codeCleaners, cancellationToken).ConfigureAwait(false); } - var model = await document.GetSemanticModelForSpanAsync(spans.Collapse(), cancellationToken).ConfigureAwait(false); - // Replace the initial node and document with the annotated node. var annotatedRoot = newNodeAndAnnotations.newNode; var annotatedDocument = document.WithSyntaxRoot(annotatedRoot); @@ -481,7 +481,7 @@ private async Task IterateAllCodeCleanupProvidersAsync( // Document was changed by the previous code cleaner, compute new spans. var root = await currentDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); previousDocument = currentDocument; - spans = spanGetter(root); + spans = GetSpans(root, spanGetter);; } // If we are at the end and there were no changes to the document, use the original document for the cleanup. @@ -516,6 +516,21 @@ private async Task IterateAllCodeCleanupProvidersAsync( } } + private ImmutableArray GetSpans( + SyntaxNode root, Func> spanGetter) + { + // Get all the spans we've been requested to clean up. + var requestedSpans = new NormalizedTextSpanCollection(spanGetter(root)); + + // See if there are any spans we should not touch. + var spansToAvoid = new NormalizedTextSpanCollection(GetSpansToAvoid(root)); + + // Remove the spans we should not touch from the requested spans and return that final set. + var result = NormalizedTextSpanCollection.Difference(requestedSpans, spansToAvoid); + + return result.ToImmutableArray(); + } + private async Task IterateAllCodeCleanupProvidersAsync( SyntaxNode originalRoot, SyntaxNode annotatedRoot, @@ -542,7 +557,7 @@ private async Task IterateAllCodeCleanupProvidersAsync( { // The root was changed by the previous code cleaner, compute new spans. previousRoot = currentRoot; - spans = spanGetter(currentRoot); + spans = GetSpans(currentRoot, spanGetter); } // If we are at the end and there were no changes to the document, use the original document for the cleanup. diff --git a/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs b/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs index ae7a4a01e9718..1459166d76a9b 100644 --- a/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs +++ b/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs @@ -25,13 +25,13 @@ internal interface ICodeCleanerService : ILanguageService /// /// This will run all provided code cleaners in an order that is given to the method. /// - Task CleanupAsync(Document document, ImmutableArray spans, ImmutableArray providers, CancellationToken cancellationToken); + Task CleanupAsync(Document document, ImmutableArray spans, ImmutableArray providers, CancellationToken cancellationToken = default(CancellationToken)); /// /// This will run all provided code cleaners in an order that is given to the method. /// /// This will do cleanups that don't require any semantic information. /// - Task CleanupAsync(SyntaxNode root, ImmutableArray spans, Workspace workspace, ImmutableArray providers, CancellationToken cancellationToken); + Task CleanupAsync(SyntaxNode root, ImmutableArray spans, Workspace workspace, ImmutableArray providers, CancellationToken cancellationToken = default(CancellationToken)); } } \ No newline at end of file diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index 3603da1a430a5..eb7823ed58d8f 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -3,23 +3,79 @@ Imports System.Collections.Immutable Imports Microsoft.CodeAnalysis.CodeCleanup Imports Microsoft.CodeAnalysis.CodeCleanup.Providers +Imports Microsoft.CodeAnalysis.Text Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Partial Friend Class VisualBasicCodeCleanerService Inherits AbstractCodeCleanerService Private Shared ReadOnly s_defaultProviders As ImmutableArray(Of ICodeCleanupProvider) = ImmutableArray.Create(Of ICodeCleanupProvider)( - New AddMissingTokensCodeCleanupProvider(), - New FixIncorrectTokensCodeCleanupProvider(), - New ReduceTokensCodeCleanupProvider(), - New NormalizeModifiersOrOperatorsCodeCleanupProvider(), - New RemoveUnnecessaryLineContinuationCodeCleanupProvider(), - New CaseCorrectionCodeCleanupProvider(), - New SimplificationCodeCleanupProvider(), - New FormatCodeCleanupProvider()) + New AddMissingTokensCodeCleanupProvider(), + New FixIncorrectTokensCodeCleanupProvider(), + New ReduceTokensCodeCleanupProvider(), + New NormalizeModifiersOrOperatorsCodeCleanupProvider(), + New RemoveUnnecessaryLineContinuationCodeCleanupProvider(), + New CaseCorrectionCodeCleanupProvider(), + New SimplificationCodeCleanupProvider(), + New FormatCodeCleanupProvider()) Public Overrides Function GetDefaultProviders() As ImmutableArray(Of ICodeCleanupProvider) Return s_defaultProviders End Function + + Protected Overrides Function GetSpansToAvoid(root As SyntaxNode) As ImmutableArray(Of TextSpan) + ' We don't want to touch nodes in the document that have syntax errors on them and which + ' contain String literals. It's quite possible that there is some string literal on a + ' previous line that was intended to be terminated on that line, but which wasn't. The + ' string may then have terminated on this line (because of the start of another literal) + ' causing the literal contents to then be considered code. We don't want to cleanup + ' 'code' that the user intends to be the content of a string literal. + + Dim result = ArrayBuilder(Of TextSpan).GetInstance() + + ProcessNode(root, root, result) + + Return result.ToImmutableAndFree() + End Function + + Private Sub ProcessNode(root As SyntaxNode, node As SyntaxNode, result As ArrayBuilder(Of TextSpan)) + If Not node.ContainsDiagnostics Then + Return + End If + + For Each child In node.ChildNodesAndTokens() + If child.IsNode Then + ProcessNode(root, child.AsNode(), result) + Else + ProcessToken(root, child.AsToken(), result) + End If + Next + End Sub + + Private Sub ProcessToken(root As SyntaxNode, token As SyntaxToken, result As ArrayBuilder(Of TextSpan)) + If token.ContainsDiagnostics Then + Dim parentMultiLineNode = If(GetMultiLineContainer(token.Parent), root) + + If ContainsStringLiteral(parentMultiLineNode) Then + result.Add(parentMultiLineNode.FullSpan) + End If + End If + End Sub + + Private Function ContainsStringLiteral(node As SyntaxNode) As Boolean + Return node.DescendantTokens().Any(Function(t) t.Kind() = SyntaxKind.StringLiteralToken) + End Function + + Private Function GetMultiLineContainer(node As SyntaxNode) As SyntaxNode + If node Is Nothing Then + Return Nothing + End If + + If Not VisualBasicSyntaxFactsService.Instance.IsOnSingleLine(node, fullSpan:=False) Then + Return node + End If + + Return GetMultiLineContainer(node.Parent) + End Function End Class End Namespace \ No newline at end of file From 114170fe2b6d4f62046d95f09bfe783be0b7e75f Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 15:28:09 -0700 Subject: [PATCH 2/7] Specifically look for multiline strings. --- .../CodeCleanup/VisualBasicCodeCleanerService.vb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index eb7823ed58d8f..bd4e80abe0247 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -40,6 +40,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Private Sub ProcessNode(root As SyntaxNode, node As SyntaxNode, result As ArrayBuilder(Of TextSpan)) If Not node.ContainsDiagnostics Then + ' Don't bother going down nodes that don't have any syntax errors in them. Return End If @@ -56,14 +57,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup If token.ContainsDiagnostics Then Dim parentMultiLineNode = If(GetMultiLineContainer(token.Parent), root) - If ContainsStringLiteral(parentMultiLineNode) Then + If ContainsMultiLineStringLiteral(parentMultiLineNode) Then result.Add(parentMultiLineNode.FullSpan) End If End If End Sub - Private Function ContainsStringLiteral(node As SyntaxNode) As Boolean - Return node.DescendantTokens().Any(Function(t) t.Kind() = SyntaxKind.StringLiteralToken) + Private Function ContainsMultiLineStringLiteral(node As SyntaxNode) As Boolean + Return node.DescendantTokens().Any( + Function(t) + Return t.Kind() = SyntaxKind.StringLiteralToken AndAlso + Not VisualBasicSyntaxFactsService.Instance.IsOnSingleLine(t.Parent, fullSpan:=False) + End Function) End Function Private Function GetMultiLineContainer(node As SyntaxNode) As SyntaxNode From f97e4414d7bc455fe11401b50d91ca5ac8d60bb0 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 15:34:36 -0700 Subject: [PATCH 3/7] Fixup comment. --- .../CodeCleanup/VisualBasicCodeCleanerService.vb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index bd4e80abe0247..4812e6c6de249 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -25,11 +25,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Protected Overrides Function GetSpansToAvoid(root As SyntaxNode) As ImmutableArray(Of TextSpan) ' We don't want to touch nodes in the document that have syntax errors on them and which - ' contain String literals. It's quite possible that there is some string literal on a - ' previous line that was intended to be terminated on that line, but which wasn't. The - ' string may then have terminated on this line (because of the start of another literal) - ' causing the literal contents to then be considered code. We don't want to cleanup - ' 'code' that the user intends to be the content of a string literal. + ' contain multi-line string literals. It's quite possible that there is some string + ' literal on a previous line that was intended to be terminated on that line, but which + ' wasn't. The string may then have terminated on this line (because of the start of + ' another literal) causing the literal contents to then be considered code. We don't + ' want to cleanup 'code' that the user intends to be the content of a string literal. Dim result = ArrayBuilder(Of TextSpan).GetInstance() From aa41ffa80b46173c835c3d3d15f2c514b1d3a125 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 15:35:59 -0700 Subject: [PATCH 4/7] Simplify code. --- .../CodeCleanup/VisualBasicCodeCleanerService.vb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index 4812e6c6de249..2d451c977b1a2 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -33,12 +33,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Dim result = ArrayBuilder(Of TextSpan).GetInstance() - ProcessNode(root, root, result) + ProcessNode(root, result) Return result.ToImmutableAndFree() End Function - Private Sub ProcessNode(root As SyntaxNode, node As SyntaxNode, result As ArrayBuilder(Of TextSpan)) + Private Sub ProcessNode(node As SyntaxNode, result As ArrayBuilder(Of TextSpan)) If Not node.ContainsDiagnostics Then ' Don't bother going down nodes that don't have any syntax errors in them. Return @@ -46,19 +46,21 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup For Each child In node.ChildNodesAndTokens() If child.IsNode Then - ProcessNode(root, child.AsNode(), result) + ProcessNode(child.AsNode(), result) Else - ProcessToken(root, child.AsToken(), result) + ProcessToken(child.AsToken(), result) End If Next End Sub Private Sub ProcessToken(root As SyntaxNode, token As SyntaxToken, result As ArrayBuilder(Of TextSpan)) If token.ContainsDiagnostics Then - Dim parentMultiLineNode = If(GetMultiLineContainer(token.Parent), root) + Dim parentMultiLineNode = GetMultiLineContainer(token.Parent) - If ContainsMultiLineStringLiteral(parentMultiLineNode) Then - result.Add(parentMultiLineNode.FullSpan) + If parentMultiLineNode IsNot Nothing Then + If ContainsMultiLineStringLiteral(parentMultiLineNode) Then + result.Add(parentMultiLineNode.FullSpan) + End If End If End If End Sub From 0fd068f8ad6810731d1bfe2181d2372b8e079307 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 15:38:31 -0700 Subject: [PATCH 5/7] Handle multi-line interpolated string segments as well. --- .../Portable/CodeCleanup/VisualBasicCodeCleanerService.vb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index 2d451c977b1a2..36283965858ce 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -68,8 +68,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Private Function ContainsMultiLineStringLiteral(node As SyntaxNode) As Boolean Return node.DescendantTokens().Any( Function(t) - Return t.Kind() = SyntaxKind.StringLiteralToken AndAlso - Not VisualBasicSyntaxFactsService.Instance.IsOnSingleLine(t.Parent, fullSpan:=False) + If t.Kind() = SyntaxKind.StringLiteralToken OrElse + t.Kind() = SyntaxKind.InterpolatedStringTextToken Then + Return Not VisualBasicSyntaxFactsService.Instance.IsOnSingleLine(t.Parent, fullSpan:=False) + End If + + Return False End Function) End Function From f5cb6d6afef6091b7f521270479802cef739ae81 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 1 Apr 2017 16:05:51 -0700 Subject: [PATCH 6/7] Fix code. --- .../Portable/CodeCleanup/VisualBasicCodeCleanerService.vb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index 36283965858ce..183ff6a69eb5d 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -53,7 +53,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup Next End Sub - Private Sub ProcessToken(root As SyntaxNode, token As SyntaxToken, result As ArrayBuilder(Of TextSpan)) + Private Sub ProcessToken(token As SyntaxToken, result As ArrayBuilder(Of TextSpan)) If token.ContainsDiagnostics Then Dim parentMultiLineNode = GetMultiLineContainer(token.Parent) From 97a35ca5c839e039e4c5e998c888221ceb506739 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sun, 2 Apr 2017 17:20:33 -0700 Subject: [PATCH 7/7] Perform less work to determine the spans to avoid. --- .../CaseCorrectionServiceTests.vb | 3 +- .../CodeCleanup/ICodeCleanerService.cs | 4 +-- .../VisualBasicCodeCleanerService.vb | 32 ++++++++++++++----- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb b/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb index 786715bed6327..4681b22166035 100644 --- a/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb +++ b/src/EditorFeatures/VisualBasicTest/CaseCorrecting/CaseCorrectionServiceTests.vb @@ -35,7 +35,8 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CaseCorrecting Dim service = document.GetLanguageService(Of ICodeCleanerService) Dim newDocument = Await service.CleanupAsync( document, ImmutableArray.Create(span), - ImmutableArray.Create(Of ICodeCleanupProvider)(New CaseCorrectionCodeCleanupProvider())) + ImmutableArray.Create(Of ICodeCleanupProvider)(New CaseCorrectionCodeCleanupProvider()), + CancellationToken.None) newDocument.Project.Solution.Workspace.ApplyDocumentChanges(newDocument, CancellationToken.None) diff --git a/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs b/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs index 1459166d76a9b..ae7a4a01e9718 100644 --- a/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs +++ b/src/Workspaces/Core/Portable/CodeCleanup/ICodeCleanerService.cs @@ -25,13 +25,13 @@ internal interface ICodeCleanerService : ILanguageService /// /// This will run all provided code cleaners in an order that is given to the method. /// - Task CleanupAsync(Document document, ImmutableArray spans, ImmutableArray providers, CancellationToken cancellationToken = default(CancellationToken)); + Task CleanupAsync(Document document, ImmutableArray spans, ImmutableArray providers, CancellationToken cancellationToken); /// /// This will run all provided code cleaners in an order that is given to the method. /// /// This will do cleanups that don't require any semantic information. /// - Task CleanupAsync(SyntaxNode root, ImmutableArray spans, Workspace workspace, ImmutableArray providers, CancellationToken cancellationToken = default(CancellationToken)); + Task CleanupAsync(SyntaxNode root, ImmutableArray spans, Workspace workspace, ImmutableArray providers, CancellationToken cancellationToken); } } \ No newline at end of file diff --git a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb index 183ff6a69eb5d..141e875eedf7f 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeCleanup/VisualBasicCodeCleanerService.vb @@ -39,8 +39,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup End Function Private Sub ProcessNode(node As SyntaxNode, result As ArrayBuilder(Of TextSpan)) - If Not node.ContainsDiagnostics Then - ' Don't bother going down nodes that don't have any syntax errors in them. + If SkipProcessing(node, result) Then Return End If @@ -54,17 +53,34 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeCleanup End Sub Private Sub ProcessToken(token As SyntaxToken, result As ArrayBuilder(Of TextSpan)) - If token.ContainsDiagnostics Then - Dim parentMultiLineNode = GetMultiLineContainer(token.Parent) + If SkipProcessing(token, result) Then + Return + End If - If parentMultiLineNode IsNot Nothing Then - If ContainsMultiLineStringLiteral(parentMultiLineNode) Then - result.Add(parentMultiLineNode.FullSpan) - End If + Dim parentMultiLineNode = GetMultiLineContainer(token.Parent) + If parentMultiLineNode IsNot Nothing Then + If ContainsMultiLineStringLiteral(parentMultiLineNode) Then + result.Add(parentMultiLineNode.FullSpan) End If End If End Sub + Private Function SkipProcessing(nodeOrToken As SyntaxNodeOrToken, result As ArrayBuilder(Of TextSpan)) As Boolean + ' Don't bother looking at nodes or token that don't have any syntax errors in them. + If Not nodeOrToken.ContainsDiagnostics Then + Return True + End If + + If result.Count > 0 AndAlso result.Last.Contains(nodeOrToken.Span) Then + ' Don't bother looking at nodes or token that are contained within a span we've already + ' marked as something to avoid. We would only ever produce mark the same (or smaller) + ' span again. + Return True + End If + + Return False + End Function + Private Function ContainsMultiLineStringLiteral(node As SyntaxNode) As Boolean Return node.DescendantTokens().Any( Function(t)