Enable NetAnalyzers

[workgroupengineering]

Add NetAnalyzers for monitor performance issue

• [x] CA1802: Use literals where appropriate #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
• [ ] CA1815: Override equals and operator equals on value types
• [ ] CA1819: Properties should not return arrays
• [x] CA1820: Test for empty strings using string length #9188
• [ ] CA1821: Remove empty Finalizers
• [ ] CA1822: Mark members as static
• [ ] CA1823: Avoid unused private fields
• [x] CA1825: Avoid zero-length array allocations #9166
• [ ] CA1826: Do not use Enumerable methods on indexable collections
• [ ] CA1827: Do not use Count() or LongCount() when Any() can be used
• [ ] CA1828: Do not use CountAsync() or LongCountAsync() when AnyAsync() can be used
• [ ] CA1829: Use Length/Count property instead of Count() when available
• [ ] 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'
• [x] CA1847: Use char literal for a single character lookup #9190
• [ ] CA1848: Use the LoggerMessage delegates
• [ ] CA1849: Call async methods when in an async method

[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>.