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

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

Open
float3 opened this issue Jan 30, 2024 · 6 comments
Open

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

float3 opened this issue Jan 30, 2024 · 6 comments
Assignees

Comments

@float3
Copy link

float3 commented Jan 30, 2024

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 [email protected]:float3/dotnet-format-repro.git
cd dotnet-format-repro
dotnet format *.sln
git add -A
dotnet format *.sln
git status
@JoeRobich
Copy link
Member

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
Copy link
Member

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 dotnet/roslyn#12306 (comment).

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 sharwell removed this from the NET8 Preview 3 milestone Feb 15, 2024
@sharwell
Copy link
Member

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

@float3
Copy link
Author

float3 commented Feb 21, 2024

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
Copy link
Member

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
Copy link
Author

float3 commented Feb 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants