Start with PSR-1 #1
Conversation
tests/cases/classes/case
Outdated
| } | ||
|
|
||
| ---MESSAGES--- | ||
| 5:1 Class name "foobar" is not in PascalCase format |
There was a problem hiding this comment.
Example of something that can't be fixed automatically.
There was a problem hiding this comment.
Might be safer to include the name of the check rather than the text, as it’s more stable.
There was a problem hiding this comment.
Preference @giorgiosironi @stephenwf? Currently have to require "squizlabs/php_codesniffer": "^3.3.1" as one of the wordings changed (it reads Squiz.Classes.ValidClassName.NotCamelCaps if using the name of the check).
There was a problem hiding this comment.
I agree the class name in the context of this text should be closer to a published API. The user would still see the human message when running the tool or in other CI builds
There was a problem hiding this comment.
It may also output multiple messages for the same line with a non-deterministic order, but the current test cases don't go into that so not needed to cater for it.
There was a problem hiding this comment.
The class name may lose some detail about what the object being checked is (Foo class, bar method) but the line numbers make that clear anyway.
tests/cases/php/closing-tag
Outdated
| <?php | ||
|
|
||
| $foo = 'bar'; | ||
|
|
There was a problem hiding this comment.
Example of something that can be fixed automatically.
tests/Tests.php
Outdated
| foreach ($messages as $line => $lineMessages) { | ||
| foreach ($lineMessages as $column => $columnMessages) { | ||
| foreach ($columnMessages as $data) { | ||
| yield "{$line}:{$column} {$data['message']}"; |
|
Interesting testing suite 👍 like it |
.travis.yml
Outdated
| - stage: Code Quality | ||
| env: CODING_STANDARDS | ||
| script: | ||
| - vendor/bin/phpcs |
There was a problem hiding this comment.
Like the multi-stage builds 👍 Does it automatically build a matrix of no-environment variables and environment variables?
There was a problem hiding this comment.
The normal matrix builds get included in the 'Test' stage.
There was a problem hiding this comment.
Not sure how to fit it in, but this stage could actually run in parallel with the rest of Test?
There was a problem hiding this comment.
Output of the command seems empty, can it be more verbose when run in CI?
tests/cases/interfaces/method-case
Outdated
| interface Foo | ||
| { | ||
| public function bar_baz() : void | ||
| { |
There was a problem hiding this comment.
Turns out this error isn't found by token_get_all() (see 3259c5a).
|
Will roll this back to PSR-1 so can merge, while the test cases for PSR-2 are finished. |
| @@ -0,0 +1,42 @@ | |||
| sudo: false | |||
|
|
|||
| language: php | |||
There was a problem hiding this comment.
important notice: outsources the maintenance of the PHP environments to Travis CI for the library to be cross-checked across PHP versions
tests/Tests.php
Outdated
| use function token_get_all; | ||
| use const TOKEN_PARSE; | ||
|
|
||
| final class Tests extends TestCase |
There was a problem hiding this comment.
PHPUnit convention would push this class name to be SomethingTest?
tests/Tests.php
Outdated
| private function createFile(string $filename, string $content) : File | ||
| { | ||
| if (!ini_get('short_open_tag') && false === strpos($content, '<?php')) { | ||
| $this->markTestSkipped('short_open_tag option is disabled'); |
There was a problem hiding this comment.
given these tests should run in a controlled CI environment and are the only ones in the library, $this->fail('...') rather than skip?
There was a problem hiding this comment.
More meant for running the tests locally.
| } | ||
|
|
||
| ---MESSAGES--- | ||
| 9:1 PSR1.Classes.ClassDeclaration.MultipleClasses |
There was a problem hiding this comment.
Doesn't trigger Squiz.Classes.ClassFileName.NoMatch because of squizlabs/PHP_CodeSniffer#2140 (and why there's not yet a file-name test for traits).
No description provided.