Uh oh!
There was an error while loading. Please reload this page.
Fix most violations of new CA1859 - #80335
Conversation
ghost
commented
Jan 8, 2023
Tagging subscribers to this area: @dotnet/area-meta Issue DetailsThe analyzer recommends replacing uses of types with other types that could reduce overheads (e.g.
|
Wow, that's a lot of stuff. CA1859 currently only recommends changing the signature of private methods and fields, the idea being that the changes are completely local to a given type. Do you think it would be useful to extend the analysis to internal symbols too, at least optionally? I bet that would raise another crop of these recommendations. Another thing I'd like to add in the analyzer is a basic understanding of collections. So if you have an |
The analyzer recommends replacing uses of types with other types that could reduce overheads (e.g. `List<T>` instead of `IList<T>`). This fixes most of the violations we currently have of the rule, but it doesn't enable it yet because there are too many false positives; fixes for those are in-flight. Some of these replacements won't help with perf (e.g. using `ArgumentException` instead of `Exception` as the return type of a throw helper create method), but there's little harm to them, and some of them do avoid overheads like allocation (e.g. foreach'ing something typed as `Dictionary<>` instead of `IDictionary<>` will avoid an enumerator allocation, accessing `Count` on a `List<T>` instead of `IList<T>` will avoid an interface dispatch, etc.)
stephentoub
commented
Jan 9, 2023
Can you share an example?
Yeah, I think that would be useful, at least for some of the most common, like IEnumerable and List. |
buyaa-n
left a comment
There was a problem hiding this comment.
Left a few NITs, overall LGTM, thank you!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
geeknoid
commented
Jan 12, 2023
Be gone Peanut Butter! |
The analyzer recommends replacing uses of types with other types that could reduce overheads (e.g.
List<T>instead ofIList<T>). This fixes most of the violations we currently have of the rule, but it doesn't enable it yet because there are too many false positives; fixes for those are in-flight. Some of these replacements won't help with perf (e.g. usingArgumentExceptioninstead ofExceptionas the return type of a throw helper create method), but there's little harm to them, and some of them do avoid overheads like allocation (e.g. foreach'ing something typed asDictionary<>instead ofIDictionary<>will avoid an enumerator allocation, accessingCounton aList<T>instead ofIList<T>will avoid an interface dispatch, etc.)cc: @buyaa-n, @geeknoid