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

Address the performance issues for bulk suppression in IDE. #6096

Merged
merged 1 commit into from
Oct 21, 2015
Merged

Address the performance issues for bulk suppression in IDE. #6096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 6 additions & 9 deletions src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixAllTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,8 @@ class Class2

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""member"", Target = ""~M:Class1.Method~System.Int32"")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class1"")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class2"")]

".Replace("<", "&lt;").Replace(">", "&gt;");
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class2"")]"
.Replace("<", "&lt;").Replace(">", "&gt;");

var expected = @"
<Workspace>
Expand Down Expand Up @@ -483,9 +482,8 @@ class Class2
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""member"", Target = ""~M:Class1.Method~System.Int32"")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class1"")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class2"")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class3"")]

".Replace("<", "&lt;").Replace(">", "&gt;");
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"", Scope = ""type"", Target = ""~T:Class3"")]"
.Replace("<", "&lt;").Replace(">", "&gt;");

var expected = @"
<Workspace>
Expand Down Expand Up @@ -710,9 +708,8 @@ class Class2
// Project-level suppressions either have no target or are given
// a specific target and scoped to a namespace, type, member, etc.

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""NoLocationDiagnostic"", ""NoLocationDiagnostic:NoLocationDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"")]

".Replace("<", "&lt;").Replace(">", "&gt;");
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""NoLocationDiagnostic"", ""NoLocationDiagnostic:NoLocationDiagnostic"", Justification = ""{FeaturesResources.SuppressionPendingJustification}"")]"
.Replace("<", "&lt;").Replace(">", "&gt;");

var expected = @"
<Workspace>
Expand Down
4 changes: 2 additions & 2 deletions src/EditorFeatures/Core/Implementation/Suggestions/FixAllGetFixesService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public async Task<Solution> GetFixAllChangedSolutionAsync(FixAllProvider fixAllP
}

fixAllContext.CancellationToken.ThrowIfCancellationRequested();
return await codeAction.GetChangedSolutionInternalAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
return await codeAction.GetChangedSolutionInternalAsync(cancellationToken: fixAllContext.CancellationToken).ConfigureAwait(false);
}

public async Task<IEnumerable<CodeActionOperation>> GetFixAllOperationsAsync(FixAllProvider fixAllProvider, FixAllContext fixAllContext, string fixAllTitle, string waitDialogMessage, bool showPreviewChangesDialog)
Expand Down Expand Up @@ -126,7 +126,7 @@ private async Task<IEnumerable<CodeActionOperation>> GetFixAllOperationsAsync(Co
}

cancellationToken.ThrowIfCancellationRequested();
var newSolution = await codeAction.GetChangedSolutionInternalAsync(cancellationToken).ConfigureAwait(false);
var newSolution = await codeAction.GetChangedSolutionInternalAsync(cancellationToken: cancellationToken).ConfigureAwait(false);

if (showPreviewChangesDialog)
{
Expand Down
3 changes: 2 additions & 1 deletion src/EditorFeatures/Core/Implementation/Suggestions/FixMultipleSuggestedAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public Solution GetChangedSolution(CancellationToken cancellationToken)
var extensionManager = this.Workspace.Services.GetService<IExtensionManager>();
extensionManager.PerformAction(Provider, () =>
{
newSolution = CodeAction.GetChangedSolutionInternalAsync(cancellationToken).WaitAndGetResult(cancellationToken);
// We don't need to post process changes here as the inner code action created for Fix multiple code fix already executes.
newSolution = CodeAction.GetChangedSolutionInternalAsync(postProcessChanges: false, cancellationToken: cancellationToken).WaitAndGetResult(cancellationToken);
});

return newSolution;
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/VisualBasicTest/Diagnostics/Suppression/SuppressionTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,7 @@ End Class]]>
' a specific target and scoped to a namespace, type, member, etc.

<Assembly: Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""<Pending>"", Scope:=""type"", Target:=""Class1"")>
<Assembly: Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
<Assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
"

Test(source.ToString(), expected)
Expand Down Expand Up @@ -1518,7 +1518,7 @@ End Class
' Project-level suppressions either have no target or are given
' a specific target and scoped to a namespace, type, member, etc.

<Assembly: Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
<Assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
"

Test(source.ToString(), expected)
Expand Down Expand Up @@ -1564,7 +1564,7 @@ End Class
' a specific target and scoped to a namespace, type, member, etc.

<Assembly: Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""<Pending>"", Scope:=""type"", Target:=""Class1"")>
<Assembly: Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
<Assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""InfoDiagnostic"", ""InfoDiagnostic:InfoDiagnostic"", Justification:=""{FeaturesResources.SuppressionPendingJustification}"", Scope:=""type"", Target:=""~T:Class2"")>
"

Test(source.ToString(), expected)
Expand Down
12 changes: 5 additions & 7 deletions src/Features/CSharp/Portable/CodeFixes/Suppression/CSharpSuppressionCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
using System.Composition;
using System.Globalization;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CodeFixes.Suppression;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Simplification;

namespace Microsoft.CodeAnalysis.CSharp.CodeFixes.Suppression
{
Expand Down Expand Up @@ -92,14 +92,15 @@ protected override bool IsEndOfFileToken(SyntaxToken token)
return token.Kind() == SyntaxKind.EndOfFileToken;
}

protected override SyntaxNode AddGlobalSuppressMessageAttribute(SyntaxNode newRoot, ISymbol targetSymbol, Diagnostic diagnostic)
protected override SyntaxNode AddGlobalSuppressMessageAttribute(SyntaxNode newRoot, ISymbol targetSymbol, Diagnostic diagnostic, Workspace workspace, CancellationToken cancellationToken)
{
var compilationRoot = (CompilationUnitSyntax)newRoot;
var isFirst = !compilationRoot.AttributeLists.Any();
var leadingTriviaForAttributeList = isFirst && !compilationRoot.HasLeadingTrivia ?
SyntaxFactory.TriviaList(SyntaxFactory.Comment(GlobalSuppressionsFileHeaderComment)) :
default(SyntaxTriviaList);
var attributeList = CreateAttributeList(targetSymbol, diagnostic, leadingTrivia: leadingTriviaForAttributeList, needsLeadingEndOfLine: !isFirst);
attributeList = (AttributeListSyntax)Formatter.Format(attributeList, workspace, cancellationToken: cancellationToken);
return compilationRoot.AddAttributeLists(attributeList);
}

Expand All @@ -110,8 +111,7 @@ private AttributeListSyntax CreateAttributeList(
bool needsLeadingEndOfLine)
{
var attributeArguments = CreateAttributeArguments(targetSymbol, diagnostic);
var attribute = SyntaxFactory.Attribute(SyntaxFactory.ParseName(SuppressMessageAttributeName), attributeArguments)
.WithAdditionalAnnotations(Simplifier.Annotation);
var attribute = SyntaxFactory.Attribute(SyntaxFactory.ParseName(SuppressMessageAttributeName), attributeArguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

how about doing minimal type name here manually as well like you did for formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What API can I use to get the minimal type name? SuppressMessageAttributeName = System.Diagnostics.CodeAnalysis.SuppressMessage.

Copy link
Contributor

Choose a reason for hiding this comment

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

get symbol for suppress attribute (using GetSymbolBymetadata ...) and symbol has ToMinimallyQualifed ... method in it.

also, for global suppression, since you are putting all those attribute to same context, get minimallyQualifed string once, and reuse them.

var attributes = new SeparatedSyntaxList<AttributeSyntax>().Add(attribute);

var targetSpecifier = SyntaxFactory.AttributeTargetSpecifier(SyntaxFactory.Token(SyntaxKind.AssemblyKeyword));
Expand All @@ -124,9 +124,7 @@ private AttributeListSyntax CreateAttributeList(
triviaList = triviaList.Add(endOfLineTrivia);
}

return attributeList
.WithLeadingTrivia(leadingTrivia.AddRange(triviaList))
.WithAdditionalAnnotations(Formatter.Annotation);
return attributeList.WithLeadingTrivia(leadingTrivia.AddRange(triviaList));
}

private AttributeArgumentListSyntax CreateAttributeArguments(ISymbol targetSymbol, Diagnostic diagnostic)
Expand Down
24 changes: 2 additions & 22 deletions .../Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.FixAllProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +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.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CodeFixes.Suppression
Expand Down Expand Up @@ -50,7 +44,7 @@ public async override Task<CodeAction> GetFixAsync(FixAllContext fixAllContext)

return !isGlobalSuppression ?
await batchFixer.GetFixAsync(documentsAndDiagnosticsToFixMap, fixAllContext).ConfigureAwait(false) :
CreateGlobalSuppressionFixAllAction(title, suppressionFixer, fixAllContext.Document, documentsAndDiagnosticsToFixMap);
GlobalSuppressMessageFixAllCodeAction.Create(title, suppressionFixer, fixAllContext.Document, documentsAndDiagnosticsToFixMap);
}
else
{
Expand All @@ -60,23 +54,9 @@ await batchFixer.GetFixAsync(documentsAndDiagnosticsToFixMap, fixAllContext).Con

return !isGlobalSuppression ?
await batchFixer.GetFixAsync(projectsAndDiagnosticsToFixMap, fixAllContext).ConfigureAwait(false) :
CreateGlobalSuppressionFixAllAction(title, suppressionFixer, fixAllContext.Project, projectsAndDiagnosticsToFixMap);
GlobalSuppressMessageFixAllCodeAction.Create(title, suppressionFixer, fixAllContext.Project, projectsAndDiagnosticsToFixMap);
}
}

private static CodeAction CreateGlobalSuppressionFixAllAction(string title, AbstractSuppressionCodeFixProvider fixer, Document triggerDocument, ImmutableDictionary<Document, ImmutableArray<Diagnostic>> diagnosticsByDocument)
{
return new CodeAction.SolutionChangeAction(title,
ct => GlobalSuppressMessageFixAllCodeAction.CreateChangedSolutionAsync(fixer, triggerDocument, diagnosticsByDocument, ct),
equivalenceKey: title);
}

private static CodeAction CreateGlobalSuppressionFixAllAction(string title, AbstractSuppressionCodeFixProvider fixer, Project triggerProject, ImmutableDictionary<Project, ImmutableArray<Diagnostic>> diagnosticsByProject)
{
return new CodeAction.SolutionChangeAction(title,
ct => GlobalSuppressMessageFixAllCodeAction.CreateChangedSolutionAsync(fixer, triggerProject, diagnosticsByProject, ct),
equivalenceKey: title);
}
}
}
}
3 changes: 2 additions & 1 deletion ...deFixes/Suppression/AbstractSuppressionCodeFixProvider.GlobalSuppressMessageCodeAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ public GlobalSuppressMessageCodeAction(ISymbol targetSymbol, Project project, Di
protected override async Task<Document> GetChangedSuppressionDocumentAsync(CancellationToken cancellationToken)
{
var suppressionsDoc = await GetOrCreateSuppressionsDocumentAsync(cancellationToken).ConfigureAwait(false);
var workspace = suppressionsDoc.Project.Solution.Workspace;
var suppressionsRoot = await suppressionsDoc.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var semanticModel = await suppressionsDoc.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
suppressionsRoot = Fixer.AddGlobalSuppressMessageAttribute(suppressionsRoot, _targetSymbol, _diagnostic);
suppressionsRoot = Fixer.AddGlobalSuppressMessageAttribute(suppressionsRoot, _targetSymbol, _diagnostic, workspace, cancellationToken);
return suppressionsDoc.WithSyntaxRoot(suppressionsRoot);
}

Expand Down
41 changes: 36 additions & 5 deletions ...s/Suppression/AbstractSuppressionCodeFixProvider.GlobalSuppressMessageFixAllCodeAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +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.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CodeFixes.Suppression
Expand All @@ -14,14 +16,42 @@ internal abstract partial class AbstractSuppressionCodeFixProvider : ISuppressio
internal sealed class GlobalSuppressMessageFixAllCodeAction : AbstractGlobalSuppressMessageCodeAction
{
private readonly IEnumerable<KeyValuePair<ISymbol, ImmutableArray<Diagnostic>>> _diagnosticsBySymbol;

private GlobalSuppressMessageFixAllCodeAction(AbstractSuppressionCodeFixProvider fixer, IEnumerable<KeyValuePair<ISymbol, ImmutableArray<Diagnostic>>> diagnosticsBySymbol, Project project)
: base(fixer, project)
{
_diagnosticsBySymbol = diagnosticsBySymbol;
}

internal static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppressionCodeFixProvider fixer, Document triggerDocument, ImmutableDictionary<Document, ImmutableArray<Diagnostic>> diagnosticsByDocument, CancellationToken cancellationToken)
internal static CodeAction Create(string title, AbstractSuppressionCodeFixProvider fixer, Document triggerDocument, ImmutableDictionary<Document, ImmutableArray<Diagnostic>> diagnosticsByDocument)
{
return new GlobalSuppressionSolutionChangeAction(title,
ct => CreateChangedSolutionAsync(fixer, triggerDocument, diagnosticsByDocument, ct),
equivalenceKey: title);
}

internal static CodeAction Create(string title, AbstractSuppressionCodeFixProvider fixer, Project triggerProject, ImmutableDictionary<Project, ImmutableArray<Diagnostic>> diagnosticsByProject)
{
return new GlobalSuppressionSolutionChangeAction(title,
ct => CreateChangedSolutionAsync(fixer, triggerProject, diagnosticsByProject, ct),
equivalenceKey: title);
}

private class GlobalSuppressionSolutionChangeAction : SolutionChangeAction
{
public GlobalSuppressionSolutionChangeAction(string title, Func<CancellationToken, Task<Solution>> createChangedSolution, string equivalenceKey)
: base(title, createChangedSolution, equivalenceKey)
{
}

protected override Task<Document> PostProcessChangesAsync(Document document, CancellationToken cancellationToken)
{
// PERF: We don't to formatting on the entire global suppressions document, but instead do it for each attribute individual in the fixer.
return Task.FromResult(document);
}
}

private static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppressionCodeFixProvider fixer, Document triggerDocument, ImmutableDictionary<Document, ImmutableArray<Diagnostic>> diagnosticsByDocument, CancellationToken cancellationToken)
{
var currentSolution = triggerDocument.Project.Solution;
foreach (var grouping in diagnosticsByDocument.GroupBy(d => d.Key.Project))
Expand All @@ -40,7 +70,7 @@ internal static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppress
return currentSolution;
}

internal static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppressionCodeFixProvider fixer, Project triggerProject, ImmutableDictionary<Project, ImmutableArray<Diagnostic>> diagnosticsByProject, CancellationToken cancellationToken)
private static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppressionCodeFixProvider fixer, Project triggerProject, ImmutableDictionary<Project, ImmutableArray<Diagnostic>> diagnosticsByProject, CancellationToken cancellationToken)
{
var currentSolution = triggerProject.Solution;
foreach (var kvp in diagnosticsByProject)
Expand All @@ -65,8 +95,9 @@ internal static async Task<Solution> CreateChangedSolutionAsync(AbstractSuppress
protected override async Task<Document> GetChangedSuppressionDocumentAsync(CancellationToken cancellationToken)
{
var suppressionsDoc = await GetOrCreateSuppressionsDocumentAsync(cancellationToken).ConfigureAwait(false);
var workspace = suppressionsDoc.Project.Solution.Workspace;
var suppressionsRoot = await suppressionsDoc.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

foreach (var kvp in _diagnosticsBySymbol)
{
var targetSymbol = kvp.Key;
Expand All @@ -75,7 +106,7 @@ protected override async Task<Document> GetChangedSuppressionDocumentAsync(Cance
foreach (var diagnostic in diagnostics)
{
Contract.ThrowIfFalse(!diagnostic.IsSuppressed);
suppressionsRoot = Fixer.AddGlobalSuppressMessageAttribute(suppressionsRoot, targetSymbol, diagnostic);
suppressionsRoot = Fixer.AddGlobalSuppressMessageAttribute(suppressionsRoot, targetSymbol, diagnostic, workspace, cancellationToken);
}
}

Expand Down
Loading