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

How should source generators emit newlines? #51437

Open
jnm2 opened this issue Feb 24, 2021 · 13 comments
Open

How should source generators emit newlines? #51437

jnm2 opened this issue Feb 24, 2021 · 13 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Feb 24, 2021

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.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 24, 2021
@333fred
Copy link
Member

333fred commented Feb 24, 2021

For reference, see dotnet/roslyn-analyzers#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
Copy link
Member

333fred commented Feb 24, 2021

/cc @chsienki

@ufcpp
Copy link
Contributor

ufcpp commented Feb 24, 2021

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

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

jnm2 commented Feb 24, 2021

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

jasonmalinowski commented Feb 24, 2021

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

333fred commented Feb 24, 2021

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

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

jaredpar commented Mar 3, 2021

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

@jaredpar jaredpar added Feature Request Feature - Source Generators Source Generators and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 3, 2021
@jaredpar jaredpar added this to the C# 10 milestone Mar 3, 2021
@333fred
Copy link
Member

333fred commented Mar 3, 2021

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

jaredpar commented Mar 3, 2021

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

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

@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

@jaredpar jaredpar modified the milestones: C# 10, 17.0 Jun 18, 2021
@jaredpar jaredpar modified the milestones: 17.0, 17.1 Sep 2, 2021
@jinujoseph jinujoseph modified the milestones: 17.1, 17.3 Apr 27, 2022
@jaredpar jaredpar modified the milestones: 17.3, Backlog Jul 1, 2022
slashP pushed a commit to ClaveConsulting/Expressionify that referenced this issue Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants