-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
New Universal.DeclareStatements.DeclareStatementsStyle
sniff
#129
base: develop
Are you sure you want to change the base?
New Universal.DeclareStatements.DeclareStatementsStyle
sniff
#129
Conversation
596eaf1
to
6511b3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed during pair programming session in which the review comments were discussed (which is why they are sometimes a bit shorthand).
Additional remarks/things to think over:
- If there is a
declare
statement with multiple directives and those directive have different requirements, i.e,strict_types
: disallow andticks
set torequire
: how should this be handled by the sniff ?
For potentially adding fixers - helpful link: https://pear.php.net/package/PHP_CodeSniffer/docs/3.5.4/apidoc/PHP_CodeSniffer/Fixer.html
General remark: in the sniffs, everything which gets a docblock ought to get a @since
tag in this repo (can't be enforced yet as there's no good sniff for it, but will be enforced in the future).
For the tests, just the @since
at class level is fine.
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Tests/DeclareStatements/DeclareStatementsStyleUnitTest.inc
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Universal/Sniffs/DeclareStatements/DeclareStatementsStyleSniff.php
Outdated
Show resolved
Hide resolved
Some ideas we discussed about adding metrics:
|
I've reworked the sniff and tests, but still need to work on the fixer and metrics. Also, I expect there will be some remarks on the PR, those are definitely welcomed 🙂 |
FYI: I'm moving this PR out of the |
516e6db
to
a775e65
Compare
@jrfnl I've added some metrics, but I'm unsure if this is correct. When I run the report for metrics, I get correct percentages per file (all adds up to 100%) but I'm not sure if I'm doing it right, as in one file (3rd test file) I have multiple block directives. Some are ok, some are not, and the 2 are ok, but for some reason they are being counted as separate directives, and the type isn't correctly recognized: vendor/bin/phpcs --standard=universal --sniffs=universal.declarestatements.blockmode Universal/Tests/DeclareStatements/BlockModeUnitTest.3.inc -sp --report=info
E 1 / 1 (100%)
PHP CODE SNIFFER INFORMATION REPORT
----------------------------------------------------------------------
Declare directive scope: Block [10/10, 100%]
Declare directive type: encoding [4/4, 100%]
----------------------------------------------------------------------
Time: 12.06 secs; Memory: 6MB When debugging, the value is different (first I wanted to group the types, and they seem to be correctly recognized when they are on their own. I still need to check if there are any fixable errors. The only fixable case I can think of could be if there is only one directive in the Edit:I tried to add the fixer, but I got stuck, as there are tons of whitespace edge cases that can trip me up. I did save my WIP for the fixer in another branch so I may play with it later, but for now I'm not sure it's worth the time spent 🤷🏼♂️ |
To do: add fixers and metric
Will probably need some fixup.
aac4f3a
to
5834c64
Compare
New sniff to detect using `static` instead of `self` in OO constructs which are `final`. Includes fixer. Includes unit tests. Includes documentation.
New sniff to enforce no spaces around union type and intersection type separator operators. Includes fixer. Includes unit tests. Includes documentation. Includes metrics. Co-authored-by: Denis Žoljom <[email protected]>
… array opener ... when a new line is required after the opener of a multi-line array. This sniff should not have an opinion on whether or not trailing comments are allowed. There are other sniffs around to forbid those, if so desired. Includes extra unit tests to safeguard this change and to safeguard handling of comments after the array opener regardless of this change.
New sniff to enforce indentation to always be a multiple of a tabstop, i.e. disallow precision alignment. For space-based standards, the `Generic.WhiteSpace.DisallowTabIndent` sniff (unintentionally/incorrectly) already removes precision alignment. For tab-based standards, the `Generic.WhiteSpace.DisallowSpaceIndent` sniff, however, does not remove precision alignment. With that in mind, this sniff is mostly intended for standards which demand tabs for indentation. In rare cases, spaces for precision alignment can be intentional and acceptable, but more often than not, precision alignment is a typo. Notes: * This sniff does not concern itself with tabs versus spaces. Use the PHPCS native `Generic.WhiteSpace.DisallowTabIndent` or the `Generic.WhiteSpace.DisallowSpaceIndent` sniffs for that. * When using this sniff with tab-based standards, please ensure that the `tab-width` is set and either don't set the `$indent` property or set it to the tab-width (or a multiple thereof). * Precision alignment *within* text strings (multi-line text strings, heredocs, nowdocs) will NOT be flagged by this sniff. * Precision alignment in (trailing) whitespace on otherwise blank lines will not be flagged by default. Most standards will automatically remove trailing whitespace anyway using the `Squiz.WhiteSpace.SuperfluousWhitespace` sniff. If the Squiz sniff is not used, set the `ignoreBlankLines` property to `false` to enable reporting for this. * The sniff also supports an option to ignore precision alignment in specific situations, like for multi-line chained method calls. **Note**: the fixer works based on "best guess" and may not always result in the desired indentation. Combine this sniff with the `Generic.WhiteSpace.ScopeIndent` sniff for more precise indentation fixes. If so desired, the fixer can be turned off by including the rule in a standard like so: `<rule phpcs-only="true" ref="Universal.WhiteSpace.PrecisionAlignment"/>` As it is, the sniff supports both PHP, as well as JS and CSS, though the support for JS and CSS should be considered incidental and will be removed once PHPCS 4.0 will be released. Includes fixer. Includes unit tests. Includes documentation.
Checks the amount of spacing between the `class` keyword and the open parenthesis (if any) for anonymous class declarations. The expected amount of spacing is configurable and defaults to `0`. The default value of `0` has been decided upon based on scanning a not insignificant number of codebases and determining the prevailing style used for anonymous classes. It is also in line with the previously proposed [errata for PSR12](php-fig/fig-standards#1206). Includes fixer. Includes unit tests. Includes documentation. Includes metrics.
... to be more descriptive.
This file is not supposed to yield errors/warnings, so the `.fixed` file is redundant.
... now upstream PR 3639 has been merged. Ref: * squizlabs/PHP_CodeSniffer 3639
…tion ... as long as the arrow function is within an OO construct which can be `final`. Includes tests. Includes enhancing the closure test a little as well.
PHPCSDevCS was previously not added to the `require-dev` dependencies as the minimum supported PHPCS version conflicted with the minimum supported PHPCS version of this package. Now this is no longer the case, the package can be safely added to `require-dev` and work-arounds can be removed.
These steps/directives all relate to testing against PHPCS 4.x, but this package is not being tested against PHPCS 4.x at this time and when this will be enabled again, these work-arounds may well no longer be needed anymore anyway, so let's remove it for now and re-evaluate what is needed when testing against PHPCS 4.x is re-enabled.
* Update the PHP version on which the CS run for this package is run to PHP `latest`. * Remove the `continue-on-error` for PHP 8.2 in the `test` workflow (and remove the `experimental` key, which is now no longer used. * Update the "Tested against" badge in the README.
* Update links to workflows. * Use re-usable links for links which are used repeatedly.
... to be before the adjustments made to the composer.json file.
_Based on a similar workflow previously added to PHPCSUtils._ Remark offers a number of "rules" which are useful, such as checking that links and link definitions match up, verifying all used links work and some formatting checks. The rule configuration is contained in the .remarkrc file. Files/directories to be ignored can be listed in the .remarkignore file which supports glob syntax, like .gitignore. Note: .-prefixed files and directories are excluded by default. To include those in the scan, they have to be passed explicitly on the command-line. To run locally, follow the same steps as per the GitHub actions workflow. Note: the workflow contains a double-run of remark-lint to allow viewing the results both in a human readable way in the actions transscripts, as well as getting annotations for found issue in PRs. Refs: * https://github.com/remarkjs/remark/tree/main/packages/remark-cli * https://github.com/remarkjs/remark-lint * https://github.com/remarkjs/remark-gfm * https://github.com/remarkjs/remark-lint/tree/main/packages/remark-preset-lint-consistent * https://github.com/remarkjs/remark-lint/tree/main/packages/remark-preset-lint-recommended * https://github.com/remarkjs/remark-lint/tree/main/packages/remark-preset-lint-markdown-style-guide Additional (external) plugins included: * https://github.com/vhf/remark-lint-heading-whitespace * https://github.com/wemake-services/remark-lint-list-item-punctuation * https://github.com/laysent/remark-lint-plugins/tree/HEAD/packages/remark-lint-match-punctuation * https://github.com/olizilla/remark-lint-no-hr-after-heading * https://github.com/wemake-services/remark-lint-are-links-valid * https://github.com/remarkjs/remark-validate-links
... to loosely safeguard valid and consistent yaml files. Includes adding a configuration file which loosens up the default ruleset to a degree I'm comfortable with. Ref: https://yamllint.readthedocs.io/
Includes a few updated unit tests to safeguard the fix.
... of the bash `date` command in the earlier pulled cache busting.
PHP 8.2 has been released today 🎉 and the `setup-php` action has announced support for PHP 8.3, so adding PHP 8.3 to the matrix and no longer allowing PHP 8.2 to fail the build. Note: PHPCS does not (yet) have full syntax support for PHP 8.2, but it does have runtime support (for the most part, see squizlabs/PHP_CodeSniffer 3629). Builds against PHP 8.3 are still allowed to fail for now.
Follow up on 186 which added some new configuration files, but didn't export ignore them. Includes aligning the values for better readability.
To do: add fixers and metric
Will probably need some fixup.
…s' into declare-statements-curly-brackets
This sniff will check the
declare
statements for a specific coding style.By default, the style of the
declare
statement is without braces and will be applied for all the directives (strict_types
,ticks
,encoding
).The sniff can be configured to require or disallow braces (except for the
strict_types
), and selectively select for which directive the selected style should be enforced (ticks
,encoding
).