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

Formatting analyzer that checks existence/format of a file/license header #1516

Open
siegfriedpammer opened this issue Sep 14, 2024 · 3 comments

Comments

@siegfriedpammer
Copy link

I could implement this in my own package, but it might be useful for other people, so I am creating an issue here.

While dotnet format has a rule for file headers, it is not really configurable, it only can check for static text. Things like copyright year numbers or different author names, which could easily be covered by a simple regex, cannot be configured.

Options:

  • list of allowed file header regexes
  • ignore whitespace before file header: boolean
  • ignore preprocessor directives: boolean
  • do we potentially want to ignore other SyntaxTrivia?

I would be willing to implement this.

@josefpihrt
Copy link
Collaborator

Hi @siegfriedpammer,

I think this would be very useful analyzer. Thank you for the initiative.

Regarding the idea of regex, Do we really need it when defining a file header? If you have multiple headers in your solution you can configure it with multiple editorconfig files.

What would be really useful in my opinion would be to provide functionality to replace old header with a new one. That would require to define option with "old" file header.

I see that you provided a list of options which is great. Could you provide a list of real editorconfig options with keys and values. It would be better to agree on the options before the implementation starts. Thanks.

References:

@siegfriedpammer
Copy link
Author

Thank you for the quick response!

Do we really need it when defining a file header? If you have multiple headers in your solution you can configure it with multiple editorconfig files.

For me the primary use case for a regex is not defining multiple file headers, but rather to allow file headers to have some dynamic elements such as copyright year or different author names for example (taken from the ILSpy project):

// Copyright (c) 2022 Siegfried Pammer
--or--
// Copyright (c) 2011 AlphaSierraPapa for the SharpDevelop Team
--or others, followed by--
// 
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
// 
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
// 
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.

which would be generalized as:

// Copyright (c) \d{4} \w[\w ]*
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
-- omitted for brevity --

This is the flexibility that is missing from https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0073 and the reason for this proposal.

What would be really useful in my opinion would be to provide functionality to replace old header with a new one. That would require to define option with "old" file header.

I agree...

I see that you provided a list of options which is great. Could you provide a list of real editorconfig options with keys and values. It would be better to agree on the options before the implementation starts.

Will do so, but it might take some time since I am not very familiar with the editorconfig conventions yet. Thank you very much for providing some references!

@siegfriedpammer
Copy link
Author

Finally, I managed to find the time to dive into the Roslynator "infrastructure" to understand how the CodeGenerator works and which XML files I need to edit to get the scaffolding for a new Analyzer/CodeFix and tests.

So here is my first attempt at putting together a list of options in .editorconfig style (omitting the roslynator_ prefix):

file_header_pattern = text|regex:<regex>|file:<path to file>|regex-file:<path to file>
file_header_comment_style = single_line|multi_line|multi_line_block
file_header_allowed_trivia = whitespace|whitespace_and_preprocessor_directives

I have omitted the file_header_old_pattern option for now, because I am not yet sure about the correct behavior for the codefix, and why such a setting might be necessary at all.

detailed descriptoin

file_header_pattern

This option features four modes:

  • plaintext (no prefix)
  • regex (indicated by the regex: prefix
  • file (indicated by the file: prefix)
  • a combination of (file and regex)

file_header_comment_style

I think the options are mostly self-explanatory. The multi_line_block differs from multi_line in that it expects a * on every line, e.g.

/* Copyright (c) blabla
 *
 * More license text.
 **/

vs.

/*
Copyright (c) blabla
   
More license text.
*/

file_header_allowed_trivia

Ideally, this should be a flags enum option, but I don't know if that's supported by editorconfig.

I have come up with the following combinations, not sure, if there are other useful ones:

  • whitespace
  • whitespace_and_preprocessor_directives

One edge case I found: it's impossible to disable the analyzer via #pragma warning disable, at least I was unable to write unit test for that case. Am I missing something?

Looking forward to your feedback and comments!

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

2 participants