dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.74k stars 3.99k forks source link

How should source generators emit newlines? #51437

Open jnm2 opened 3 years ago

jnm2 commented 3 years ago

There's a source generator that always emits CRLF, whether running on Windows or not. Is there a guideline for what source generators should do?

@333fred says there should likely be a helper to prevent mistakes such as source generators writing to disk and changing line endings in source-controlled files.

333fred commented 3 years ago

For reference, see https://github.com/dotnet/roslyn-analyzers/issues/4749.

The main issue is that Environment.NewLine is not sufficient. If the generator output is written to disk, and this is a git repo on windows with autocrlf turned off or if the partition is shared with linux, then the line endings of the generator output will differ from those on disk and git will either warn when staging the file or error (if safecrlf is on). Further, if it's a new file, it could potentially not match the line endings of the rest of the files in the project.

333fred commented 3 years ago

/cc @chsienki

ufcpp commented 3 years ago

My source generator uses @"" to append new lines:

buffer.Append(@"
");

With autocrlf, this generator emit CRLF if built on Windows and LF on Linux. The nupkg is built with GitHub action runs-on ubuntu, thus always emits LF. However, it emits CRLF if referencing locally on Windows with autocrlf, or built on Windows and manually uploaded to nuget.org.

P.S. Appending new lines with @"" reports CA 1834 only if eol=lf.

image

CyrusNajmabadi commented 3 years ago

Tagging @jasonmalinowski. This is solved in the IDE and we woudl want the same solution to work for the compiler layer. Note that there are potentially many options in play here, including (but not limited to):

  1. What editorconfig says the newline character is.
  2. What a host may say the newline character is.
  3. What the platform/OS thinks the newline character is.
jnm2 commented 3 years ago

Does this also matter for deterministic builds, so that embedded source in binaries doesn't fluctuate between line ending types depending on the OS of the CI machine that compiled it? Or are source generator outputs already considered out of scope for the purposes of determinism?

jasonmalinowski commented 3 years ago

So yeah, this might just be a prioritization thing:

  1. .editorconfig has an end_of_line property, so if you're doing the case with checking in files with autocrlf off or sharing partitions, then I'm guessing your workflow has that set. (If it doesn't, even something like "create new file" in Visual Studio for Windows will do the wrong thing.)
  2. If that doesn't exist, use platform's choice.

Or are source generator outputs already considered out of scope for the purposes of determinism?

We obviously hope that generators are well-behaved, but we can't guarantee it depending on how it's written. This in some sense is no different than analyzers, in that an analyzer may be non-deterministic too. We obviously hope that's not the case (the user experience is then crap) but there's nothing we can do to specifically prevent it.

333fred commented 3 years ago

If that doesn't exist, use platform's choice.

That probably isn't a good solution, for the reasons I mentioned above. If there is an existing file on disk, it should match to that first.

CyrusNajmabadi commented 3 years ago

In the IDE we have a service (provided by the platform) to perform those heuristics. Sounds like you may need to port this to the compiler as well and expose through a well-known api for generators to consume.

jaredpar commented 3 years ago

For reference, see dotnet/roslyn-analyzers#4749.

Don't think that dotnet/roslyn-analyzers#4749 is a good reference point here. That is a discussion about how new lines should be inserted into an existing file. Essentially it's a discussion about new line consistency. There is no issue with consistency in a source generated file because it's a blank state, it's all about what should the default be.

Further, if it's a new file, it could potentially not match the line endings of the rest of the files in the project.

Generators always emit new files.

Does this also matter for deterministic builds, so that embedded source in binaries doesn't fluctuate between line ending types depending on the OS of the CI machine that compiled it?

This does not impact deterministic builds because determinism does not exist across OS. By that I mean if you compile a fully deterministic build on Windows and Linux you will get different binaries. The OS contributes to the signature of the binaries in a number of unintuitive and essentially unavoidable ways

333fred commented 3 years ago

Generators always emit new files.

They do not. If the write to disk flag is set, then there exists a copy of the file on disk, and if a generator uses different lines endings, then it will look to git like a problem. Yes, they don't read from the existing file today, but the end effect is not a new file, the end effect is an edited file.

jaredpar commented 3 years ago

That is still a new file, not an edit from the perspective of the generator. It is explicitly not an input to the process.

joshlang commented 3 years ago

I'll just throw 2-cents in here.

A silly request that .Trim() just magically gets called I'd like to see generated files in VS

Selfishly, I want my code generated files to "look nice". Whether they're on disk or not, I also want to see them in visual studio, even if it was just some virtual magic that makes it work and they're never emitted to disk.

So, if those requests are ever a thing, then the CRLF debate may have an impact on it.

dferretti commented 3 years ago

@joshlang I know you mentioned this a little while ago, but in case you are still looking for it: you can view the generated files in VS. Whether or not you actually persist them to disk the IDE will show them to you here. See this link and CTRL-F for "view the generated code" https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#NET-Productivity