Skip to content

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

2 participants