Additional possible diagnostics analyzers/suppressors

[roji]

• N+1 detection (apparently Jetbrains are working on this).
• Internal usage analyzer: generic type argument for generic types (e.g. in instantiation)
• Add an analyzer for conventions #25022
• FindAsync + CancellationToken issue #12012
• Analyzer: warn (and code fix) for use of interpolation in SQL methods accepting raw strings #30965
• Analyzer to warn against accidental usage of AddEntityFrameworkProvider #25455
• Add an analyzer for new objects in OnConfiguring that are used as part of the ServiceProvider key #27856
• Analyzer suggestion: mark DbSet's nullability suppression property initializer as unnecessary #29988
• Analyzer: Unnecessary Configuration #11217
• Warn for use of nameof within raw SQL interpolated strings. This typically happens when people mistakenly want to interpolate column names, but they get parameterized and the query fails.

Scenarios requiring identifying EF Core LINQ queries (could use common infrastructure):

• Support ContainsKey (as an analyzer is telling us to use it) #26856
• Create DiagnosticSuppressor for CS8602 in Linq expressions and Includes #21663
• Translate int.Parse and similar #28287 (comment)
• Diagnostics analyzer for unnecessary Include #28449
• Analyzer warning and quick fix for ToDictionary over GroupBy #16838

One idea here would be to have users tell us in .editorconfig that all queryable LINQ queries are e.g. SQL Server; this would be an opt-in that could unlock provider-specific warnings/code fixes.

See #20206 for previous unanalyzed scenarios.

[vslee]

composite-issue label? Or is that already implied by User Story?

[bricelam]

One area I've been thinking we could improve using analyzers is cleaning up people's OnModelCreating code. We've had a few workarounds that later get a better API. I suspect the old workarounds are just sitting around providing bad examples to people.

For example, the old way to configure precision and scale was using the column type:

// EF Core 1-4
modelBuilder.Entity<Entity>()
    .Property(e => e.DecimalProperty)
        .HasColumnType("decimal(18, 2)");

But in EF Core 5, we added a better way to do it:

// EF Core 5
modelBuilder.Entity<Entity>()
    .Property(e => e.DecimalProperty)
        .HasPrecision(18, 2);

In addition, I think there's an opportunity to discourage people from dropping down to the core metadata APIs. For example, we could also provide a fix for code like this:

var decimalProperty = modelBuilder.Entity<Entity>().Property(e => e.DecimalProperty).Metadata;
decimalProperty.SetPrecision(18);
decimalProperty.SetScale(2);

[ranma42]

IIUC now suppressions work as intended even when TreatWarningsAsErrors is set #29582 and efcore contains some infrastructure to identify EF Core LINQ queries (maybe not in all cases, but at least in many relevant cases that are being used to pre-compile queries).
Is this a good time to try and tackle some of these issues? (given that now there seems to be the infrastructure already in place, would some of these be good first issue candidates?)

[roji]

@ranma42 the infrastructure introduced for identifying EF LINQ queries (for NativeAOT/precompiled queries) is very new, and is currently not being used in a Roslyn analyzer/source generator at all (but rather in a custom step, meant to be executed outside Roslyn at publish-time). So in theory you're absolutely right, and this does open up a future where we can start emitting/suppressing diagnostics inside LINQ queries; but in practice, it will take a while before this is something that we're able to tackle.

In any case, I definitely wouldn't consider any analyzer feature as a good first issue; writing analyzers/suppressors correctly (and efficiently!) can be quite tricky/advanced.