Count > 0 normalized to Any should also work on ICollection

[ChrisJollyAU]

When normalizing .Count > 0 or .Count != 0 to .Any it wasn't picking up the ICollection interface if the current declaring type was already an ICollection

Fixes #35365

[roji]

Looks great, thanks @ChrisJollyAU. See suggestion below, once that's fixed and the CI works again I'll merge.

[ChrisJollyAU]

@Roj Just thinking on this, I might have a look around at some stage to see if there is any other GetInterfaces that might encounter a similar style problem

[roji]

@ChrisJollyAU good idea, I wouldn't be surprised if we made this mistake in other places...

[ChrisJollyAU]

Just seen some others do it like this memberExpression.Expression.Type.GetInterfaces().Append(memberExpression.Expression.Type)

Shall I fix this one to be like that?

[roji]

@ChrisJollyAU I've disabled auto-merge here, so we can maybe push some more related fixes to this PR.

memberExpression.Expression.Type.GetInterfaces().Append(memberExpression.Expression.Type)

I guess it depends on the specific case - not every usage of GetInterfaces() is necessarily wrong...

BTW if this is a common thing, we may want to simply extract out a GetInterfacesInclusive() extension method which does this. We already have GetBaseTypesAndInterfacesInclusive() on SharedTypeExtensions, we could add GetInterfacesInclusive() there...

[ChrisJollyAU]

Just quick search for GetInterfaces only produced 24 results. Not that many
15 under src folder
9 under test folder

Of those under src, can eliminate 9 (4 from ProxyFactory and 5 from the Shared extensions (doesnt look to be needed and 1 already like the previous example I mentioned))

In the end it looks like theres only the ones from RelationalSqlTranslatingExpressionVisitor and its Cosmos equivalent that could maybe do with the change

efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

Line 1146 in 02d37a2

&& unaryExpression.Type.GetInterfaces().Any(e => e == operand.Type)

[roji]

Yeah, that one looks legit, though quite contrived - that seems to strip away up-casts from an interface to a base interface, so the change would also strip away no-op "up-casts" from the interface to itself. The compiler most probably never generates such a useless node, but we could include it for completeness.

[ChrisJollyAU]

changing it to && unaryExpression.Type.GetInterfaces().Append(unaryExpression.Type).Any(e => e == operand.Type) doesnt make any difference so not really needed

On this quick look, it doesn't appear that there are any that would need to be changed

[roji]

OK, thanks @ChrisJollyAU!