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

Merge in upstream #1

Merged
merged 160 commits into from
Mar 27, 2021
Merged

Merge in upstream #1

merged 160 commits into from
Mar 27, 2021

Conversation

cdayjr
Copy link
Owner

@cdayjr cdayjr commented Mar 27, 2021

No description provided.

jrfnl and others added 30 commits March 17, 2020 04:18
As `declare()` is recognized as a parentheses owner, there is no need for it to be listed in the "additional tokens indicating that parenthesis are not arbitrary" list.

Tested via an existing test: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Generic/Tests/WhiteSpace/ArbitraryParenthesesSpacingUnitTest.inc#L29
I was running into this while working on another sniff. One trivial example I found to demonstrate the bug is `<?php return array_map(`. This can happen when PHPCS runs on a file I'm currently typing in, but the file is not complete yet.

I believe it's safe to assume the 'parenthesis_closer' array key only exists on T_OPEN_PARENTHESIS tokens, and the 'bracket_closer' key only on T_OPEN_CURLY_BRACKET tokens. I had a quick look at the tokenizer and I believe this is indeed the case. But I might be wrong.

Sorry for not providing a test. I would love to, but I'm not familiar enough with the test setup here.
…ForInterface, TraitSuffixRequiredForTrait sniffs to Generic.NamingConventions
The logic could get confused when `goto` would be used in a mixed PHP/HTML file.

Includes adding a limited set of unit tests for the `T_GOTO_LABEL` tokenization.
PHP 8.0 introduces named function call parameters:
```php
array_fill(start_index: 0, num: 100, value: 50);

// Using reserved keywords as names is allowed.
array_foobar(array: $array, switch: $switch, class: $class);
```

Ref: https://wiki.php.net/rfc/named_params

This PR adds support to PHPCS for named function call arguments by adding a special custom token `T_PARAM_NAME` and tokenizing the _labels_ in function calls using named arguments to that new token, as per the proposal in 3159.

I also ensured that the colon _after_ a parameter label is always tokenized as `T_COLON`.

Includes some minor efficiency fixes to the code which deals with the colon vs inline else determination as there is no need to run the "is this a return type" or the "is this a `case` statement" checks if it has already been established that the colon is a colon and not an inline else.

Includes a ridiculous amount of unit tests to safeguard the correct tokenization of both the parameter label as well as the colon after it (and potential inline else colons in the same statement).
Please also see my comment about this here: #3159 (comment)

**Note**: The only code samples I could come up with which would result in "incorrect" tokenization to `T_PARAM_NAME` are all either parse errors or compile errors. I've elected to let those tokenize as `T_PARAM_NAME` anyway as:
1. When there is a parse error/compile error, there will be more tokenizer issues anyway, so working around those cases seems redundant.
2. The code will at least tokenize consistently (the same) across PHP versions. (which wasn't the case for parse errors/compile errors with numeric literals or arrow functions, which is why they needed additional safeguards previously).

Fixes 3159
This commit:
* Adds a GH  Actions workflow for the PHPStan check which was previously run on Travis.
*  Removes that part of the `.travis.yml` configuration.

Notes:
Builds will run on all pushes and on pull requests, with the exception  of those just modifying files which are irrelevant to this workflow.
This commit:
* Adds a GH  Actions workflow for the build which validated the XML file against schema and checked their code style consistency, as well as validate the PEAR package file, as was previously run on Travis.
*  Removes that part of the `.travis.yml` configuration.

Notes:
Builds will run on all pushes and on pull requests, with the exception  of those just modifying files which are irrelevant to this workflow.
The PEAR native package validation script used to be run on every PHP build, but the results should be no different on any particular PHP version.

To that end, move the running of the script from the "test" cycles, to the one-time only run PEAR specific job.
This commit:
*  Adds a GH Actions workflow for the CI checks which were previously run on Travis as the default test script.
    Includes only running the base tests for the builds using custom ini setting.
* Removes the, now redundant,  `.travis.yml` configuration.
* Remove the custom PHP ini configurations.
    Setting these is now handled in the script itself.
* Updates the `.gitattributes` file.
* Updates the  "Build Status" badge in the Readme to use the results from the GH `Test`  Actions runs.
…return type

This adds some additional safeguards to the sniff to prevent it from triggering when `static` is used in a return type declaration, as allowed since PHP 8.0.

Includes unit tests.

Fixes 3188
jrfnl and others added 29 commits March 5, 2021 02:22
The `Fixer::generateDiff()` method calls the `shell_exec()` method to retrieve the `diff` between two files.

In the unit tests, this is used to compare the expected content in a `.fixed` file with the generate fixed file contents.
If the expected content matches the generated content, the diff command will produce no output and `shell_exec()` will return `null`.
Ref: https://www.php.net/manual/en/function.shell-exec.php

This result is subsequently passed to `explode()` as the second parameter, but `explode()` only excepts strings as the second parameter.
Ref: https://www.php.net/manual/en/function.explode

As of PHP 8.1, this will generate a deprecation notice `explode(): Passing null to parameter #2 ($string) of type string is deprecated`.

Discovered while testing an external standard against PHPCS `master` on PHP 8.1.

Fixed now.
…arameters

... to confirm that the sniff does not break on these.
The new test was giving _three_ instead of _two_ errors on line 194 for this snippet `$padding * -1 + 3`, with the errors being thrown on the `*`, `-` and the `+` operators.

The `-` operator, however, is a unary operator and should not trigger this error.

Fixed now.
…erty promotion

Prevent the sniff from ignoring the spacing after the scope keyword in case of PHP 8.0 constructor property promotion.

As it was, the sniff would presume "normal" property syntax, which meant that in the case of constructor property promotion in a multi-line constructor declaration, the sniff would look for a semicolon to end the statement and bow out when the semicolon wasn't found.

By explicitly checking for constructor property promotion and skipping the next above mentioned check in that case, we ensure that scope keywords in constructors are still handled correctly.

Includes tests.
…l parameters

... to confirm that the code snippet in the error message will be correct (set the `$contextLength` to a higher value to confirm).
…ll parameters

... to confirm that the sniff does not break on these.
…hods.FunctionCallSignature in a match block (ref #3255)
In `PHP::processAdditional()`, in the part which retokenizes `T_BITWISE_OR` tokens to `T_TYPE_UNION`, the code already took reference operators and spread operators between the type declaration and the variable into account, but would also increment the current position in the loop, while this is already done in the `for()`. My bad.

Fixed now.

Includes additional unit tests for all relevant functions within the chain, as well as for the sniff for which the issue was reported.

Fixes 3267
…r property promotion

When constructor property promotion is used, the properties declared in the class constructor should never be marked as unused by this sniff.

Includes tests.

Fixes 3269
…Promotion

The sniff already handles this correctly. This just adds some tests to confirm it and safeguard it for the future.
…reports line break as 0 spaces between parenthesis
… promotion

The sniff already handles this correctly. This just adds some tests to confirm it and safeguard it for the future.
This also adds unit tests for findStartOfStatement and fixes some edge cases where passing the last token in an expression return the same token back again
@cdayjr cdayjr merged commit 9161764 into cdayjr:main Mar 27, 2021
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.

10 participants