Update SA1121 and SA1404 to detect global using aliases [on hold] #3671
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3671 +/- ##
==========================================
- Coverage 93.26% 93.26% -0.01%
==========================================
Files 1080 1082 +2
Lines 113982 114065 +83
Branches 4045 4047 +2
==========================================
+ Hits 106303 106380 +77
- Misses 6641 6646 +5
- Partials 1038 1039 +1 |
…ntax tree, by filling the dictionary directly when it is created. In the previous commit, it was checked for global using aliases once and then again for normal using aliases. DotNetAnalyzers#3594
bjornhellander
force-pushed
the
feature/global-using-alias
branch
from
cc8ce74 to
f2a69ed
Compare
|
|
||
| return nodes.OfType<UsingDirectiveSyntax>().Any(x => x.Alias != null); | ||
| foreach (var tree in compilation.SyntaxTrees) |
There was a problem hiding this comment.
❗ I have concerns about the performance impact of this change. We've seen many cases where analyzer initialization overhead is the dominant contributor to a negative performance trace. I believe it would be better to use SemanticModel.GetImportScopes for this purpose.
There was a problem hiding this comment.
Alright. GetImportScopes seems to be supported from Roslyn 4.3 and global usings from 4.0, so I guess that there will be some versions where the bug will still exist if that approach is taken, but some false negatives might be ok since it will only happen for a few versions?
Would this method be supported automatically with newer (than c# 8) light-up support or would it require manually implementing that support?
Is this method quick? I have gotten the impression that the semantic model should be used with care, but maybe this is not a problematic method.
bjornhellander
changed the title
Fixes #3594. SA1404 also suffered from the same problem.