Refactor Introduce Variable doesn't work if expression includes this.XXX

Issue #2006 resolved
Eric Kintzer created an issue

Starting with this simple code in Anonymous Apex

class Foo {
    Map<Id,Contact> contactsById = new Map<Id,Contact>();
    void doStuff(Id contactId) {
        Decimal count = this.contactsById.get(contactId).Kit_Replacement_Request_Count__c;
    }
}

If you highlight the expression this.contactsById.get(contactId and Refactor | Introduce Variable, it does not work, you get error Selected block should represent an expression

If you remove the this. and select contactsById.get(contactId) and Refactor | Introduce Variable, it works fine.

Presumably same issue with super.XXX although I did not try the super. use case.

Comments (12)

  1. Scott Wells repo owner

    Thanks for filing, Eric. Yes, the same thing happens with super. qualifiers which is not surprising given that those are both specific--and almost identical--expression types in IC2's Apex parser. This is actually less about the refactoring than about the resulting AST for that expression. You can see the difference if you use the Extend Selection action on the expression where you'll see that for this.<expression>.<otherExpression>.<yetAnotherExpression>, it goes straight from selecting this to selecting the entire chained expression. Conversely, if you remove the this. qualifier, it properly extends the selection one expression at a time. Perhaps it has something to do with associativity? Not sure, but I'll dig in and figure it out next week. I'm definitely curious...

  2. Xander Victory

    also seems to happen with unqualified static method calls from the same class - adding the class name in front makes it work

  3. Scott Wells repo owner

    Okay, good to know. I'll work on both of these for next week's build as this week's is already pretty much ready to go. Thanks for providing the example.

  4. Scott Wells repo owner

    Xander, I'm not seeing the behavior you describe with an unqualified static method invocation:

    Issue_2006_static.png

    Obviously I do see it in your screenshot, though, so I know it's happening. Can you provide a simple standalone example that consistently reproduces that behavior?

  5. Scott Wells repo owner

    I have a prospective fix for the original issue with this. and super. expressions. It won't go into this week's build as I definitely want significantly more test time since it's a parser change, albeit a small one. I'm also going to see if I can treat expression selectors as pseudo-expressions so that .class, array index operators, etc., can also be extracted distinctly. I'll provide an update sometime over the next week as I get it ready for release.

  6. Xander Victory

    It seems to be related to the assignment to the custom field… The first screenshot was where it was being assigned to the field too (and is now the line above after manual refactor)

  7. Scott Wells repo owner

    The core issue with this. and super. will be fixed in tomorrow's build:

    Issue_2006.png

    I'll see if I can reproduce the variant that Xander reported, but it's unlikely a fix for that would be in tomorrow's build at this point unless it's a brain-dead easy fix.

    In the process I also implemented a partial fix for including expression selectors such as array index operators distinctly when doing extract/introduce refactorings, but that change is pretty involved and won't go into this build either. The current fix is very focused on making sure that this. and super. expressions are handled properly, both in those refactorings and in a few other situations.

  8. Scott Wells repo owner

    Xander, I'm still unable to reproduce the issue you're seeing. I've tried it with the following:

    Datetime now = System.now();
    al__Foo__c foo = new al__Foo__c(
        al__Text__c = UserInfo.getName()
    );
    foo.al__Datetime__c = now;
    

    by trying to extract now on the RHS of the assignment in the last line with the caret placed pretty much everywhere in the identifier name and with the identifier name selected. In all cases it's successfully extracting it. Perhaps I fixed it with some of these changes I've made?

  9. Xander Victory

    Hi Scott,

    Mine was specifically with static methods. Small repro:

    public class TestICIssue
    {
        public static Datetime someStaticMethod(String foo) {
            return null;
        }
    
        public static void main() {
            Contact foo = new Contact();
            foo.CreatedDate = someStaticMethod('blah');//this line
        }
    }
    

  10. Scott Wells repo owner

    Xander, I'm still not seeing the issue:

    Issue_2006_again.png

    I tried to start the refactoring at the beginning, end, and middle of the method invocation, and in all cases it worked. Perhaps I did fix this in the next build. Let me get it out there and if it's still happening for you after updating, let's chat through the steps to reproduce.

  11. Scott Wells repo owner

    Fix for the originally-reported problem included in 2.2.0.0. Xander, if you're able to reproduce your problem after updating, please open a new issue with the details since it's different from the originally-reported one.

  12. Log in to comment