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

Add Imports Formatter #693

Merged
merged 12 commits into from Jun 1, 2020
8 changes: 7 additions & 1 deletion docs/Supported-.editorconfig-options.md
Expand Up @@ -59,4 +59,10 @@ These formatting rules concern the use of space characters to format code.
These formatting rules concern the use of single lines versus separate lines for statements and code blocks.

- csharp_preserve_single_line_statements (default value: `true`)
- csharp_preserve_single_line_blocks (default value: `true`)
- csharp_preserve_single_line_blocks (default value: `true`)

**Organize using directives**
These formatting rules concern the sorting and display of using directives and Imports statements.

- dotnet_sort_system_directives_first (default value: `true`)
- dotnet_separate_import_directive_groups (default value: `true`)
1 change: 1 addition & 0 deletions src/CodeFormatter.cs
Expand Up @@ -27,6 +27,7 @@ internal static class CodeFormatter
new FinalNewlineFormatter(),
new EndOfLineFormatter(),
new CharsetFormatter(),
new ImportsFormatter(),
}.ToImmutableArray();

public static async Task<WorkspaceFormatResult> FormatWorkspaceAsync(
Expand Down
2 changes: 1 addition & 1 deletion src/Formatters/CharsetFormatter.cs
Expand Up @@ -20,7 +20,7 @@ internal sealed class CharsetFormatter : DocumentFormatter
private static Encoding Utf8 => new UTF8Encoding(encoderShouldEmitUTF8Identifier: false);
private static Encoding Latin1 => Encoding.GetEncoding("iso-8859-1");

protected override Task<SourceText> FormatFileAsync(
internal override Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
Expand Down
2 changes: 1 addition & 1 deletion src/Formatters/DocumentFormatter.cs
Expand Up @@ -38,7 +38,7 @@ internal abstract class DocumentFormatter : ICodeFormatter
/// <summary>
/// Applies formatting and returns the changed <see cref="SourceText"/> for a <see cref="Document"/>.
/// </summary>
protected abstract Task<SourceText> FormatFileAsync(
internal abstract Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
Expand Down
13 changes: 12 additions & 1 deletion src/Formatters/EndOfLineFormatter.cs
Expand Up @@ -15,7 +15,7 @@ internal sealed class EndOfLineFormatter : DocumentFormatter
{
protected override string FormatWarningDescription => Resources.Fix_end_of_line_marker;

protected override Task<SourceText> FormatFileAsync(
internal override Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
Expand Down Expand Up @@ -81,5 +81,16 @@ private static string GetEndOfLine(string endOfLineOption)
_ => Environment.NewLine,
};
}

internal static string GetEndOfLineOption(string newLine)
{
return newLine switch
{
"\n" => "lf",
"\r" => "cr",
"\r\n" => "crlf",
_ => GetEndOfLineOption(Environment.NewLine),
};
}
}
}
2 changes: 1 addition & 1 deletion src/Formatters/FinalNewlineFormatter.cs
Expand Up @@ -15,7 +15,7 @@ internal sealed class FinalNewlineFormatter : DocumentFormatter
{
protected override string FormatWarningDescription => Resources.Fix_final_newline;

protected override async Task<SourceText> FormatFileAsync(
internal override async Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
Expand Down
75 changes: 75 additions & 0 deletions src/Formatters/ImportsFormatter.cs
@@ -0,0 +1,75 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Text;
using Microsoft.Extensions.Logging;

namespace Microsoft.CodeAnalysis.Tools.Formatters
{
/// <summary>
/// ImportsFormatter that uses the <see cref="Formatter"/> to format document import directives.
/// </summary>
internal sealed class ImportsFormatter : DocumentFormatter
{
protected override string FormatWarningDescription => Resources.Fix_imports_ordering;
private readonly DocumentFormatter _endOfLineFormatter = new EndOfLineFormatter();

internal override async Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
AnalyzerConfigOptions? analyzerConfigOptions,
FormatOptions formatOptions,
ILogger logger,
CancellationToken cancellationToken)
{
try
{
var organizedDocument = await Formatter.OrganizeImportsAsync(document, cancellationToken);

var isSameVersion = await IsSameDocumentAndVersionAsync(document, organizedDocument, cancellationToken).ConfigureAwait(false);
if (isSameVersion)
{
return sourceText;
}

// Because the Formatter does not abide the `end_of_line` option we have to fix up the ends of the organized lines.
// See https://github.com/dotnet/roslyn/issues/44136
var organizedSourceText = await organizedDocument.GetTextAsync(cancellationToken);
return await _endOfLineFormatter.FormatFileAsync(organizedDocument, organizedSourceText, optionSet, analyzerConfigOptions, formatOptions, logger, cancellationToken);
}
catch (InsufficientExecutionStackException)
{
// This case is normally not hit when running against a handwritten code file.
// https://github.com/dotnet/roslyn/issues/44710#issuecomment-636253053
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably tell the user that the file was too large to format.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, although is it a size issue or an expression complexity issue. Perhaps can just be vague and say "Unable to organize file {filename}"

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the message the compiler gives here is expression is too complex

logger.LogWarning(Resources.Unable_to_organize_imports_for_0_The_document_is_too_complex, Path.GetFileName(document.FilePath));
return sourceText;
}
}

private static async Task<bool> IsSameDocumentAndVersionAsync(Document a, Document b, CancellationToken cancellationToken)
{
if (a == b)
{
return true;
}

if (a.Id != b.Id)
{
return false;
}

var aVersion = await a.GetTextVersionAsync(cancellationToken);
var bVersion = await b.GetTextVersionAsync(cancellationToken);

return aVersion == bVersion;
}
}
}

2 changes: 1 addition & 1 deletion src/Formatters/WhitespaceFormatter.cs
Expand Up @@ -17,7 +17,7 @@ internal sealed class WhitespaceFormatter : DocumentFormatter
{
protected override string FormatWarningDescription => Resources.Fix_whitespace_formatting;

protected override async Task<SourceText> FormatFileAsync(
internal override async Task<SourceText> FormatFileAsync(
Document document,
SourceText sourceText,
OptionSet optionSet,
Expand Down
2 changes: 1 addition & 1 deletion src/Logging/SimpleConsoleLoggerFactoryExtensions.cs
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.Extensions.Logging;
using System.CommandLine;
using Microsoft.Extensions.Logging;

namespace Microsoft.CodeAnalysis.Tools.Logging
{
Expand Down
2 changes: 1 addition & 1 deletion src/Logging/SimpleConsoleLoggerProvider.cs
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.Extensions.Logging;
using System.CommandLine;
using Microsoft.Extensions.Logging;

namespace Microsoft.CodeAnalysis.Tools.Logging
{
Expand Down
8 changes: 7 additions & 1 deletion src/Resources.resx
Expand Up @@ -249,7 +249,13 @@
<data name="The_files_option_is_deprecated_Use_the_include_option_instead" xml:space="preserve">
<value>The `--files` option is deprecated. Use the `--include` option instead.</value>
</data>
<data name="The_dry_run_option_is_deprecated_Use_the_check_option_instead" xml:space="preserve">
<data name="The_dry_run_option_is_deprecated_Use_the_check_option_instead" xml:space="preserve">
<value>The `--dry-run` option is deprecated. Use the `--check` option instead.</value>
</data>
<data name="Fix_imports_ordering" xml:space="preserve">
<value>Fix imports ordering.</value>
</data>
<data name="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<value>Unable to organize imports for '{0}'. The document is too complex.</value>
</data>
</root>
2 changes: 1 addition & 1 deletion src/Utilities/EditorConfigFinder.cs
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.IO;
using System.Collections.Immutable;
using System.IO;
using System.Linq;

namespace Microsoft.CodeAnalysis.Tools.Utilities
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.cs.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.de.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.es.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.fr.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.it.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.ja.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.ko.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/xlf/Resources.pl.xlf
Expand Up @@ -77,6 +77,11 @@
<target state="new">Fix final newline.</target>
<note />
</trans-unit>
<trans-unit id="Fix_imports_ordering">
<source>Fix imports ordering.</source>
<target state="new">Fix imports ordering.</target>
<note />
</trans-unit>
<trans-unit id="Fix_whitespace_formatting">
<source>Fix whitespace formatting.</source>
<target state="new">Fix whitespace formatting.</target>
Expand Down Expand Up @@ -207,6 +212,11 @@
<target state="new">Unable to locate dotnet CLI. Ensure that it is on the PATH.</target>
<note />
</trans-unit>
<trans-unit id="Unable_to_organize_imports_for_0_The_document_is_too_complex">
<source>Unable to organize imports for '{0}'. The document is too complex.</source>
<target state="new">Unable to organize imports for '{0}'. The document is too complex.</target>
<note />
</trans-unit>
<trans-unit id="Using_msbuildexe_located_in_0">
<source>Using MSBuild.exe located in '{0}'.</source>
<target state="new">Using MSBuild.exe located in '{0}'.</target>
Expand Down