CA1002: Do not expose generic lists #3387
Conversation
| { | ||
| public static class Helper | ||
| { | ||
| public static bool Ext1(this List<int> list) => true; |
There was a problem hiding this comment.
This exclusion is not part of the original implementation but that seems weird to get a penalty to extend List<T> so I added an exclusion. Please feel free to let me know if you think this shouldn't be done.
Codecov Report
@@ Coverage Diff @@
## master #3387 +/- ##
========================================
Coverage 95.30% 95.31%
========================================
Files 1029 1031 +2
Lines 233279 233669 +390
Branches 15092 15107 +15
========================================
+ Hits 222338 222728 +390
Misses 9278 9278
Partials 1663 1663 |
|
FYI: You will need to fix a new RS diagnostic once #3444 goes in. |
| context.RegisterSymbolAction(context => | ||
| { | ||
| var symbol = context.Symbol; | ||
| var methodSymbol = symbol as IMethodSymbol; |
There was a problem hiding this comment.
Did you mean to register separate actions for each symbol kind? It seems better to move the shared code in a separate helper and register separate actions for each symbol kind that needs different checks.
There was a problem hiding this comment.
Do you mean replacing
context.RegisterSymbolAction(ctx => {}, SymbolKind.Field, SymbolKind.Property, SymbolKind.Method);with
context.RegisterSymbolAction(Analyze, SymbolKind.Field);
context.RegisterSymbolAction(Analyze, SymbolKind.Property);
context.RegisterSymbolAction(Analyze, SymbolKind.Method);
private static Analyze() {}There was a problem hiding this comment.
I meant having separate AnalyzeMethod, AnalyzeField, etc. for each symbol kind, so you can directly cast to IMethodSymbol in AnalyzeMethod and have method specific logic only in AnalyzeMethod.
There was a problem hiding this comment.
Ah ok, yeah makes sense I have overused the helpers :)
|
|
||
| analysisContext.RegisterCompilationStartAction(context => | ||
| { | ||
| var genericListType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericList1); |
There was a problem hiding this comment.
| var genericListType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericList1); | |
| if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericList1, out var genericListType)) | |
| { | |
| return; | |
| } |
| var symbol = context.Symbol; | ||
| var methodSymbol = symbol as IMethodSymbol; | ||
|
|
||
| // FxCop compat: only analyze externally visible symbols by default. |
There was a problem hiding this comment.
Please update https://github.com/dotnet/roslyn-analyzers/blob/master/docs/Analyzer%20Configuration.md#analyzed-api-surface and file a doc issue to mention api surface configurability for this rule. Same for other rules that you are supporting this option.
Fix #369