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

Review and update code fixers to use correct new line character sequence #3656

Open
bjornhellander opened this issue May 14, 2023 · 8 comments

Comments

@bjornhellander
Copy link
Contributor

bjornhellander commented May 14, 2023

There is a bunch of code fixers which either always use CRLF when adding line breaks, or get the character sequence from document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp) and thereby fail to honor a end_of_line setting in .editorconfig.

I see these:

  • SA1102: Currently CRLF
  • SA1103: Currently CRLF
  • SA1104: Currently CRLF
  • SA1107: Currently CRLF
  • SA1116: Currently CRLF
  • SA1127: Currently CRLF
  • SA1132: Currently CRLF
  • SA1133: Currently CRLF
  • SA1134: Currently CRLF
  • SA1136: Currently CRLF
  • SA1200, SA1208-SA1211, SA1216, SA1217 (UsingCodeFixProvider): Currently CRLF
  • SA1201-SA1204, SA1214 (ElementOrderCodeFixProvider): Currently CRLF
  • SA1212: Currently CRLF
  • SA1213: Currently CRLF
  • SA1500: Currently CRLF
  • SA1501: Currently CRLF
  • SA1502: Currently CRLF
  • SA1504: Currently CRLF
  • SA1507: Currently Workspace.Options
  • SA1513: Currently CRLF (See Applying autofixes uses wrong line endings / should honour .editorconfig #3360)
  • SA1514: Currently CRLF
  • SA1515: Currently CRLF
  • SA1516: Currently CRLF
  • SA1600: Currently Workspace.Options
  • SA1609 Currently Workspace.Options
  • SA1615: Currently Workspace.Options
  • SA1642: Currently Workspace.Options
  • SA1633-SA1638, SA1640, SA1641 (FileHeaderCodeFixProvider): Currently Workspace.Options (See File header code fix causes mixed line endings #3247)

I assume the primary updates should be to either look at surrounding text and use the existing new line sequence or read from an .editorconfig file. What is the wanted behaviour for the ones listed above?

@bjornhellander
Copy link
Contributor Author

bjornhellander commented May 14, 2023

I would vote for ignoring any config files and auto-detecting the wanted new line sequence for all analyzers, like the proposed solution for #3360.

@manfred-brands
Copy link

According to a comment Roslyn formats codefixes.
This came up in the nunit.analyzers and as far as I tested it that seems to be the case.

@MartyIX
Copy link
Contributor

MartyIX commented May 16, 2023

I would vote for ignoring any config files and auto-detecting the wanted new line sequence for all analyzers, like the proposed solution for #3360.

What is the advantage of that approach? I mean if a user specifies in .editorconfig a newline setting, it seems like that's what he/she really wants to achieve.

@bjornhellander
Copy link
Contributor Author

@MartyIX It could be specified there, or in a .globalconfig, or as an IDE setting. Simply adapting to what is already used would make it work without having to worry about different ways of configuring newlines. As long as the file is consistent, that is. But if it's not, then would this shortcut really make it worse? :-) Just an idea.

@bjornhellander
Copy link
Contributor Author

Very interesting @manfred-brands. I have seen "elastic" being mentioned, but I have apparently not understood. Will check it out!

@bjornhellander
Copy link
Contributor Author

Also see this comment regarding the probably incomplete method for detecting the actually used newline sequence, if more code fixers are updated to use it: https://github.com/DotNetAnalyzers/StyleCopAnalyzers/pull/3607/files#r1237263110

@sharwell
Copy link
Member

sharwell commented Jun 23, 2023

I would vote for ignoring any config files and auto-detecting the wanted new line sequence for all analyzers, like the proposed solution for #3360.

What is the advantage of that approach? I mean if a user specifies in .editorconfig a newline setting, it seems like that's what he/she really wants to achieve.

It's not the responsibility of every analyzer to enforce line endings for the full file. Each code fix should only fix the issue at hand; even if a file deviates from the end_of_line setting in .editorconfig, the code fix should avoid causing a file to go from consistent line endings to mixed line endings.

@sharwell
Copy link
Member

Very interesting @manfred-brands. I have seen "elastic" being mentioned, but I have apparently not understood. Will check it out!

Elastic trivia is formatted by the IDE and the end of code fix applications. StyleCop Analyzers sometimes avoids using elastic trivia (e.g. via the WithoutFormatting extension) because its fixes need absolute control over the whitespace in the result.

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