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

SA1518 - The line endings at the end of a file do not match the setti… #7571

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

elachlan
Copy link
Contributor

@elachlan elachlan commented Apr 25, 2022

Relates to #7174
https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1518.md

The two remaining warnings/suggestions are from RoslynImmutableInterlocked pulled in from Microsoft.CodeAnalysis.Collections in Microsoft.Build.Framework.

Solution for the warnings from the nuget package is outlined in dotnet/roslyn#55992

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SA 1518 even applies to newlines at the end of files? I'm not (personally) opposed to taking those out, but I imagine they'd come back in with various editors so probably not worth worrying about. Removing multiple blank lines at the end of a file seems more clear-cut to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The analyser uses the editor config settings for new lines. So I don't think they will get overridden by the editor later unless the settings change.

@Nirmal4G
Copy link
Contributor

This only affects the cs souces, right? Lot of other files have this issue too.

@elachlan
Copy link
Contributor Author

This only affects the cs souces, right? Lot of other files have this issue too.

The analyser has been applied to the whole solution.

@Nirmal4G
Copy link
Contributor

No, only C# files have been touched.

@elachlan
Copy link
Contributor Author

No, only C# files have been touched.

This issue is specific to the stylecop analyser. I ran the specific analyser fix all for the solution. It only works against .net code and not anything else. Its not a solution wide analyser for all files.

@Nirmal4G
Copy link
Contributor

That clears it up. I have a PR that touches the XAML, Props and Targets files. I was looking to see if I should rebase on top of yours but yours doesn't touch those files. So, no need.

@elachlan
Copy link
Contributor Author

can you link it here? Are you using an Analyser so they don't sneak back in?

@Nirmal4G
Copy link
Contributor

#7168 and no, I don't think there's an analyzer for these files.

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Is the reason we can't enable the rule the Roslyn source package again?

@sharwell
Copy link
Member

It is not desirable for Microsoft.CodeAnalysis.Collections to omit the trailing newline, so feel free to send a Roslyn PR to just fix those two cases.

@Forgind Forgind merged commit 7c4597e into dotnet:main Apr 29, 2022
@Forgind
Copy link
Member

Forgind commented Apr 29, 2022

Thanks everyone!

@elachlan
Copy link
Contributor Author

elachlan commented May 6, 2022

Rolsyn files have been fixed. Awaiting a release and msbuild to update before we enable the rule.

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

Successfully merging this pull request may close these issues.

5 participants