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.

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

Formatting analyzer #28873

Merged
merged 27 commits into from
Oct 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
864e937
Implement a formatting analyzer
sharwell Jul 27, 2018
ba0f5e1
Implement a copy of the formatting analyzer in the CodeStyle layer
sharwell Aug 29, 2018
83c76f7
Update the CodeStyle layer to reference the 2.8.2 release
sharwell Aug 29, 2018
0c4f655
Work around lack of Workspaces layer during command line builds
sharwell Aug 29, 2018
0dc9b96
Move version reference to Packages.props
sharwell Aug 29, 2018
d54ee1d
Minor project configuration updates to make Build Boss happy
sharwell Sep 18, 2018
79120bd
Update the code style package to provide code fixes and VB support
sharwell Sep 19, 2018
82fda81
Update the formatting analyzer to IDE0054
sharwell Sep 19, 2018
73f952d
Fix incorrect InternalsVisibleTo lines in code style packages
sharwell Sep 19, 2018
8a82977
Add a few initial tests for the formatter analyzer
sharwell Sep 19, 2018
75c90a0
Code cleanup based on review feedback
sharwell Sep 19, 2018
1124126
Avoid storing code fixes in the diagnostic property bags
sharwell Sep 19, 2018
14d31d0
Updates from code review feedback
sharwell Oct 4, 2018
3d98f8f
Merge remote-tracking branch 'dotnet/dev16-preview1' into formatting-…
sharwell Oct 4, 2018
dc1bfba
Disable VerifyAllSymbolEndActionsExecuted
sharwell Oct 4, 2018
e05b2d5
Share code in implementation of AbstractCodeStyleDiagnosticAnalyzer
sharwell Oct 9, 2018
9ce3c05
Code cleanup from review feedback
sharwell Oct 9, 2018
1b402fb
Share code in FixAllContextHelper and FixAllContext.DiagnosticProvider
sharwell Oct 9, 2018
24da46e
Reduce contention in GetDocumentDiagnosticsToFixAsync
sharwell Oct 9, 2018
c539707
Fix expected test output following behavior change in 1124126
sharwell Oct 9, 2018
7edc553
Share code in formatting analyzer and code fix
sharwell Oct 9, 2018
031cd69
Fix resource string names for style consistency
sharwell Oct 11, 2018
58d8c6c
Merge remote-tracking branch 'dotnet/dev16.0.x' into formatting-analyzer
sharwell Oct 11, 2018
45877d3
Remove workaround for #30309 since it was fixed separately
sharwell Oct 11, 2018
8a88bc9
Add test for formatting code fix relying on .editorconfig
sharwell Oct 11, 2018
2ccd302
Allow the formatter to report NOP text changes
sharwell Oct 11, 2018
a4a1961
Document reflection load pattern in derived types
sharwell Oct 12, 2018
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
Prev Previous commit
Next Next commit
Avoid storing code fixes in the diagnostic property bags
  • Loading branch information
sharwell committed Sep 19, 2018
commit 1124126a863e106c4577c457cf3301f5dca1ec9c
29 changes: 29 additions & 0 deletions src/CodeStyle/CSharp/Tests/FormattingAnalyzerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,34 @@ void MyMethod()

await Verify.VerifyCodeFixAsync(testCode, fixedCode);
}

[Fact]
public async Task TestIncrementalFixesFullLine()
{
var testCode = @"
class MyClass
{
int Property1$${$$get;$$set;$$}
int Property2$${$$get;$$}
}
";
var fixedCode = @"
class MyClass
{
int Property1 { get; set; }
int Property2 { get; }
}
";

await new CSharpCodeFixTest<CSharpFormattingAnalyzer, FormattingCodeFixProvider, XUnitVerifier>
{
TestCode = testCode,
FixedCode = fixedCode,

// Each application of a single code fix covers all diagnostics on the same line. In total, two lines
// require changes so the number of incremental iterations is exactly 2.
NumberOfIncrementalIterations = 2,
}.RunAsync();
}
}
}
18 changes: 1 addition & 17 deletions src/CodeStyle/Core/CodeFixes/AbstractFormattingAnalyzerImpl.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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 System.IO;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Formatting;
Expand All @@ -13,11 +12,6 @@ namespace Microsoft.CodeAnalysis.CodeStyle
{
internal abstract class AbstractFormattingAnalyzerImpl
{
public static readonly string ReplaceTextKey = nameof(ReplaceTextKey);

public static readonly ImmutableDictionary<string, string> RemoveTextProperties =
ImmutableDictionary.Create<string, string>().Add(ReplaceTextKey, "");

private readonly DiagnosticDescriptor _descriptor;

protected AbstractFormattingAnalyzerImpl(DiagnosticDescriptor descriptor)
Expand Down Expand Up @@ -87,23 +81,13 @@ private void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context, Workspace work
throw new InvalidOperationException("This program location is thought to be unreachable.");
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}

ImmutableDictionary<string, string> properties;
if (change.NewText.Length == 0)
{
properties = RemoveTextProperties;
}
else
{
properties = ImmutableDictionary.Create<string, string>().Add(ReplaceTextKey, change.NewText);
}

var location = Location.Create(tree, change.Span);
context.ReportDiagnostic(DiagnosticHelper.Create(
sharwell marked this conversation as resolved.
Show resolved Hide resolved
_descriptor,
location,
ReportDiagnostic.Default,
additionalLocations: null,
properties));
properties: null));
}
}
}
Expand Down
39 changes: 19 additions & 20 deletions src/CodeStyle/Core/CodeFixes/FormattingCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// 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.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Threading;
Expand All @@ -26,34 +25,34 @@ public override FixAllProvider GetFixAllProvider()

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
context.RegisterCodeFix(
CodeAction.Create(
CodeStyleFixesResources.Formatting_analyzer_code_fix,
c => FixOneAsync(context.Document, context.Diagnostics, c),
nameof(FormattingCodeFixProvider)),
context.Diagnostics);
foreach (var diagnostic in context.Diagnostics)
{
context.RegisterCodeFix(
CodeAction.Create(
CodeStyleFixesResources.Formatting_analyzer_code_fix,
c => FixOneAsync(context.Document, diagnostic, c),
nameof(FormattingCodeFixProvider)),
diagnostic);
}

return Task.CompletedTask;
}

protected async Task<Document> FixOneAsync(Document document, ImmutableArray<Diagnostic> diagnostics, CancellationToken cancellationToken)
protected async Task<Document> FixOneAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var changes = new List<TextChange>();
foreach (var diagnostic in diagnostics)
{
if (!diagnostic.Properties.TryGetValue(AbstractFormattingAnalyzerImpl.ReplaceTextKey, out var replacement))
{
continue;
}

changes.Add(new TextChange(diagnostic.Location.SourceSpan, replacement));
}
// The span to format is the full line(s) containing the diagnostic
Copy link
Member

Choose a reason for hiding this comment

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

out of curiosity, why?

Copy link
Member Author

@sharwell sharwell Oct 7, 2018

Choose a reason for hiding this comment

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

Intent is avoiding the need to apply three separate fixes to this code:

int Property{get;}

My hypothesis is the special behavior here would be well-received, but we can always change it before release if people don't agree.

var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var diagnosticSpan = diagnostic.Location.SourceSpan;
var diagnosticLinePositionSpan = text.Lines.GetLinePositionSpan(diagnosticSpan);
var spanToFormat = TextSpan.FromBounds(
text.Lines[diagnosticLinePositionSpan.Start.Line].Start,
text.Lines[diagnosticLinePositionSpan.End.Line].End);

changes.Sort((left, right) => left.Span.Start.CompareTo(right.Span.Start));

sharwell marked this conversation as resolved.
Show resolved Hide resolved
return document.WithText(text.WithChanges(changes));
var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
return await Formatter.FormatAsync(document, spanToFormat, options, cancellationToken).ConfigureAwait(false);
}

private class FixAll : DocumentBasedFixAllProvider
sharwell marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
33 changes: 16 additions & 17 deletions src/Features/Core/Portable/Formatting/FormattingCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.Composition;
using System.Threading;
Expand All @@ -23,31 +22,31 @@ public override ImmutableArray<string> FixableDiagnosticIds

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
context.RegisterCodeFix(
new MyCodeAction(c => FixOneAsync(context.Document, context.Diagnostics, c)),
context.Diagnostics);
foreach (var diagnostic in context.Diagnostics)
{
context.RegisterCodeFix(
new MyCodeAction(c => FixOneAsync(context.Document, diagnostic, c)),
diagnostic);
}

return Task.CompletedTask;
}

protected async Task<Document> FixOneAsync(Document document, ImmutableArray<Diagnostic> diagnostics, CancellationToken cancellationToken)
protected async Task<Document> FixOneAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
sharwell marked this conversation as resolved.
Show resolved Hide resolved
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var changes = new List<TextChange>();
foreach (var diagnostic in diagnostics)
{
if (!diagnostic.Properties.TryGetValue(FormattingDiagnosticAnalyzer.ReplaceTextKey, out var replacement))
{
continue;
}

changes.Add(new TextChange(diagnostic.Location.SourceSpan, replacement));
}
// The span to format is the full line(s) containing the diagnostic
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var diagnosticSpan = diagnostic.Location.SourceSpan;
var diagnosticLinePositionSpan = text.Lines.GetLinePositionSpan(diagnosticSpan);
var spanToFormat = TextSpan.FromBounds(
text.Lines[diagnosticLinePositionSpan.Start.Line].Start,
text.Lines[diagnosticLinePositionSpan.End.Line].End);
sharwell marked this conversation as resolved.
Show resolved Hide resolved

changes.Sort((left, right) => left.Span.Start.CompareTo(right.Span.Start));

return document.WithText(text.WithChanges(changes));
var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
return await Formatter.FormatAsync(document, spanToFormat, options, cancellationToken).ConfigureAwait(false);
}

protected override async Task FixAllAsync(Document document, ImmutableArray<Diagnostic> diagnostics, SyntaxEditor editor, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// 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.Collections.Immutable;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -12,11 +11,6 @@ namespace Microsoft.CodeAnalysis.Formatting
internal class FormattingDiagnosticAnalyzer
: AbstractCodeStyleDiagnosticAnalyzer
sharwell marked this conversation as resolved.
Show resolved Hide resolved
{
public static readonly string ReplaceTextKey = nameof(ReplaceTextKey);

public static readonly ImmutableDictionary<string, string> RemoveTextProperties =
ImmutableDictionary.Create<string, string>().Add(ReplaceTextKey, "");

public FormattingDiagnosticAnalyzer()
: base(
IDEDiagnosticIds.FormattingDiagnosticId,
Expand Down Expand Up @@ -91,23 +85,13 @@ private void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context)
throw ExceptionUtilities.Unreachable;
}
sharwell marked this conversation as resolved.
Show resolved Hide resolved

ImmutableDictionary<string, string> properties;
if (change.NewText.Length == 0)
{
properties = RemoveTextProperties;
}
else
{
properties = ImmutableDictionary.Create<string, string>().Add(ReplaceTextKey, change.NewText);
}

var location = Location.Create(tree, change.Span);
context.ReportDiagnostic(DiagnosticHelper.Create(
Descriptor,
location,
ReportDiagnostic.Default,
additionalLocations: null,
properties));
properties: null));
}
}
}
Expand Down