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

IDE0051 should not be reported when methods are referenced in nameof() expressions #31581

Closed
jhinder opened this issue Dec 6, 2018 · 4 comments
Assignees
Labels
Area-IDE Bug Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@jhinder
Copy link
Contributor

jhinder commented Dec 6, 2018

Version Used: VS 16.0 Preview 1

Steps to Reproduce:

  1. Copy the code into a C# file.
public string P => nameof(M);
private void M() { }

Expected Behavior: IDE0051 is not reported for M.

Actual Behavior: IDE0051 is reported for M. The code fix breaks any code that references M.

@mavasani
Copy link
Contributor

mavasani commented Dec 6, 2018

The diagnostic is by design, though the wording should be improved for this case - why would someone define a method only to use it's name? We should avoid offering a code fix for this case.

@jhinder
Copy link
Contributor Author

jhinder commented Dec 6, 2018

why would someone define a method only to use it's name?

Reflection. type.GetMethod(nameof(PrivateImplMethod))...

Our application server uses a framework that exposes some interfaces with methods whose parameters are either System.Type or its own reflection API types. But the ORM we use uses type parameters. How do you bridge those two? We found the easiest solution was to create a private method that has the type parameter, and call that method with reflection.

That might look something like this:

// This is an implicit implementation of an interface method, thus we can't change the signature:
public bool DoSomething(ITypeInfo type, string id)
{
    return (bool)typeof(ThisType)
        .GetMethod(nameof(DoSomethingInternal), /* binding flags */)
        .MakeGenericMethod(type.Type)
        .Invoke(this, new object[] { id });
}

private void DoSomethingInternal<T>(string id)
{
    return _queryService.Query<T>(id) // ...
}

I'll admit, this is about the only case I can think of where you'd define a method only to use its name, but it's a bit odd to see IDE0051 complain about the method not being used, while the CodeLens info right above tells you "1 reference".

@mavasani
Copy link
Contributor

mavasani commented Dec 6, 2018

@jhinder Thanks for the detailed explanation. I will fix this case to ensure we report IDE0052 with a different message for methods (Private method '{0}' can be removed as it is never invoked.). We also don't offer a code fix for IDE0052.

IDE0051 and IDE0052 diagnostics are not written to account for reflection based usage, you basically don't even need to have a name reference to access a member symbol via reflection. I would recommend the following for code bases that use reflection to access private members:

  1. Disable IDE0052 if you follow the pattern mentioned here and have name only references to members for reflection.
  2. Disable both IDE0051 and IDE0052 if you access private members through reflection by using constant strings for member names.
  3. Once Allow an analyzer to suppress diagnostics #20242 is implemented, you can potentially write a diagnostic suppressor extension that analyze instances of IDE0051 and IDE0052 to detect reflection usage and suppress false positives in such a context.

@mavasani mavasani added the 4 - In Review A fix for the issue is submitted for review. label Dec 6, 2018
@jinujoseph jinujoseph modified the milestones: 16.0, 16.0.P2 Dec 10, 2018
@gafter gafter removed 4 - In Review A fix for the issue is submitted for review. labels Dec 17, 2018
@sharwell sharwell added the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Dec 22, 2018
@mavasani
Copy link
Contributor

FYI @jhinder - #32675 makes the analyzer conservative by not flagging symbols which are used in nameof expressions. You will still get reports of the reflection uses explicit strings instead of nameof expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

No branches or pull requests

6 participants