Distinguish reference usages

I have a plugin that appends "this." to the front of member variables and method calls (in the same class). It has one problem that I can't figure out how to solve and that is when a member variable of another instance of the same class is referenced it sees that as a usage and appends "this." to it. This is most common in an equals() method. For example, with this class:

public class Person {
    private String name;

    public Person() {
        this.name = "Smitty Johnson";
    }

    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        Person person = (Person) o;
        return name.equals(person.name);

    }

    @Override
    public int hashCode() {
        return name.hashCode();
    }
}

My plugin will cause the return statement in the equals() method to become "return this.name.equals(person.this.name)" because it sees "name" as a reference for a member variable and dutifully appends "this." to it. With a PsiElement how can distinguish that the usage belongs to another instance of the same class (i.e. how can I get it to leave "person.name") alone?

Here is the code that accomplishes it (more or less, the actual code also does the same thing for methods, filters out static fields, filters out some special cases, and some error checking but this is the heart of it. I also don't show the code to get the document and current class):

Document document = null; //get document from editor
PsiClass currentClass = null; //get the class from the editor

PsiElement[] allFields = currentClass.getFields();
for (PsiElement field : allFields) {
    final Collection<PsiReference> references = ReferencesSearch.search(field).findAll();
    for (PsiReference reference : references) {
         final PsiElement element = reference.getElement();
         int offset = element.getTextOffset();
         document.insertString(offset, "this.");
         PsiDocumentManager.getInstance(this.project).commitDocument(document);
    }
}

I have stared at the the document tree in the PsiViewer plugin (btw, that is very handy!) and nothing is jumping out at me to keep person.name from becoming person.this.name.

11 comments
Comment actions Permalink

are you aware that an inspection exists that does exactly that ?

0
Comment actions Permalink

i didn't know that until shortly after I wrote it.  I still have yet to figure out how to trigger the inspection you mention and since I do now how to trigger my plugin I guess I will stick with my plugin. I have tried the normal alt-enter inspection trigger but can never get anything about adding "this." to appear.

0
Comment actions Permalink

I figured it out. I can compare the text of the PsiIdentifier to the text of the PsiReference, if they don't match the member has a prefix of some sort.

0
Comment actions Permalink

The inspection is called "Instance field not qualified with 'this'" and it has a quick fix. It is disabled by default, because not many developers want its behavior, but you can enable it in the Errors dialog accessible by clicking on the text to the right of "Hector the Inspector" in the status bar.

Bas

0
Comment actions Permalink

The elements return by calling getElement() on the references can be cast to PsiReferenceExpression (if they are references from java code). On PsiReferenceExpression there are the methods isQualified(), getQualifierExpression() and getQualifier() you might use. You can do something like this for example:




        JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
        PsiElementFactory factory = psiFacade.getElementFactory();
        PsiElement[] allFields = currentClass.getFields();
        for (PsiElement field : allFields) {
            for (PsiReference reference : ReferencesSearch.search(field)) {
                final PsiElement element = reference.getElement();
                if (element instanceof PsiReferenceExpression) {
                    PsiReferenceExpression referenceExpression = (PsiReferenceExpression) element;
                    if (!referenceExpression.isQualified()) {
                        final PsiExpression newExpression =
                                factory.createExpressionFromText(
                                        "this." + referenceExpression.getText(),
                                        referenceExpression);
                        referenceExpression.replace(newExpression);
                    }
                }
            }
        }



0
Comment actions Permalink

I found the inspection. Cool. There are actually a lot of inspections that are off, might have to look through them to see what other goodies are in here. Although the inspection doesn't do method calls, just members. My plugin also does methods. I searched for a inspection that did methods but there doesn't appear to be one (or I just can't find it).

0
Comment actions Permalink

Thanks for the isQualifer() pointer! Going to check that out.

Also, thanks for the createExpressionFromText() code! I remember seeing that when I first started looking at how to accomplish this, but for some reason couldn't put all the pieces together to figure it out. Although that was before I had a decent grasp on the PSI model so at the time I probably just didn't have the context I needed to properly use it.

That seems superior to to the insertString() way I am doing. Will it perform better as well?

0
Comment actions Permalink

mjparme wrote:

Although the inspection doesn't do method calls, just members. My plugin also does methods. I searched for a inspection that did methods but there doesn't appear to be one (or I just can't find it).

The inspection intentionally does not highlight method calls currently. I considered it more useful for field accesses because it helps there to disambiguate between accesses to local variables, static fields and instance fields. For method calls the only types of methods that can be called are instance methods and static methods. Thus by qualifying static methods with their class name (there is an inspection for that), all chance of confusion is removed.
If necessary I could add an option to the inspection to check method calls too.

Bas

0
Comment actions Permalink

mjparme wrote:

That seems superior to to the insertString() way I am doing. Will it perform better as well?

It should perform better, but I have not tested that. It is the way all of IntelliJ IDEA's refactorings and inspections modify code, thus I imagine performance is excellent:-)

Bas

0
Comment actions Permalink

I like it being on method calls too, for no particular reason that I can articulate though, probably just because I am obsessive compulsive about my code:-) Not sure if it would help other people to add methods to the inspection, no one else may be as anal as me:-)

0
Comment actions Permalink

It seems a little snappier now compared to when I was using insertString(). I didn't do any real benchmarks though, just a seat-of-the-pants benchmark:-)

0

Please sign in to leave a comment.