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

Implement diagnostic and code fix for SA1101 PrefixLocalCallsWithThis #447

Merged
merged 5 commits into from
Feb 9, 2015

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Feb 3, 2015

Fixes #35

The number of test cases is large to say the least. Basically I need to choose one item from each of the categories below and build a test for that set of conditions.

  • Element type
    • Type
    • Method
    • Property
    • Field
    • Event
    • Parameter
    • Local
  • Inheritance
    • New element
    • Override
    • Interface implementation
    • Explicit interface implementation
    • Hides static base element
    • Hides instance base element
  • Is Static
    • True
    • False
  • Expression type
    • Assignment to element
    • Read from (simple, e.g. var x = this.Property;)
      • For methods this is a reference to the method group
    • Read from (qualified, e.g. var x = this.Property.GetHashCode();)
    • nameof expression
  • Expression location
    • Attribute value
    • In static member
    • In instance member
    • Field initializer for static field
    • Field initializer for instance field
    • String interpolation
    • Documentation comment
  • Receiver
    • Current instance (or the containing type if Is Static is true)
    • Another instance

This gives a maximum of 4704 tests for complete coverage, although some combinations do not make since (e.g. you cannot assign to an element within a documentation comment or nameof expression).

@sharwell
Copy link
Member Author

sharwell commented Feb 3, 2015

I think the only reasonable way to do the testing for this is create a helper method that takes an enum value representing each of the above category and builds a test for that case.

@sharwell
Copy link
Member Author

sharwell commented Feb 7, 2015

nameof poses a remarkably bizarre set of challenges for this feature.

  • Instance members of a type can be referenced without an instance of the type available.
  • The nameof expression can be qualified with this when the target of the expression is an instance member, as long as you are in a context where this is available.
  • If the target of the expression is a method group, you can use this even if none of the methods in the group are instance methods.
  • The expression can always be qualified with the name of the containing class, regardless of whether or not the target is an instance member.

@sharwell
Copy link
Member Author

sharwell commented Feb 7, 2015

Since I really want to get this feature in a release, I'm implemented initial testing on a variety of edge cases in the above list.

@sharwell
Copy link
Member Author

sharwell commented Feb 9, 2015

I'm merging this one now that tests are in place so it gets included in the Alpha 2 release.

sharwell added a commit that referenced this pull request Feb 9, 2015
Implement diagnostic and code fix for SA1101 PrefixLocalCallsWithThis
@sharwell sharwell merged commit 1bc13f8 into DotNetAnalyzers:master Feb 9, 2015
@sharwell sharwell deleted the fix-35 branch February 9, 2015 03:02
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

Successfully merging this pull request may close these issues.

SA1101: PrefixLocalCallsWithThis
1 participant