Skip to content
This repository was archived by the owner on Nov 21, 2019. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1acbda6
Start with PSR-2
thewilkybarkid Aug 24, 2018
7efbe3f
Set up Travis
thewilkybarkid Aug 24, 2018
1979bd0
Enable short open tags
thewilkybarkid Aug 24, 2018
f638519
Not working
thewilkybarkid Aug 24, 2018
b9b3942
Try lowest dependency versions
thewilkybarkid Aug 24, 2018
fee6a87
Try and run separate tasks
thewilkybarkid Aug 25, 2018
1f95bfb
XDebug might not be installed
thewilkybarkid Aug 25, 2018
e431d44
Not sure what happened there
thewilkybarkid Aug 25, 2018
f47dfc9
And again
thewilkybarkid Aug 25, 2018
9a8378d
Check file names
thewilkybarkid Aug 25, 2018
48955b2
Method case
thewilkybarkid Aug 25, 2018
82f73d1
Constant names
thewilkybarkid Aug 25, 2018
3f3180b
Namespaces
thewilkybarkid Aug 25, 2018
350ec47
Namespace/use blank lines
thewilkybarkid Aug 25, 2018
d93b055
Property visibility
thewilkybarkid Aug 25, 2018
44bfd4a
Method visibility
thewilkybarkid Aug 25, 2018
3259c5a
Start trying to make sure code is actually valid
thewilkybarkid Aug 26, 2018
c0d0000
Invalid PHP
thewilkybarkid Aug 28, 2018
cdddeda
Braces
thewilkybarkid Aug 28, 2018
6a3e68b
PHP casing
thewilkybarkid Aug 28, 2018
f03df17
Back to PSR-1
thewilkybarkid Aug 29, 2018
87f919b
More details
thewilkybarkid Aug 29, 2018
1f296ad
Show PHP_CodeSniffer progress
thewilkybarkid Aug 30, 2018
753d1b1
Better name
thewilkybarkid Aug 30, 2018
ca2db28
Use sniff names
thewilkybarkid Aug 30, 2018
5db81cf
Lowest working version for short tags
thewilkybarkid Aug 30, 2018
e557817
Match sniff name
thewilkybarkid Aug 30, 2018
3c893dd
Missing one-per-file tests
thewilkybarkid Aug 30, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/*.* export-ignore
/tests export-ignore
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/.phpcs-cache
/composer.lock
/phpcs.xml
/phpunit.xml
/vendor/
42 changes: 42 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
sudo: false

language: php

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important notice: outsources the maintenance of the PHP environments to Travis CI for the library to be cross-checked across PHP versions


php:
- 7.2
- nightly

before_install:
- phpenv config-rm xdebug.ini || true
- phpenv config-add .travis/php.ini

install:
- travis_retry composer install --classmap-authoritative --no-suggest --prefer-dist

script:
- vendor/bin/phpunit

jobs:
include:

- stage: Test
env: DEPENDENCIES=low
php: 7.2
install:
- travis_retry composer update --classmap-authoritative --no-suggest --prefer-dist --prefer-lowest --prefer-stable

- stage: Code Quality
env: CODING_STANDARDS
script:
- vendor/bin/phpcs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the multi-stage builds 👍 Does it automatically build a matrix of no-environment variables and environment variables?

Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Aug 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normal matrix builds get included in the 'Test' stage.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to fit it in, but this stage could actually run in parallel with the rest of Test?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output of the command seems empty, can it be more verbose when run in CI?


allow_failures:
- php: nightly

cache:
directories:
- $HOME/.composer/cache/files

branches:
only:
- master
1 change: 1 addition & 0 deletions .travis/php.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
short_open_tag = On
29 changes: 29 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "libero/coding-standard",
"description": "Libero PHP coding standard",
"type": "phpcodesniffer-standard",
"license": "MIT",
"autoload-dev": {
"psr-4": {
"tests\\Libero\\CodingStandard\\": "tests/"
}
},
"require": {
"php": "^7.2",
"dealerdirect/phpcodesniffer-composer-installer": "^0.4",
"squizlabs/php_codesniffer": "^3.3.1"
},
"require-dev": {
"lstrojny/functional-php": "^1.8",
"phpunit/phpunit": "^7.3",
"symfony/finder": "^4.1"
},
"config": {
"sort-packages": true
},
"extra": {
"branch-alias": {
"dev-master": "1.0.x-dev"
}
}
}
21 changes: 21 additions & 0 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="UTF-8"?>

<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/squizlabs/php_codesniffer/phpcs.xsd">

<arg name="basepath" value="."/>
<arg name="cache" value=".phpcs-cache"/>
<arg name="colors"/>
<arg name="extensions" value="php"/>
<arg value="s"/>

<rule ref="Libero"/>

<file>src/</file>
<file>tests/</file>

<rule ref="PSR1.Methods.CamelCapsMethodName.NotCamelCaps">
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>

</ruleset>
23 changes: 23 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/7.3/phpunit.xsd" colors="true"
bootstrap="vendor/autoload.php">

<php>
<const name="PHP_CODESNIFFER_CBF" value="true"/>
</php>

<testsuites>
<testsuite name="Libero Coding Standard">
<directory suffix=".php">tests</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory>src</directory>
</whitelist>
</filter>

</phpunit>
10 changes: 10 additions & 0 deletions src/Libero/ruleset.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>

<ruleset name="Libero" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="../../vendor/squizlabs/php_codesniffer/phpcs.xsd">

<description>The Libero coding standard.</description>

<rule ref="PSR2"/>

</ruleset>
123 changes: 123 additions & 0 deletions tests/Tests.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?php

namespace tests\Libero\CodingStandard;

use LogicException;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Files\DummyFile;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Runner;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Finder\Finder;
use function array_combine;
use function array_filter;
use function array_map;
use function explode;
use function Functional\flatten;
use function ini_get;
use function preg_match_all;
use function sort;
use function strpos;

final class Tests extends TestCase

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPUnit convention would push this class name to be SomethingTest?

{
/** @var Runner */
private static $codeSniffer;

public static function setUpBeforeClass()
{
self::$codeSniffer = new Runner();
self::$codeSniffer->config = new Config(['--standard=Libero']);
self::$codeSniffer->init();
}

/**
* @test
* @dataProvider cases
*/
public function it_finds_and_fixes_violations(
string $filename,
string $contents,
string $fixed,
array $messages,
?string $description
) : void {
$file = $this->createFile($filename, $contents);
$actual = flatten($this->getMessages($file));

sort($actual);
sort($messages);

$this->assertSame($messages, $actual, $description);
$this->assertSame($fixed, $file->fixer->getContents());
}

public function cases() : iterable
{
$files = Finder::create()->files()->in(__DIR__.'/cases');

foreach ($files as $file) {
preg_match_all('~(?:---)?([A-Z]+)---\s+([\s\S]+?)\n---~', $file->getContents(), $matches);

$parts = array_combine(array_map('strtolower', $matches[1]), $matches[2]);

if (isset($parts['messages'])) {
$parts['messages'] = array_filter(explode("\n", $parts['messages']));
}

if (empty($parts['contents'])) {
throw new LogicException("Couldn't find contents in {$file->getRelativePathname()}");
} elseif (empty($parts['fixed']) && empty($parts['messages'])) {
throw new LogicException("Expected one of fixed or messages in {$file->getRelativePathname()}");
}

yield $file->getRelativePathname() => [
$parts['filename'] ?? 'test.php',
$parts['contents'],
$parts['fixed'] ?? $parts['contents'],
$parts['messages'] ?? [],
$parts['description'] ?? null,
];
}
}

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');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given these tests should run in a controlled CI environment and are the only ones in the library, $this->fail('...') rather than skip?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More meant for running the tests locally.

}

$file = new DummyFile(
"phpcs_input_file:${filename}\n{$content}",
self::$codeSniffer->ruleset,
self::$codeSniffer->config
);

$file->process();

$file->fixer->fixFile();

$file = new DummyFile(
"phpcs_input_file:${filename}\n{$file->fixer->getContents()}",
self::$codeSniffer->ruleset,
self::$codeSniffer->config
);

$file->process();

return $file;
}

private function getMessages(File $file) : iterable
{
foreach ([$file->getErrors(), $file->getWarnings()] as $messages) {
foreach ($messages as $line => $lineMessages) {
foreach ($lineMessages as $column => $columnMessages) {
foreach ($columnMessages as $data) {
yield "{$line}:{$column} {$data['message']}";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blergh.

}
}
}
}
}
}
6 changes: 6 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

namespace tests\Libero\CodingStandard;

require __DIR__.'/../vendor/autoload.php';
require __DIR__.'/../vendor/squizlabs/php_codesniffer/autoload.php';
16 changes: 16 additions & 0 deletions tests/cases/classes/case
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---DESCRIPTION---
Filename must match class name
---FILENAME---
foobar.php
---CONTENTS---
<?php

namespace Vendor;

class foobar
{
}

---MESSAGES---
5:1 Class name "foobar" is not in PascalCase format
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of something that can't be fixed automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be safer to include the name of the check rather than the text, as it’s more stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

---
16 changes: 16 additions & 0 deletions tests/cases/interfaces/case
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---DESCRIPTION---
Filename must match class name
---FILENAME---
foobar.php
---CONTENTS---
<?php

namespace Vendor;

interface foobar
{
}

---MESSAGES---
5:1 Interface name "foobar" is not in PascalCase format
---
15 changes: 15 additions & 0 deletions tests/cases/php/closing-tag
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---DESCRIPTION---
No closing PHP tags at the end of files
---CONTENTS---
<?php

$foo = 'bar';

?>

---FIXED---
<?php

$foo = 'bar';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of something that can be fixed automatically.

---
10 changes: 10 additions & 0 deletions tests/cases/php/line-length
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---DESCRIPTION---
Lines must not exceed 120 characters
---CONTENTS---
<?php

$foo = '890123456789001234567890012345678900123456789001234567890012345678900123456789001234567890012345678900123456789';

---MESSAGES---
3:121 Line exceeds 120 characters; contains 121 characters
---
13 changes: 13 additions & 0 deletions tests/cases/php/short-tag
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---DESCRIPTION---
PHP short tags are not allowed
---CONTENTS---
<?

$foo = 'bar';

---FIXED---
<?php

$foo = 'bar';

---
15 changes: 15 additions & 0 deletions tests/cases/php/side-effects
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---DESCRIPTION---
No closing PHP tags at the end of files
---CONTENTS---
<?php

$foo = 'bar';

?>

---FIXED---
<?php

$foo = 'bar';

---
16 changes: 16 additions & 0 deletions tests/cases/trait/case
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---DESCRIPTION---
Filename must match class name
---FILENAME---
foobar.php
---CONTENTS---
<?php

namespace Vendor;

trait foobar
{
}

---MESSAGES---
5:1 Trait name "foobar" is not in PascalCase format
---
Loading