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
21 changes: 21 additions & 0 deletions src/CodeStyle/CSharp/CodeFixes/CSharpFormattingCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.Composition;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Options;
using Microsoft.VisualStudio.CodingConventions;

namespace Microsoft.CodeAnalysis.CodeStyle
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.FixFormatting)]
[Shared]
internal class CSharpFormattingCodeFixProvider : AbstractFormattingCodeFixProvider
{
private readonly EditorConfigOptionsApplier _editorConfigOptionsApplier = new EditorConfigOptionsApplier();

protected override OptionSet ApplyFormattingOptions(OptionSet optionSet, ICodingConventionContext codingConventionContext)
{
return _editorConfigOptionsApplier.ApplyConventions(optionSet, codingConventionContext.CurrentConventions, LanguageNames.CSharp);
}
}
}
50 changes: 48 additions & 2 deletions src/CodeStyle/CSharp/Tests/FormattingAnalyzerTests.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// 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.IO;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Testing.Verifiers;
using Xunit;

namespace Microsoft.CodeAnalysis.CodeStyle
{
using Verify = CSharpCodeFixVerifier<CSharpFormattingAnalyzer, FormattingCodeFixProvider, XUnitVerifier>;
using Verify = CSharpCodeFixVerifier<CSharpFormattingAnalyzer, CSharpFormattingCodeFixProvider, XUnitVerifier>;

public class FormattingAnalyzerTests
{
Expand Down Expand Up @@ -184,7 +185,7 @@ class MyClass
}
";

await new CSharpCodeFixTest<CSharpFormattingAnalyzer, FormattingCodeFixProvider, XUnitVerifier>
await new CSharpCodeFixTest<CSharpFormattingAnalyzer, CSharpFormattingCodeFixProvider, XUnitVerifier>
{
TestCode = testCode,
FixedCode = fixedCode,
Expand All @@ -194,5 +195,50 @@ class MyClass
NumberOfIncrementalIterations = 2,
}.RunAsync();
}

[Fact]
public async Task TestEditorConfigUsed()
sharwell marked this conversation as resolved.
Show resolved Hide resolved
{
var testCode = @"
class MyClass {
void MyMethod()[| |]{
}
}
";
var fixedCode = @"
class MyClass {
void MyMethod()
{
}
}
";
var editorConfig = @"
root = true

[*.cs]
csharp_new_line_before_open_brace = methods
";

var testDirectoryName = Path.GetRandomFileName();
Directory.CreateDirectory(testDirectoryName);
try
{
File.WriteAllText(Path.Combine(testDirectoryName, ".editorconfig"), editorConfig);

// The contents of this file are ignored, but the coding conventions library checks for existence before
// .editorconfig is used.
File.WriteAllText(Path.Combine(testDirectoryName, "Test0.cs"), string.Empty);

await new CSharpCodeFixTest<CSharpFormattingAnalyzer, CSharpFormattingCodeFixProvider, XUnitVerifier>
{
TestState = { Sources = { (Path.GetFullPath(Path.Combine(testDirectoryName, "Test0.cs")), testCode) } },
FixedState = { Sources = { (Path.GetFullPath(Path.Combine(testDirectoryName, "Test0.cs")), fixedCode) } },
}.RunAsync();
}
finally
{
Directory.Delete(testDirectoryName, true);
}
}
}
}
4 changes: 2 additions & 2 deletions src/CodeStyle/Core/CodeFixes/FormattingCodeFixHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis
{
internal static class FormattingCodeFixHelper
{
internal static async Task<Document> FixOneAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
internal static async Task<Document> FixOneAsync(Document document, OptionSet options, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);

Expand All @@ -21,7 +22,6 @@ internal static async Task<Document> FixOneAsync(Document document, Diagnostic d
text.Lines[diagnosticLinePositionSpan.Start.Line].Start,
text.Lines[diagnosticLinePositionSpan.End.Line].End);

var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
return await Formatter.FormatAsync(document, spanToFormat, options, cancellationToken).ConfigureAwait(false);
}
}
Expand Down
53 changes: 42 additions & 11 deletions src/CodeStyle/Core/CodeFixes/FormattingCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -1,55 +1,86 @@
// 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 System.Composition;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Options;
using Microsoft.VisualStudio.CodingConventions;

namespace Microsoft.CodeAnalysis.CodeStyle
{
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic, Name = PredefinedCodeFixProviderNames.FixFormatting)]
[Shared]
internal class FormattingCodeFixProvider : CodeFixProvider
internal abstract class AbstractFormattingCodeFixProvider : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds
public sealed override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(IDEDiagnosticIds.FormattingDiagnosticId);

public override FixAllProvider GetFixAllProvider()
public sealed override FixAllProvider GetFixAllProvider()
{
return new FixAll();
return new FixAll(this);
}

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

return Task.CompletedTask;
}

protected abstract OptionSet ApplyFormattingOptions(OptionSet optionSet, ICodingConventionContext codingConventionContext);

private async Task<Document> FixOneAsync(CodeFixContext context, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var options = await GetOptionsAsync(context.Document, cancellationToken).ConfigureAwait(false);
return await FormattingCodeFixHelper.FixOneAsync(context.Document, options, diagnostic, cancellationToken).ConfigureAwait(false);
}

private async Task<OptionSet> GetOptionsAsync(Document document, CancellationToken cancellationToken)
{
OptionSet options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);

// The in-IDE workspace supports .editorconfig without special handling. However, the AdhocWorkspace used
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a test-only code? If so, can it be refactored into a test-only method?

Copy link
Member Author

@sharwell sharwell Oct 11, 2018

Choose a reason for hiding this comment

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

It's not strictly test-only code. It's addressing a limitation of the workspace which would naturally go away once we switch the code to using the new .editorconfig support in the compiler.

It could be considered test-only code from the perspective that removing the code would not produce a failure inside the IDE, but it could still produce failures in tools like AnalyzerRunner.

// in testing requires manual handling of .editorconfig.
if (document.Project.Solution.Workspace is AdhocWorkspace && File.Exists(document.FilePath ?? document.Name))
{
var codingConventionsManager = CodingConventionsManagerFactory.CreateCodingConventionsManager();
var codingConventionContext = await codingConventionsManager.GetConventionContextAsync(document.FilePath ?? document.Name, cancellationToken).ConfigureAwait(false);
options = ApplyFormattingOptions(options, codingConventionContext);
}

sharwell marked this conversation as resolved.
Show resolved Hide resolved
return options;
}

/// <summary>
/// Provide an optimized Fix All implementation that runs
/// <see cref="Formatter.FormatAsync(Document, Options.OptionSet, CancellationToken)"/> on the document(s)
/// included in the Fix All scope.
/// </summary>
private class FixAll : DocumentBasedFixAllProvider
sharwell marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly AbstractFormattingCodeFixProvider _formattingCodeFixProvider;

public FixAll(AbstractFormattingCodeFixProvider formattingCodeFixProvider)
{
_formattingCodeFixProvider = formattingCodeFixProvider;
}

protected override string CodeActionTitle => CodeStyleResources.Fix_formatting;

protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
{
var options = await document.GetOptionsAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
var options = await _formattingCodeFixProvider.GetOptionsAsync(document, fixAllContext.CancellationToken).ConfigureAwait(false);
var updatedDocument = await Formatter.FormatAsync(document, options, fixAllContext.CancellationToken).ConfigureAwait(false);
return await updatedDocument.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
' 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.Composition
Imports Microsoft.CodeAnalysis.CodeFixes
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.VisualStudio.CodingConventions

Namespace Microsoft.CodeAnalysis.CodeStyle
<ExportCodeFixProvider(LanguageNames.VisualBasic, Name:=PredefinedCodeFixProviderNames.FixFormatting)>
<[Shared]>
Friend Class VisualBasicFormattingCodeFixProvider
Inherits AbstractFormattingCodeFixProvider

Protected Overrides Function ApplyFormattingOptions(optionSet As OptionSet, codingConventionContext As ICodingConventionContext) As OptionSet
Return optionSet
End Function
End Class
End Namespace

2 changes: 1 addition & 1 deletion src/CodeStyle/VisualBasic/Tests/FormattingAnalyzerTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Imports Xunit
Imports VerifyVB = Microsoft.CodeAnalysis.VisualBasic.Testing.VisualBasicCodeFixVerifier(
Of Microsoft.CodeAnalysis.CodeStyle.VisualBasicFormattingAnalyzer,
Microsoft.CodeAnalysis.CodeStyle.FormattingCodeFixProvider,
Microsoft.CodeAnalysis.CodeStyle.VisualBasicFormattingCodeFixProvider,
Microsoft.CodeAnalysis.Testing.Verifiers.XUnitVerifier)

Namespace Microsoft.CodeAnalysis.CodeStyle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
foreach (var diagnostic in context.Diagnostics)
{
context.RegisterCodeFix(
new MyCodeAction(c => FormattingCodeFixHelper.FixOneAsync(context.Document, diagnostic, c)),
new MyCodeAction(c => FixOneAsync(context, diagnostic, c)),
diagnostic);
}

return Task.CompletedTask;
}

private static async Task<Document> FixOneAsync(CodeFixContext context, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var options = await context.Document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
return await FormattingCodeFixHelper.FixOneAsync(context.Document, options, diagnostic, cancellationToken).ConfigureAwait(false);
}

protected override async Task FixAllAsync(Document document, ImmutableArray<Diagnostic> diagnostics, SyntaxEditor editor, CancellationToken cancellationToken)
{
var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
Expand Down