Enable Analyzers

[workgroupengineering]

Add NetAnalyzers for monitor performance issue

• CS0162: Unreachable code detected feat: Enable Rule CS0162 #9717
• CS0649: Field 'field' is never assigned to, and will always have its default value 'value' feat: Enable Rule CS0649 #9991
• CA1018: Mark attributes with AttributeUsageAttribute Cleanup attributes #10169
• CA1304: Specify CultureInfo feat: Enable Rule CA1304 #9588
• CA1802: Use literals where appropriate fix: CA1802 #9162
• CA1805: Do not initialize unnecessarily see comment
• CA1806: Do not ignore method results
• CA1810: Initialize reference type static fields inline
• CA1812: Avoid uninstantiated internal classes
• CA1813: Avoid unsealed attributes Cleanup attributes #10169
• CA1815: Override equals and operator equals on value types feat: Enable Rule CA1815 #9628
• CA1819: Properties should not return arrays
• CA1820: Test for empty strings using string length feat: Enable rule CA1820 #9188
• CA1821: Remove empty Finalizers
• CA1822: Mark members as static feat: Enable Rule CA1822 #9189
• CA1823: Avoid unused private fields feat: Enable Rule CA1823 #9685
• CA1825: Avoid zero-length array allocations Features/net analyzers/ca1825 #9166
• CA1826: Do not use Enumerable methods on indexable collections feat: Enable Rules from 1826 to 1829 #9593
• CA1827: Do not use Count() or LongCount() when Any() can be used feat: Enable Rules from 1826 to 1829 #9593
• CA1828: Do not use CountAsync() or LongCountAsync() when AnyAsync() can be used feat: Enable Rules from 1826 to 1829 #9593
• CA1829: Use Length/Count property instead of Count() when available feat: Enable Rules from 1826 to 1829 #9593
• CA1835: Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'
• CA1838: Avoid 'StringBuilder' parameters for P/Invokes
• CA1844: Provide memory-based overrides of async methods when subclassing 'Stream'
• CA1846: Prefer 'AsSpan' over 'Substring'
• CA1847: Use char literal for a single character lookup feat: Enable Rule CA1847 #9190
• CA1848: Use the LoggerMessage delegates
• CA1849: Call async methods when in an async method
• CA1851: Possible multiple enumerations of IEnumerable collection feat: Enable Rule CA1851 #16779
• CA1852: Seal internal types
• CA1854: Prefer the IDictionary.TryGetValue(TKey, out TValue) method feat: Enable Rule CA1854 #11288
• CA2000: Dispose objects before losing scope
• CA2211: Non-constant fields should not be visible feat: Enable Rule CA2211 #9857

[workgroupengineering]

Can I work on this enhancement?

[maxkatz6]

Personally I believe we should have them enabled.
But most likely it should be enabled slowly project by project with limited number of warnings to avoid noise.
Less important warnings can be disabled or enabled later in the future.

[robloo]

I dont necessarily think all these analyzers have defined performance benefits. Changing this much code in so many areas when not the original author also risks breaking things. Changes are made very quickly without much thought refactoring like this. That said, analyzers are pretty good about finding changes without causing problems.

For some of them, as an example, I added an internal ArrayList class to colorpicker in a PR. That returns an array as a property. That is intentional and I don't want you changing blindly to follow CA1819. And BTW, there is no way that is a performance issue...

There are also cases where you want zero length arrays so in downstream code you can use iterators without null checks or to about uninitialized variable compile errors. I also wouldn't blindly change this across the code base.

I'm sure there are other areas where the original authors would have the similar concerns.

Fundamentall, I would approach this differently. If we know an analyzer has a clear performance benefit we can follow it. Otherwise, I wouldn't do all of these at once across the code base. In fact I wouldn't do any of then unless there is a good reason.

[maxkatz6]

That is intentional and I don't want you changing blindly to follow CA1819.

That's exactly why I want these rules to be fixed one by one or in small batches. So it can be reviewed and enabled. Otherwise we would stuck in refactorings, huge diffs or tons of warnings.

And BTW, there is no way that is a performance issue...

I think it's more an API issue, but still matters mostly in case of public API.

[maxkatz6]

There are also cases where you want zero length arrays so in downstream code you can use iterators without null checks or to about uninitialized variable compile errors.

This specific rule is not about replacing zero length array with a null object, but replacing it with cached Array.Empty<int>() instance. It will reuse same zero length array while new int[0] will allocate a new one each call.

Sometimes new array makes sense, if you want to have unique array references, but that's rare.

[robloo]

This specific rule is not about replacing zero length array with a null object, but replacing it with cached Array.Empty() instance

Yea, I misunderstood and saw that with the PR.

[hez2010]

Would also like to see Avalonia marking all libraries with <IsTrimmable>true</IsTrimmable> and enabling trimming Roslyn analyzer with <EnableTrimAnalyzer>true</EnableTrimAnalyzer>.