Do not run 'remove unnecessary imports' on generated code#74762
Do not run 'remove unnecessary imports' on generated code#74762CyrusNajmabadi merged 13 commits intodotnet:mainfrom
Conversation
ghost
added
Area-IDE
untriaged
| protected abstract bool IsRegularCommentOrDocComment(SyntaxTrivia trivia); | ||
| protected abstract IUnnecessaryImportsProvider<TSyntaxNode> UnnecessaryImportsProvider { get; } | ||
|
|
||
| protected override GeneratedCodeAnalysisFlags GeneratedCodeAnalysisFlags => GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics; |
There was a problem hiding this comment.
That way I think -generatedCodeClassificationIdDescriptor can be removed along with the check that selects the descriptor based on whether this is generated code or not
| @@ -186,22 +186,9 @@ public async Task TestGeneratedCode() | |||
| var source = """ | |||
There was a problem hiding this comment.
Maybe add a [WorkItem] to the test to clarify, why no diagnostics there are expected
Youssef1313
left a comment
Youssef1313
left a comment
There was a problem hiding this comment.
LGTM, but one thing to note. Do we know for sure that this is actually a performance issue? In some cases, the analyzer can be showing as expensive because it's calculating something that will happen anyways during the compilation (i.e, the cost is just moving around between the rest of the compilation process and the analyzer).
There was a problem hiding this comment.
This is a red herring. Remove unnecessary imports has no actual performance impact. It incorrectly captures unavoidable performance overhead from a different part of the system, suggesting it can be removed by disabling this analyzer when in fact it cannot.
|
@sharwell what value does this analyzer have though? The user cannot change the usings added. we don't run other style analyzers on generated code, why do we run this one.
Then we shouldn't report that it does. It also makes the other analyzers look good by comparison. I'd still like to take this though, if there's no use value provided. |
|
I have found value in this feature while writing source generators. |
|
The best code is no code. I don't see why we are special casing this style analyzer for generated code, and nothing else. |
akhera99
left a comment
akhera99
left a comment
There was a problem hiding this comment.
looks good, I agree that the test should have some indication that the behavior has changed
6 participants
Fixes #74761.