Autocomplete, colorization and goto type definition behaves differently on the same variable

Issue #2038 resolved
Attila Hajdrik created an issue

request variable is resolved to System.Request class and popup help is not looking at the immediate or other scopes first, before going to System.* types to resolve it. Colorization also shows red for params, but funny enough that autocomplete works, it shows the right properties for RestRequest as `params` was added with autocomplete.

If I command click on `request` before .params then it goes to the System.Request type.

So everything looks like a mixup.

Autocomplete:

Help/goto type definition:

Comments (7)

  1. Scott Wells repo owner

    Ah. I think the issue is that the parser at that point is in a state where it thinks that you're entering the type for a variable declaration based on the next line of code after that one (the one that beings with String, though it's obscured. The parser doesn't care about whether intermediate whitespace is based on spaces, tabs, or newlines, so it sees:

    request.params String ...
    

    And of course the case-insensitive nature of Apex isn't helping either.

    As a result IC2 is adding a type reference for System.Request (where System is of course an optional/implicit namespace) to what should be a variable reference to request and is then evaluating the rest of the expression as thinking that you're trying to reference a class constant within that class named params.

    I think if you stub a line terminator the parser will recover properly and you'll see the correct behavior, e.g.:

    request.params<caret>;
    ...
    String ...
    

    Now obviously you shouldn't have to do that. I'll take a look at the code that adds that type reference and see if I can make it not do so when there's more to the expression, in this case the .params part. It can just be tricky in these transient moments when you have partially-formed code where one lexical construct overlaps with another, in this case an incomplete expression just before an existing variable declaration (or other expression statement that doesn't begin with a well-known Apex keyword).

    So that's probably more than you wanted to know, but the reality is that this may not be as simple to address as it seems like it should be. I'll definitely take a look, though, and see what I can do to have the parser recover a bit more gracefully in situations like this.

  2. Scott Wells repo owner

    Okay, I'm just about to commit the fix for this:

    Issue2038.png

    Again more than you probably care to know, but the real issue ended up being a singular representation of what a declaration could be named in IC2's Apex parser such that non-class declarations were allowed to use the names of Apex's "primitive" data types, e.g., String, Integer, Boolean, etc. But in reality those names are reserved for Apex classes...with one exception, the enum constants in the Schema.DisplayType enum. So I just made IC2's Apex parser more selective about which names are allowed for which declarations, and the parser now recovers very gracefully when you have an incomplete statement just before a declaration of an Apex primitive type.

    This fix will be included in the next released build, likely Thursday morning (my time).

  3. Attila Hajdrik reporter

    I appreciate all the details Scott, and thanks for the fix, it will enhance the DX for sure.

    Making APEX case insensitive…and their reserved word handling…don’t know who was the original language designer, but…just makes a lot of things harder, like naming things would not be hard already 😃 😃 😃

  4. Scott Wells repo owner

    Yup. There are some very interesting examples, e.g., this one that I posted to the GDS Slack group recently:

    Fun with Apex, January 2022 edition...

    An IC2 user has brought to my attention some OST omissions/inaccuracies in the sfdc_checkout namespace. In the process of updating IC2 to correct these, I found that the class sfdc_checkout.IntegrationStatus has an inner enum called Status and a property called status (remember that Apex is case-insensitive), and the latter is an instance of the former.

    This all manifests in Apex as:

    global class /*Sfdc_Checkout.*/IntegrationStatus {
        global enum Status {
            FAILED,
            SUCCESS
        }
    
        global Sfdc_Checkout.IntegrationStatus.Status status;
    }
    

    But put another way, the full declaration of that property (pseudo-code) is:

    sfdc_checkout.IntegrationStatus.Status sfdc_checkout.IntegrationStatus.status;
    

    which, because Apex is case-insensitive, can also be written as:

    sfdc_checkout.IntegrationStatus.status sfdc_checkout.IntegrationStatus.status;
    

    Sigh... (edited)

  5. Scott Wells repo owner

    There was a bit of fallout from the original fix for this that should be resolved in 2.2.0.6. Nothing big, but if you noticed slightly weird behavior after updating to 2.2.0.5 with class, constructor, or enum constant declarations/references, it should be resolved in 2.2.0.6. If not, please let me know.

  6. Log in to comment