Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Enum InvalidCastException analysis #215

Open
jzabroski opened this issue Apr 26, 2019 · 2 comments
Open

[Feature] Enum InvalidCastException analysis #215

jzabroski opened this issue Apr 26, 2019 · 2 comments

Comments

@jzabroski
Copy link

Placeholder to fully describe positive and negative test cases.

public enum ABEnum { A = 0, B = 1}
public class Test
{
  public void TestAB()
  {
    foreach (var item in Enum.GetValues(typeof(ABEnum)))
    {
      Console.WriteLine(((Enum)item).GetDisplayName()); // Feature logged in: https://github.com/DotNetAnalyzers/ReflectionAnalyzers/issues/214
                     // ^^^^^^^^^^^^ bonus: don't raise Possible InvalidCastException if typeof(ABEnum) is an enum defined as an int64 sub-class (default scenario).
    }
  }
}

Ideally, the code will recommend the correct cast to the appropriate integer size. For example, if the lvalue being assigned to is an int, and the enum is a long, it will recommend a down-cast. If the lvalue being assigned to is a long, and the enum is a short, there will be no loss of precision but it may make sense to warn low severity about data type mismatch.

@brian-reichle
Copy link

I cant seem to reconcile your description with the sample code.

Is the issue with casting the result of Enum.GetValues to the correct type?

foreach (var item in Enum.GetValues(typeof(ABEnum)))
{
    Console.WriteLine((ABEnum)item); // ok, we know item is an instance of ABEnum
    Console.WriteLine((Enum)item); // ok, ABEnum inherits from Enum
    Console.WriteLine((int)(ABEnum)item); // ok, there is a conversion cast from ABEnum to int.

    Console.WriteLine((int)item); // not ok, we need to perform the unboxing cast before we can perform the conversion cast.
}

or is the issue with value truncation?

public enum ABEnum { A = 0, B = 1 }
public enum CDEnum : long { A = 0, B = 1 }

void Foo(ABEnum a, CDEnum c)
{
    var value1 = (int)a; // ok
    var value2 = (long)c; // ok

    var value3 = (int)c; // warn - truncating conversion.
    var value4 = (long)a; // ok - widening conversion.
}

@jzabroski
Copy link
Author

jzabroski commented Apr 27, 2019

It should be split into two cases. I was just quickly jotting down thoughts as fast and clear as possible while trying to get my tasks done at work.

Specifically, truncation should be a separate issue. There should probably be a third analysis for out of range casts:

public enum ABEnum { A = 1, B = 2 }
public bool TestAB() {
  const int c = 3;
  return (ABEnum)c; // out of range value for cast
} 

For out of range, it should probably suggest checking (typeof(ABEnum).IsEnumDefined(c)) if the input is statically unknown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants