xUnit1010 is triggered for strings which are convertible to decimal / int

[Dreamescaper]

        [Theory]
#pragma warning disable xUnit1010 // The value is not convertible to the method parameter type
        [InlineData("2.234")]
#pragma warning restore xUnit1010 // The value is not convertible to the method parameter type
        public void TestAnalyzer(decimal d)
        {
            Assert.Equal(d, 2.234m);
        }

The test above works fine, as string value is converted to decimal internally. Still, xUnit1010 is produced (with Error priority by default).

[Dreamescaper]

Probably analyzer should be updated in a similar fashion to xunit/xunit.analyzers#97 .

[slaneyrw]

In this specific example, the test runner would be using Convert.ChangeType. Since the argument to InlineData is a string, which implements IConvertable, shouldn't the analyzer just check if the source argument's type implements IConvertable and the corresponding method parameter is one of the 15 standard types supported by the CLR, using matrix defined at

https://github.com/microsoft/referencesource/blob/master/mscorlib/system/convert.cs

You already have the 3 char special cases in the numerical conversion, so any fall should defer to runtime cast failures, yes ?

Therefore I don't think a version check should be required

[slaneyrw]

I've played with checking IConvertable support... there are a lot of cases that previously would be reported as a failure that would now be allowed.

Numerics <-> bool
bool -> string
true/false strings -> bool
strings -> almost anything

If you ignore the error (via pragma), all the above cases will work in practice. But doesn't mean the system should condone it. Maybe convert to a warning ?

[Dreamescaper]

Also Enum -> string

[slaneyrw]

Also Enum -> string

But not the reverse

[bradwilson]

@slaneyrw

shouldn't the analyzer just check if the source argument's type implements IConvertable and the corresponding method parameter is one of the 15 standard types supported by the CLR,

The problem is that IConvertible doesn't guarantee that the types can be converted to: https://github.com/microsoft/referencesource/blob/51cf7850defa8a17d815b4700b67116e3fa283c2/mscorlib/system/iconvertible.cs#L45-L51

// The ToXXX methods convert the value of the underlying object to the
// given type. If a particular conversion is not supported, the
// implementation must throw an InvalidCastException. If the value of the
// underlying object is not within the range of the target type, the
// implementation must throw an OverflowException.  The 
// IFormatProvider will be used to get a NumberFormatInfo or similar
// appropriate service object, and may safely be null.

There are several exceptional situations that it makes sense to wait until runtime to discover, but the "always fails with InvalidCastException because it's not a compatible type" is a tough one. You can't realistically new up instances of things in an analyzer and run the code and see what happens; even if it weren't punishingly slow to do at code analysis time, it's not guaranteed to even be possible.

I think the better logic here would be to stick to things which are known to convert, though even there I'm concerned about how that logic table gets updated over time. Maybe it's overthinking it, as the list of built-in types doesn't really grow often. But that's the way I would lean is to basically just copy the support table into code and do a lookup against that.