Skip running StyleCop rules on generated files and elements #670
Conversation
|
Obviously this needs to be applied to all rules and not just SA1600. I didn't want to go too far before getting feedback as this is my first contribution. Please let me know what you think, including:
|
|
I'm primarily concerned about the performance implications of this. In the proposed implementation, the check would be run once for each item analyzed, as opposed to once per file. When you consider all the other analyzers we have here, we could be talking about multiple calls to check for exclusions for every token in the input, including trivia tokens. |
|
Good point. But how else can we skip an entire file? At the time the analyzer registers itself, we don't have a file context to check. |
|
No idea. Best guess right now is it might be possible to use If it worked, this would allow the result to be shared across all analyzers in the solution. If it worked. |
|
Oh hey, it would certainly work if we treat the |
|
Cool. I'll try that out and see if I can test its effectiveness as well. |
|
What does the 'do not merge' label mean? Is that a bad thing? |
|
No, it just means issues were identified during the last code review (it saves me time when looking through the list of outstanding PRs). |
| @@ -60,7 +60,7 @@ public override void Initialize(AnalysisContext context) | |||
| context.RegisterSyntaxTreeAction(this.HandleSyntaxTree); | |||
|
|
|||
| // handle nameof (which appears as an invocation expression??) | |||
| context.RegisterSyntaxNodeAction(this.HandleInvocationExpressionSyntax, SyntaxKind.InvocationExpression); | |||
| context.RegisterSyntaxNodeActionHonorExclusions(this.HandleInvocationExpressionSyntax, SyntaxKind.InvocationExpression); | |||
There was a problem hiding this comment.
❗ We're also going to need to handle RegisterSyntaxTreeAction above.
There was a problem hiding this comment.
Great. I had used a search and replace for the first method, so I missed this other one.
There were several "return false" statements that would skip storing the negative result in the cache, resulting in repeated checks. Moving the entire check logic into another method allowed me to keep the logic simple but guarantee storage of the result.
|
|
||
| private static bool IsOnGeneratedFile(this string filePath) => | ||
| Regex.IsMatch(filePath, @"(^service|^TemporaryGeneratedFile_.*|^assemblyinfo|^assemblyattributes|\.(g\.i|g|designer|generated|assemblyattributes))\.(cs|vb)$", | ||
| RegexOptions.IgnoreCase); |
There was a problem hiding this comment.
💡 Add RegexOptions.ExplicitCapture.
|
@AArnott pro-tip: GitHub does not send me a notification if you push more commits. You have to actually comment afterwards for it to notify me. 👍 (or break the build and have AppVeyor email me 😉) |
…covers the file name as well as the content
|
@sharwell thanks for that. I wasn't aware. :) |
Auto generated code updates
Skip running StyleCop rules on generated files and elements
|
I love this! I think it would be nice if the changes made to GeneratedCodeAnalysisExtensions would be contributed back to code cracker as well. |
Fixes #555