-
Notifications
You must be signed in to change notification settings - Fork 457
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
False positive RS1035 error for Environment.NewLine #6467
Comments
Speaking of |
Thanks for pinging on this issue. I think the expectation is that we get the preferred newline style from the project we are generating for instead of picking the OS-level default for the source generator's own build environment. Do I have that right @jaredpar? FWIW, it seems extremely easy to include newlines in the generated code which don't match the user project's expectations, by using a Assuming this error is expected, the error message could be improved here, since it mainly had GetEnvironmentVariable, etc. in mind when it was written. |
@RikkiGibson How would a generator do that? It is extremely frustrating that |
In an analyzer/source generator I would expect people to do: var syntaxTree = ... // syntaxTree you want to know the newline settings for
var options = context.AnalyzerConfigOptions.GetOptions(syntaxTree);
if (options.TryGetValue("end_of_line", out var newline))
{
// do something with newline
} If you are in a workspace context I would expect: var workspace = ... // get a reference to a workspace
var newline = workspace.GetOption(new OptionKey(FormattingOptions.NewLine, LanguageNames.CSharp)); But that is if you would actually do anything different based on the value of this setting. If you are generating code and want it to just be "whatever the users settings say" then use |
For a source generator, it would also be acceptable to just use |
The core problem in a source generator is that if you use |
Where should we record the above advice to help people who encounter this warning? |
@jmarolf The recommendation for new line handling has always been to not specify "end_of_line" in editorconfig, and let git normalize whitespaces, which means that if a codebase contains multiline raw/verbatim string literals, they will end up being either Why/how is the case for source generators any different? |
I'm wondering if we should unban NewLine here. Separately, I'm also wondering if we should unban CurrentCulture, at least until we have a clear pattern we want people to use instead. |
This rule is super problematic for source generators that use T4 for generation because the runtime code generated by the T4 system includes references to both |
I wanted to include a newline in a diagnostic descriptor message, but apparently this is also forbidden. |
So, we also ran into this when we updated the |
Why would you get non-deterministic build? Enironment.NewLine is a function call into System.Runtime, it should be a function call into System.Runtime on all frameworks and for any compiler. The Jitter might optimize it, but I wouldn't expect the compiler to do so. EDIT: Ah, never mind, I believe you are saying that the TARGET of the source generator would result in a non-deterministic build. |
Analyzer
Diagnostic ID: RS1035
The symbol '{0}' is banned for use by analyzers: Analyzers should not read their settings directly from environment variables
Analyzer source
Unknown, I am using SDK 6.0.405 and the following Nuget packages
Microsoft.CodeAnalysis.CSharp 4.3.0
Microsoft.CodeAnalysis.Analyzers 3.3.4
Microsoft.CodeAnalysis.Workspaces.Common 4.3.0
Describe the bug
When I use
Environment.NewLine
inside a source generator, I get the errorI understand that it maybe be dangerous to read settings from environment variables but the class
Environment
has members that AFAIK have nothing to do with environment variables and therefore should be save to use.In the case of
Environment.NewLine
, what is the right way add a newline dynamically in generated code?Steps To Reproduce
Environment.NewLine
anywhere in the generator,Expected behavior
No error occurs when I use
Environment.NewLine
Actual behavior
The error
occurs.
The text was updated successfully, but these errors were encountered: