-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
WIP - Add fantomas formatting support #44801
base: main
Are you sure you want to change the base?
Changes from all commits
93b8cd0
94fa8f7
2c67f35
eb844bf
09a4ca5
046edaf
ebba7af
ab73ca0
f613920
49ad306
5c48e8d
bd1a2fd
cde04f6
6abc86f
df1ff38
51e2bab
51fdebc
60f780d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
using Microsoft.CodeAnalysis.Tools.Formatters; | ||
using Microsoft.CodeAnalysis.Tools.Utilities; | ||
using Microsoft.CodeAnalysis.Tools.Workspaces; | ||
using Microsoft.CodeAnalysis.Tools; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace Microsoft.CodeAnalysis.Tools | ||
|
@@ -279,5 +280,91 @@ private static async Task<Solution> RunCodeFormattersAsync( | |
formattableDocuments.AddRange(sourceGeneratedDocuments); | ||
return (projectFileCount + sourceGeneratedDocuments.Count, formattableDocuments.ToImmutable()); | ||
} | ||
|
||
public static bool AnyFSharpFiles(string directoryPath) | ||
{ | ||
foreach (var file in Directory.EnumerateFiles(directoryPath, "*.*", SearchOption.AllDirectories)) | ||
{ | ||
if (file.EndsWith(".fs", StringComparison.OrdinalIgnoreCase) || | ||
file.EndsWith(".fsx", StringComparison.OrdinalIgnoreCase) || | ||
file.EndsWith(".fsproj", StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
return true; // Early return if any F# file is found | ||
} | ||
} | ||
|
||
return false; // No F# files found after recursive search | ||
} | ||
|
||
// using Microsoft.DotNet.Tools.Tool.List; not possible at the moment, | ||
// will replicate records privately with the only data needed atm | ||
private record ToolListJsonContractInternal(string PackageId); | ||
private record ToolListResponseInternal(ToolListJsonContractInternal[] Data); | ||
|
||
public static async Task<bool> IsFantomasInstalled() | ||
{ | ||
var processStartInfo = new ProcessStartInfo | ||
{ | ||
FileName = "dotnet", | ||
Arguments = "tool list --local --format json", | ||
RedirectStandardOutput = true, | ||
RedirectStandardError = true, | ||
UseShellExecute = false, | ||
CreateNoWindow = true | ||
}; | ||
|
||
using var process = new Process { StartInfo = processStartInfo }; | ||
|
||
process.Start(); | ||
|
||
var outputString = process.StandardOutput.ReadToEnd(); | ||
|
||
var result = | ||
System.Text.Json.JsonSerializer.Deserialize<ToolListResponseInternal>( | ||
outputString | ||
); | ||
|
||
await process.WaitForExitAsync(); | ||
|
||
return result is not null | ||
&& result.Data.Any(r => r.PackageId.Contains("fantomas", StringComparison.OrdinalIgnoreCase)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deserializing json output |
||
} | ||
|
||
public static Task FantomasFormatAsync(FormatOptions formatOptions, ILogger logger) | ||
{ | ||
var processStartInfo = new ProcessStartInfo | ||
{ | ||
FileName = "dotnet", | ||
Arguments = $"fantomas {formatOptions.WorkspaceFilePath}", | ||
RedirectStandardOutput = true, | ||
RedirectStandardError = true, | ||
UseShellExecute = false, | ||
CreateNoWindow = true | ||
}; | ||
|
||
using var process = new Process { StartInfo = processStartInfo }; | ||
process.Start(); | ||
var output = process.StandardOutput.ReadToEnd(); | ||
|
||
// log fantomas output | ||
logger.LogInformation(output); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. log fantomas output |
||
|
||
return process.WaitForExitAsync(); | ||
} | ||
|
||
public static void LogFantomasInstallationInstructions(ILogger logger) | ||
{ | ||
var message = | ||
""" | ||
Formatting F# code is not natively supported; however, there is a community project called Fantomas that can format F# code. | ||
You can find more information about Fantomas at https://fsprojects.github.io/fantomas/docs/. | ||
You can install Fantomas via dotnet tools: | ||
|
||
> dotnet new tool-manifest [ only if no previous manifest is installed ] | ||
> dotnet tool install fantomas | ||
|
||
"""; | ||
logger.LogInformation(message); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated helper message for fantomas installation instructions |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,19 @@ public override async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio | |
|
||
formatOptions = formatOptions with { FixCategory = FixCategory.Whitespace | FixCategory.CodeStyle | FixCategory.Analyzers }; | ||
|
||
if (CodeFormatter.AnyFSharpFiles(formatOptions.WorkspaceFilePath)) | ||
{ | ||
var isFantomas = await CodeFormatter.IsFantomasInstalled(); | ||
if (!isFantomas) | ||
{ | ||
CodeFormatter.LogFantomasInstallationInstructions(logger); | ||
Comment on lines
+64
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If something attempts to run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually many times F# and C# project can be mixed within a single solution, so i wouldnt report an hard error? C# code can still be formatted. in my case i often have e.g. F# unit tests or integ tests on aspnet and maybe few F# modules, but most projects are C# There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm… ok, but if the user specifies the path of a F#-only project without Fantomas installed, then that deserves at least a warning, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes then i should log warning instead of info, right? |
||
} | ||
else | ||
{ | ||
await CodeFormatter.FantomasFormatAsync(formatOptions, logger).ConfigureAwait(false); | ||
} | ||
} | ||
|
||
return await FormatAsync(formatOptions, logger, cancellationToken).ConfigureAwait(false); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RedirectStandardError = true
might cause a deadlock if the child process outputs a lot of text to standard error and the parent process doesn't read fromprocess.StandardError
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall we switch it off? or what's the best solution for this item?