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

Tokenizer arrow functions backfill tests: fix it ;-) #2863

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Feb 9, 2020

Follow up on #2860. This fixes the tests.

Test case file:

  • Adds tests with mixed case fn keyword.
  • Removes a few redundant tests (weren't testing anything which wasn't covered already).

Test file:

  • Adds a couple of tests which were missing from the data provider.
  • Updates the test itself to use a $testContent parameter to allow for the fn in the tests being non-lowercase.

Follow up on 2860. This fixes the tests.

#### Test case file:
* Adds tests with mixed case `fn` keyword.
* Removes a few redundant tests (weren't testing anything which wasn't covered already).

#### Test file:
* Adds a couple of tests which were missing from the data provider.
* Updates the test itself to use a `$testContent` parameter to allow for the `fn` in the tests being non-lowercase.
@jrfnl jrfnl force-pushed the feature/2860-tokenizer-arrow-function-backfill-fix-tests branch from 78c5000 to 2adcb64 Compare February 9, 2020 23:20
@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 9, 2020

Rebased, added mixed case tests and updated the PR description above. Should be good to go.

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 9, 2020

Related to #2523

@gsherwood gsherwood merged commit 2adcb64 into squizlabs:master Feb 9, 2020
@jrfnl jrfnl deleted the feature/2860-tokenizer-arrow-function-backfill-fix-tests branch February 9, 2020 23:45
@gsherwood
Copy link
Member

Thanks a lot

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 9, 2020

@gsherwood You're welcome.

gsherwood pushed a commit that referenced this pull request Feb 9, 2020
This commit implements the proposal as outlined in 2859#issuecomment-583695917.

The `fn` keyword will now only be tokenized as `T_FN` if it really is an arrow function, in which case it will also have the additional indexes set in the token array.

In all other cases - including when the keyword is used in a function declaration for a named function or method -, it will be tokenized as `T_STRING`.

Includes numerous additional unit tests.

Includes removing the changes made to the `File::getDeclarationName()` function and the `AbstractPatternSniff` in commit 37dda44 as those are no longer necessary.

Fixes #2859

Fixed tests for #2860

Tokenizer arrow functions backfill tests: fix it ;-)

Follow up on 2860. This fixes the tests.

* Adds tests with mixed case `fn` keyword.
* Removes a few redundant tests (weren't testing anything which wasn't covered already).

* Adds a couple of tests which were missing from the data provider.
* Updates the test itself to use a `$testContent` parameter to allow for the `fn` in the tests being non-lowercase.

Added a few more tests for the T_FN backfill (ref #2863, #2860, #2859)
tobias-trozowski pushed a commit to tobias-trozowski/PHP_CodeSniffer that referenced this pull request Feb 13, 2020
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.

2 participants