dotnet / format

Home for the dotnet-format command
MIT License
1.92k stars 171 forks source link

Repro of files requiring 2 passes of dotnet format to fully format #2086

Open float3 opened 5 months ago

float3 commented 5 months ago

here is my repro https://github.com/float3/dotnet-format-repro

there are 2 cs files here that take two passes to fully format.

repro steps:

git clone git@github.com:float3/dotnet-format-repro.git
cd dotnet-format-repro
dotnet format *.sln
git add -A
dotnet format *.sln
git status
JoeRobich commented 5 months ago

Taking a look at the code that didn't fully format on the first pass.

Adding braces did not always anchor correctly.

first format:

image

second format:

image

This if statement did not get moved to a newline in the first pass, possibly because it is a single line. However, it became multi line when braces were added and is moved in the second pass.

first format:

image

second format:

image
sharwell commented 5 months ago

It appears the repository uses the following setting:

csharp_new_line_before_open_brace = none

This setting, along with some other non-default settings, have almost no test coverage due to mathematical impossibility described in https://github.com/dotnet/roslyn/issues/12306#issuecomment-1890077858.

We would likely accept a pull request to dotnet/roslyn to make the formatter work with a single pass, provided it did not result in an overly-complex implementation and included comprehensive test coverage for the alternative configurations. However, keep in mind that achieving these conditions may be exceedingly time consuming and generally impractical for most contributors.

sharwell commented 5 months ago

:memo: If this issue reproduces inside Visual Studio using its Format Document operation, the issue needs to be moved to dotnet/roslyn.

float3 commented 4 months ago

I don't use Visual Studio so can't test this

How can this be a "mathematical impossibility", do you mean it's computationally expensive? in the linked comment you say that "Item (1) has been demonstrated computationally impossible" but I don't see where that is shown

so far my take away is that it would require infinite lookahead or something similar.

sharwell commented 4 months ago

We have at least 50 options in the C# formatting engine. If each of these options had only two values (some have more), and testing the complete format engine on all coding structures took 1ms (it takes many seconds today), it would take over 35000 years to test all the combinations.

float3 commented 4 months ago

ah okay so you're saying testing is "impossible", not the option itself, that makes more sense