Write DiagnosticSuppressor for CS8618 #21618
Conversation
|
|
||
| namespace Microsoft.EntityFrameworkCore | ||
| { | ||
| public class NullabilityDiagnosticSuppressorTest |
There was a problem hiding this comment.
I recommend you take a look at InternalUsageDiagnosticAnalyzerTest and try to do something similar.
There was a problem hiding this comment.
@roji I tried to do that. But seems like the current BaseTest won't fit well with testing suppressor.
There was a problem hiding this comment.
In that case I'd look at some other well-known suppressor and try to see what it does..
There was a problem hiding this comment.
We don't have a good way to test suppressors currently. We're still looking into options.
| continue; | ||
| } | ||
|
|
||
| // TODO: Check if the class derives from DbContext. (Not sure how to do it yet) |
There was a problem hiding this comment.
The code as is only looking at syntax nodes, which isn't sufficient for identifying inheritance AFAIK - you need to look at the semantic model (GetSymbolInfo). However, GetSymbolInfo apparently has some perf issues, so #18637 is about modifying our current analyzer to use IOperation instead; am not sure exactly how that looks like for a suppressor but it should be possible.
At least for now, our analyzers package does cannot reference EF Core types directly (I'll try to see about improving this in the future), so you can only identify/recognize EF types by name.
There was a problem hiding this comment.
@roji, I'll try to play around with SemanticModel.
There was a problem hiding this comment.
@Youssef1313 please hold off on this, @davidroth's suggestion in #21608 will likely obviate this.
|
@Youssef1313 am closing this as @davidroth's suggestion to simply initialize the DbSet properties is cleaner and obviates this - but thanks for your work on this. There's still #21663 which could definitely benefit from a suppressor, but please note it's unlikely we'll merge this for 5.0 this late. |
Fixes #21608
This is not ready yet. I need some guidance on writing the unit test, as well as reviewing the suppressor and guidance on the TODO comment.
cc: @ajcvickers @roji