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

Add arrow function support #179

Merged
merged 22 commits into from
Jul 7, 2020
Merged

Add arrow function support #179

merged 22 commits into from
Jul 7, 2020

Conversation

sirbrillig
Copy link
Owner

@sirbrillig sirbrillig commented May 16, 2020

Fixes #134

@sirbrillig sirbrillig mentioned this pull request Jun 13, 2020
@sirbrillig
Copy link
Owner Author

The problem I'm running into is that I don't know how to find out where the scope ends for an arrow function. PHPCS doesn't seem to consider it a scope at all:

        Process token 55 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 56 on line 9 [opener:49;]: T_FN => fn
        Process token 57 on line 9 [opener:49;]: T_OPEN_PARENTHESIS => (
        Process token 58 on line 9 [opener:49;]: T_VARIABLE => $foo
        Process token 59 on line 9 [opener:49;]: T_CLOSE_PARENTHESIS => )
        Process token 60 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 61 on line 9 [opener:49;]: T_DOUBLE_ARROW => =>
        Process token 62 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 63 on line 9 [opener:49;]: T_VARIABLE => $foo
        Process token 64 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 65 on line 9 [opener:49;]: T_STRING_CONCAT => .
        Process token 66 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 67 on line 9 [opener:49;]: T_VARIABLE => $bar
        Process token 68 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 69 on line 9 [opener:49;]: T_STRING_CONCAT => .
        Process token 70 on line 9 [opener:49;]: T_WHITESPACE => ·
        Process token 71 on line 9 [opener:49;]: T_VARIABLE => $subject
        Process token 72 on line 9 [opener:49;]: T_SEMICOLON => ;
        Process token 73 on line 9 [opener:49;]: T_WHITESPACE => ·

However, variables defined inside the arguments list for the function ($foo in the above example) do not exist outside of the arrow function.

There's no obvious token to use to find the end to an arrow function; it's the end of the expression but I'm not sure of an easy way to find that.

@sirbrillig
Copy link
Owner Author

Ah, I was wrong, there's additional processing done to find the arrow function scope:

        => token 459 on line 57 processed as arrow function
                * scope opener set to 464 *
                * scope closer set to 467 *
                * parenthesis opener set to 460 *
                * parenthesis closer set to 462 *
                * token 464 on line 57 changed from T_DOUBLE_ARROW to T_FN_ARROW
        * token 448 on line 56 changed from T_OPEN_SQUARE_BRACKET to T_OPEN_SHORT_ARRAY
        * token 449 on line 56 changed from T_CLOSE_SQUARE_BRACKET to T_CLOSE_SHORT_ARRAY

@jrfnl
Copy link
Collaborator

jrfnl commented Jun 17, 2020

@sirbrillig Arrow function support in PHPCS was only added in PHPCS 3.5.3 and was pretty broken until 3.5.5 and there are still bugs being found.

As PHPCS 3.1 is your minimum, you will need to sniff for both T_STRING as well as T_FN tokens.

The PHPCSUtils Collections::arrowFunctionTokensBC(), FunctionDeclarations::isArrowFunction() and the FunctionDeclarations::getArrowFunctionOpenClose() methods can help you handle all this in a PHP cross-version manner.

Caveat: that is: in so far as possible of course, as when PHPCS sets the wrong scope_opener somewhere making all scope_openers after that for the rest of the file be off, that's not something PHPCSUtils can conceivably correct for.

@jrfnl
Copy link
Collaborator

jrfnl commented Jun 17, 2020

Just noticed that you are changing the minimum PHPCS version to 3.5 in this PR ;-) Even so, that still won't solve the issue as, as I mention before, arrow function support is basically broken until master.

@sirbrillig
Copy link
Owner Author

I'm just experimenting so far. I've got it to recognize the scope of variables defined within an arrow function. Now I just need to get it to properly scan the variables within that scope at scope close. The tricky thing is that variables in the scope also include variables from the parent scope.

@sirbrillig sirbrillig force-pushed the add-arrow-function-support branch from f52c260 to e079598 Compare July 5, 2020 01:12
@sirbrillig
Copy link
Owner Author

sirbrillig commented Jul 6, 2020

Hey! We're getting there! Arrow functions now work.

I got stuck because there's a bunch of old poorly-named, poorly-documented code lying around that I've been meaning to refactor for a long time. I spent a few days tearing the scope detection apart and then re-building it from scratch and I think it works much better now. I need to rebase this PR to make some of the commits make sense, but I'll wait till this is all done.

Now just to support PHP 5.6...

@sirbrillig sirbrillig force-pushed the add-arrow-function-support branch from 0cdaf57 to e099bc9 Compare July 6, 2020 22:50
@sirbrillig
Copy link
Owner Author

sirbrillig commented Jul 6, 2020

✅ all tests pass

💥 🥳 💥

@sirbrillig
Copy link
Owner Author

Arrow function support in PHPCS was only added in PHPCS 3.5.3 and was pretty broken until 3.5.5 and there are still bugs being found.

Coming back around to this, since we rely on scope_opener, scope_closer, and conditions from a T_FN token, if we can't trust them we'll need to use your work-arounds. I'll see what I can do.

@sirbrillig sirbrillig force-pushed the add-arrow-function-support branch from 35e2583 to fc01fa6 Compare July 7, 2020 00:29
@sirbrillig sirbrillig marked this pull request as ready for review July 7, 2020 00:29
@sirbrillig sirbrillig force-pushed the add-arrow-function-support branch from fc01fa6 to 79dec4c Compare July 7, 2020 00:45
@sirbrillig sirbrillig merged commit 58b0fd8 into master Jul 7, 2020
@sirbrillig sirbrillig deleted the add-arrow-function-support branch July 7, 2020 00:48
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.

Add support for PHP 7.4 arrow functions
2 participants