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 must have DisplayName to get DisplayName #214

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

[Feature] Enum must have DisplayName to get DisplayName #214

jzabroski opened this issue Apr 26, 2019 · 8 comments

Comments

@jzabroski
Copy link

public class AB { }
public enum ABEnum { A = 0, B = 1}
public class Test
{
  public void TestAB()
  {
    foreach (var item in Enum.GetValues(typeof(AB))) // Feature logged in https://github.com/DotNetAnalyzers/ReflectionAnalyzers/issues/212
    {
      Console.WriteLine(((Enum)item).GetDisplayName()); // This issue: verify the enum has a [DisplayName] attribute on each value of the enum
                     // ^^^^^^^^^^^^ bonus: don't raise Possible InvalidCastException if typeof(AB) is an enum.
    }
  }
}

where GetDisplayName is defined as an extension method:

    public static string GetDisplayName(this Enum enumValue)
    {
      return enumValue.GetAttribute<DisplayAttribute>()?.Name;
    }

Is this do-able? In the general case if GetDisplayName is in a different library, how will you know to peer into the extension method to validate its behavior? On one hand, the only scenarios I know of for putting extension methods on enums are exactly this scenario. On the other, I don't want to complicate ReflectionAnalyzers with potentially slow analyzers. Would this be efficient?

@JohanLarsson
Copy link
Collaborator

This is a potentially expensive analyzer as it requires walking via symbols. Expensive as in it will use some CPU and may slow down builds.

@jzabroski
Copy link
Author

Yeah, it's a shame this logic doesn't exist in netstandard API base. Everyone copies and pastes the same StackOverflow answer for how to do this everywhere.

@JohanLarsson
Copy link
Collaborator

If it is in a different library it is tricky, if it is in a nuget package we can specialcase it.

@jzabroski
Copy link
Author

I guess the solution is to search StackOverflow for all the code snippets and edit the posts so that people are encouraged to specialcase with the nuget package we provide. One more yak to shave before the day is won.

@JohanLarsson
Copy link
Collaborator

An alternative is to use heuristics, name of the method, return type etc. simple but could mean false negatives. If we go that route we want separate id for sure so it can be disabled if it is annoying.

@jzabroski
Copy link
Author

I would if such an approach should have a second assembly of analyzers called ReflectionAnalyzers.Expensive and that the warning in the base library should suggest you can disable it and enable the expensive one instead.

@brian-reichle
Copy link

I would suggest that if you have a package to introduce this api, then that package should probably have it's own analyzer to enforce it's correct usage rather than relying on ReflectionAnalyzers.

@jzabroski
Copy link
Author

That makes a lot of sense - as a consultant, if I jump from project to project, all I would have to do is pull in my project and have it pull in the analyzer as a private asset and I could automatically know when team members are not following my architecture.

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

3 participants