Roslyn-analyzers: CA1307 reported for LINQ Expression, suggested fix fails at runtime

[Bouke]

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.8 (latest release)

Diagnostic ID

Example: CA1307

Repro steps

new DbContext().MyEntities.Where(x => x.Name.StartsWith("Hello"))

Expected behavior

No diagnostic is reported; actual comparison is done inside SQL so the "thread" is not relevant (I think).

Actual behavior

Issue CA1307 is reported, suggesting "StartsWith(string, StringComparison)":

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in '...' with a call to 'string.StartsWith(string, System.StringComparison)'.

However that suggestion fails at runtime:

System.NotSupportedException: 'LINQ to Entities does not recognize the method 'Boolean StartsWith(System.String, System.StringComparison)' method, and this method cannot be translated into a store expression.'

[myblindy]

actual comparison is done inside SQL so the "thread" is not relevant (I think).

That is correct, your lambda is sent as an Expression, not as actual executable code, and EF/LinqToSql parses the expression tree to build its own Sql expression. If possible, these checks should absolutely not run on expression parsers like these.

[cryolithic]

Also ran into this same suggestion. I would prefer to err on the side of caution, and if this is found within the bounds of an IQueryable, the check should move on.

[langdonx]

You guys find the most creative way around this?

I'm thinking GlobalSupression.cs to just skip CA1307 altogether until it can be resolved in a better way.

[Evangelink]

Let me try to see if I can find how to fix this behavior.

[damieng]

A lot of these analyzers should probably be disabled for Expression<Func<T...> while enabled for Func<T...> as Roslyn can't possibly know enough about what the LINQ provider is going to do with the expression tree in the former.

(Title on this bug should probably be just changed to LINQ as it's a provider-independent problem and indeed the error message indicates that Entity Framework's LINQ is being used not the legacy LINQ to SQL)

[Evangelink]

@damieng can you think of any case that would be covered by excluding Expression<Func<T...>> which wouldn't be covered by checking that the IQueryable?

[damieng]

Expressions aren't tied to IQueryable and I can think of a number of scenarios where you might have standalone expressions - a comprehensive unit test suite for a LINQ provider might look at translation of expression parts themselves independently of IQueryable.

Is there some limitation in the analyzer that makes it harder for it to ignore Expression? (I would have thought the latter would be harder especially given its an interface and not the concrete class)

[Evangelink]

I don't think it is any harder, I was just trying to add a meaningful test that would prevent the analyzer to be changed in the future. I will just add a call to some dummy method taking Expression<Func<T...>>.