Skip to content

Commit

Permalink
Initial commit
Browse files Browse the repository at this point in the history
  • Loading branch information
DaveLiddament committed Dec 29, 2022
0 parents commit d7ea4a5
Show file tree
Hide file tree
Showing 28 changed files with 923 additions and 0 deletions.
61 changes: 61 additions & 0 deletions .github/workflows/full-checks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: "Full checks"

on:
schedule:
- cron: '0 10 * * *'
pull_request:
push:
branches:
- main

jobs:
full-checks:
name: "Full CI checks for all supported PHP versions"

runs-on: ${{ matrix.operating-system }}

strategy:
fail-fast: false
matrix:
dependencies:
- "lowest"
- "highest"
php-version:
- "8.0"
- "8.1"
- "8.2"
operating-system:
- "ubuntu-22.04"

steps:
- name: "Checkout"
uses: "actions/checkout@v2"

- name: "Install PHP"
uses: "shivammathur/setup-php@v2"
with:
coverage: Xdebug
php-version: "${{ matrix.php-version }}"
tools: composer

- name: Get composer cache directory
id: composercache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"

- name: Cache dependencies
uses: actions/cache@v2
with:
path: ${{ steps.composercache.outputs.dir }}
key: "php-${{ matrix.php-version }}-${{ matrix.dependencies }}-${{ hashFiles('**/composer.lock') }}"
restore-keys: "php-${{ matrix.php-version }}-${{ matrix.dependencies }}-${{ hashFiles('**/composer.lock') }}"

- name: "Install lowest dependencies"
if: ${{ matrix.dependencies == 'lowest' }}
run: "composer update --prefer-lowest --no-interaction --no-progress"

- name: "Install highest dependencies"
if: ${{ matrix.dependencies == 'highest' }}
run: "composer update --no-interaction --no-progress"

- name: "Full CI"
run: "composer ci"
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.vagrant
Vargrantfile
vendor/
.idea/
.phpunit.cache/
.php-cs-fixer.cache
29 changes: 29 additions & 0 deletions .php-cs-fixer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

$finder = PhpCsFixer\Finder::create()
->exclude('tests/Rules/data')
->in(__DIR__);

$config = new PhpCsFixer\Config();
return $config
->setRiskyAllowed(true)
->setRules(
[
'@PSR1' => true,
'@PSR2' => true,
'@PSR12' => true,
'@Symfony' => true,
'@Symfony:risky' => true,
'array_syntax' => ['syntax' => 'short'],
'no_useless_else' => true,
'no_useless_return' => true,
'ordered_imports' => true,
'phpdoc_order' => true,
'strict_comparison' => true,
'phpdoc_align' => false,
'phpdoc_to_comment' => false,
'native_function_invocation' => false,
]
)
->setFinder($finder)
;
9 changes: 9 additions & 0 deletions LICENSE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# The MIT License (MIT)

Copyright (c) 2022 Dave Liddament

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
209 changes: 209 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
# PHPStan rule testing helper

This is a helper library for slight improvement to DX for testing PHPStan rules.
It allows you to write the expected error message in the fixture file.
Anything after `// ERROR ` is considered the expected error message.
The test classes are simplified as you now specify just the fixture files, and this library will extract the expected error and calculate the correct line number.

You can also use an `ErrorMessageFormatter` to further decouple tests from the actual error message.
See [ErrorMessageFormatter](#error-formatter) section.

## Example

Test code extends [AbstractRuleTestCase](src/AbstractRuleTestCase.php).
As with the PHPStan's `RuleTestCase` use the `getRule` method to setup the rule used by the test.
For each test list the fixture file(s) needed by the test, using the `assertIssuesReported` method.

#### Test code:
```php
use DaveLiddament\PhpstanRuleTestingHelper\AbstractRuleTestCase;

class CallableFromRuleTest extends AbstractRuleTestCase
{
protected function getRule(): Rule
{
return new CallableFromRule($this->createReflectionProvider());
}

public function testAllowedCall(): void
{
$this->assertIssuesReported(__DIR__ . '/Fixtures/SomeCode.php');
}
}
```

The fixture file contains the expected error message.
#### Fixture:

```php
class SomeCode
{
public function go(): void
{
$item = new Item("hello");
$item->updateName("world"); // ERROR Can not call method
}
}
```

Every line that contains `// ERROR ` is considered an issue that should be picked up by the rule.
The text after `// ERROR` is the expected error message.

With this approach you don't need to work out the line number of the error.
There are further benefits by using the [ErrorMessageFormatter](#error-formatter) to decouple the error messages from the test code.



NOTE: You can pass in multiple fixture files. E.g.
```php
$this->assertIssuesReported(
__DIR__ . '/Fixtures/SomeCode.php',
__DIR__ . '/Fixtures/SomeCode2.php',
// And so on...
);
```


## Installation

```shell
composer require --dev dave-liddament/phpstan-rule-test-helper
```

## Error Formatter

The chances are when you developing PHPStan rules the error message for violations will change.
Making any change will require you to update all the related tests.

### Constant string error messages

In the simplest case the error is a message that does provide any context, other than line number.
E.g. in the example the error is `Can not call method`. No additional information (e.g. who was trying to call the method) is provided.

Create a class that extends `ConstantErrorFormatter` and pass the error message to the constructor.

```php
class CallableFromRuleErrorFormatter extends ConstantStringErrorMessageFormatter
{
public function __construct()
{
parent::__construct('Can not call method');
}
}
```

The next step is to update the test to tell it to use this formatter.

```php
class CallableFromRuleTest extends AbstractRuleTestCase
{
// getRule method omitted for brevity
// testAllowedCall method omitted for brevity

protected function getErrorFormatter(): ErrorMessageFormatter
{
return new CallableFromRuleErrorFormatter();
}
}
```

Now if the error message is changed, the text only needs to be updated in one place.

Finally, the fixture can be simplified.
There is no need to specify the error message in the fixture file, we just need to specify where the error is.

Updated fixture:
```php
class SomeCode
{
public function go(): void
{
$item = new Item("hello");
$item->updateName("world"); // ERROR
}
}
```

### Error messages with context

Good error message will provide context.
For example, the error message could be improved to give the name of the calling class.
The calling class is `SomeClass` so let's update the error message to `Can not call method from SomeCode`.

The fixture is updated to include the calling class name after `// ERROR`

```php
class SomeCode
{
public function go(): void
{
$item = new Item("hello");
$item->updateName("world"); // ERROR SomeCode
}
}
```

The `CallableFromRuleErrorFormatter` is updated.
Firstly it now extends `ErrorMessageFormatter` instead of `ConstantErrorFormatter`.
An implementation of `getErrorMessage` is added.
This is passed everything after `\\ ERROR`, with whitespace trimmed from each side, and must return the expected error message.

```php
class CallableFromRuleErrorFormatter extends ErrorMessageFormatter
{
public function getErrorMessage(string $errorContext): string
{
return 'Can not call method from ' . $errorContext;
}
}
```

### Error message helper methods

Sometimes the contextual error messages might have 2 or more pieces of information.
Continuing the example above, the error message could be improved to give the name of the calling class and the method being called.
E.g. `Can not call Item::updateName from SomeCode`.

The fixture is updated to include both `Item::updateName` and `SomeCode` seperated by the `|` character.

E.g. `// ERROR`

```php
class SomeCode
{
public function go(): void
{
$item = new Item("hello");
$item->updateName("world"); // ERROR Item::updateName|SomeCode
}
}
```

Use the `getErrorMessageAsParts` helper method to do this, as shown below:

```php

class CallableFromRuleErrorFormatter extends ErrorMessageFormatter
{
public function getErrorMessage(string $errorContext): string
{
$parts = $this->getErrorMessageAsParts($errorContext, 2);
return sprintf('Can not call %s from %s', $parts[0], $parts[1]);
}
}
```

The signature of `getErrorMessageAsParts` is:

```php
/**
* @return list<string>
*/
protected function getErrorMessageAsParts(
string $errorContext,
int $expectedNumberOfParts,
string $separator = '|',
): array
```

If you use the `getErrorMessageAsParts` and the number of parts is not as expected, the test will error with a message that tells you file and line number of the invalid error.
49 changes: 49 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"name": "dave-liddament/phpstan-rule-test-helper",
"description": "Library to help make testing of PHPStan rules easier",
"type": "library",
"require": {
"php": ">=8.0 <8.3",
"phpstan/phpstan": "^1.6"
},
"require-dev": {
"phpunit/phpunit": "^9.0",
"friendsofphp/php-cs-fixer": "^3.7",
"php-parallel-lint/php-parallel-lint": "^1.3.2"
},
"license": "MIT",
"autoload": {
"psr-4": {
"DaveLiddament\\PhpstanRuleTestHelper\\": "src/"
}
},
"autoload-dev": {
"psr-4": {
"DaveLiddament\\PhpstanRuleTestHelper\\Tests\\": "tests/"
}
},
"authors": [
{
"name": "Dave Liddament",
"email": "[email protected]"
}
],
"scripts": {
"composer-validate": "@composer validate --no-check-all --strict",
"cs-fix": "php-cs-fixer fix",
"cs": [
"@putenv PHP_CS_FIXER_IGNORE_ENV=1",
"php-cs-fixer fix --dry-run -v"
],
"analyse": "phpstan analyse",
"lint": "parallel-lint src tests",
"test": "phpunit",
"ci": [
"@composer-validate",
"@lint",
"@cs",
"@test",
"@analyse"
]
}
}
10 changes: 10 additions & 0 deletions fixtures/InvalidErrorMessage.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

class SomeCode
{
public function go(): void
{
$item = new Item('hello');
$item->updateName('world'); // ERROR Item
}
}
15 changes: 15 additions & 0 deletions fixtures/ValidFileWith2ExpectedIssues.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

class SomeCode
{
public function go(): void
{
$item = new Item('hello');
$item->updateName('world'); // ERROR Item::updateName|SomeCode
}

public function anotherMethod(Item $item): void
{
$item->updateName('world'); // ERROR Item::anotherMethod|SomeCode
}
}
Loading

0 comments on commit d7ea4a5

Please sign in to comment.