format icon indicating copy to clipboard operation
format copied to clipboard

[v6] dotnet format don't respect --include option

Open alirezanet opened this issue 3 years ago • 9 comments

Hi, I'm the maintainer of Husky.Net tool, In big projects using dotnet-format is a real pain, it takes a lot of time to complete. This is one of the reasons that I created Husky.Net. usually, people only want to format changed files( or staged files). In the previous versions (e.g v5) we could've easily limited the dotnet-format working area to the specific files using --include but in the last version looks like dotnet-format doing something else in the background and don't respect that option.

dotnet format --include folder/targetFile.cs folder/targetFile2.cs

The expected behavior is ignoring everything else except the target files. or providing an option to change its behavior. This issue makes dotnet-format v6 almost unusable, how can I fix this problem or change its behavior? Thank you

alirezanet avatar Dec 25 '21 16:12 alirezanet

Hey guys, any updates on this?

I suppose that @alirezanet means that include doesn't do anything? If so, I believe I can confirm:

For example:

dotnet format solution.sln --include some-script-in-this-folder.sh -v d

results in lots of logs including projects that are completely irrevant:

...
Project ApiTestClient is using configuration from 'C:\projects\PROJECT-NAME\.editorconfig'.
  Project ApiTestClient is using configuration from 'C:\projects\PROJECT-NAME\src\Utilities\ApiTestClient\obj\Debug\netcoreapp3.1\ApiTestClient.GeneratedMSBuildEditorConfig.editorconfig'.
...
Running 1 analyzers on ApiTestClient.
...

It's pretty slow even though it should format 1 script.. which is not related to any of my real c# projects..

I also tried stuff like --include ./some-script.sh but that also makes no difference. Is that syntax supported? Powershell prefers it that way...

sander1095 avatar Mar 18 '22 15:03 sander1095

@sander1095 The --include option is useful for narrowing which files from your solution or project you want to format. We do not support adding files outside of you solution or project to the list of files to format. Currently dotnet-format supports formatting C# and VB.NET files only.

@alirezanet The issue seems to be with how the CLI is passing this option to our tool. I believe you can use dotnet format --include=folder/targetFile.cs folder/targetFile2.cs as a workaround.

JoeRobich avatar May 19 '22 17:05 JoeRobich

@JoeRobich Hey Joe! Giving a quick reply here.. I guess my example is bad, because when I use --include with actual supported files in my solution, it's really slow.

So.. to improve my example:

dotnet format solution.sln --include src/WebApi/Startup.cs

still results in other non-relevant projects being built/analyzed..

One workaround is simply using dotnet format whitespace which is super quick, but kinda defeats the point of using editorconfig and specific dotnet rules..


Also, what do you mean with the workaround? How is that different than what @alirezanet is posting? The only difference i see is that @alirezanet 's example contain 2 files, where yours only contains one.

I dont think it is a functional workaround to call dotnet format --options here for EACH changed file.. this would kill performance even more!

Curious to hear about your reply!

sander1095 avatar May 19 '22 19:05 sander1095

when I use --include with actual supported files in my solution, it's really slow.

@sander1095 Two things go into why Style and Analyzer operations are slower.

First, they require that we have correct semantic information about your code. To do this we rely on MSBuild performing a design-time build where we can know the exact set of code files as well as what references to include with each project. This is equivalent to what Visual Studio or OmniSharp does when opening a project/solution. It is less costly than a full compile, but it can take anywhere from seconds to minutes depending on the size of your solution.

Second, we have to actually run the analyzers and apply code fixes. Running analyzers requires additional in-memory compilations. The results of one analysis become invalid as we apply code fixes from earlier analysis.

You can run with verbosity set to diagnostic (-v diag) to see timings for workspace loading and analysis.

One workaround is simply using dotnet format whitespace which is super quick

Whitespace formatting only requires syntactic information. We provide the --folder option for the whitespace command as a way to avoid the cost of design-time builds. This allows us to simply search beneath the target folder for C# and VB.NET files to format. No effort is made to determine what other references your projects may actually contain because that is not needed when working with syntax trees.

How is that different than what @alirezanet is posting?

Specifically it is the = that is different. I updated my example to include the second filename.

JoeRobich avatar May 19 '22 20:05 JoeRobich

Hm alright, but do you know of any work being planned to fix the issue I/OP mention? Or the linked #1378 one :D?

Im not asking you to do it, I am just wondering about any planned perf improvements :)

sander1095 avatar May 19 '22 20:05 sander1095

I believe currently dotnet format missing a feature to control the formatting rules without MsBuild like the whitespace formatting.

Hm alright, but do you know of any work being planned to fix the issue I/OP mention? Or the linked #1378 one :D? Im not asking you to do it, I am just wondering about any planned perf improvements :)

Me too, I'm wondering is any plan for the future to support this kind of formatting scenario?

alirezanet avatar May 19 '22 20:05 alirezanet

To handle the options not being parsed correctly, we plan to remove the parsing from the CLI side and simply pass through to the dotnet-format tool. I have opened https://github.com/dotnet/sdk/pull/25541 to get this addressed.

As for performance, we have discussed alternate ways to speed up populating our workspace. Ultimately MSBuild will be involved at some point because .NET builds are so highly customizable the only way not to be wrong is to have MSBuild do the work.

  1. We could use a cache of MSBuild .binlog (or other format) containing the code files and dependency information we need. At which point this becomes a staleness problem. If dependencies aren't being added/removed, then this cache may remain a viable source of information for a long period.
  2. We could develop a Project Server of sorts. This service would have an initial slow startup that you experience today but, while it remains running, future invocations wouldn't have to pay the MSBuild cost.

We are not committing to either approach today, but know that this a problem that we are thinking on.

JoeRobich avatar May 19 '22 22:05 JoeRobich

This is a real nightmare when you try to set up dotnet format as a pre-commit hook. "Why is running this taking several minutes for like 40 files total? OOOOOOH."

tillig avatar Aug 23 '23 22:08 tillig

Hi! :) What's the status here? I'm runnning: dotnet format --verify-no-changes --no-restore --include Core/src/TestFile.cs And it takes like 30s. I've also tried: dotnet format --verify-no-changes --no-restore --include=Core/src/TestFile.cs

and

dotnet format --verify-no-changes --no-restore -- include=Core/src/TestFile.cs

I'm on net6.0 and net8.0. Also tried using the --verbosity d. I can clearly see that it doesn't give a s*** about my include filter :P

DennisDyallo avatar Mar 08 '24 16:03 DennisDyallo