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

Introduced NoConfigResolverParametersInConstructorRule PHPStan custom rule #1

Merged
merged 2 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
71 changes: 71 additions & 0 deletions .github/workflows/backend-ci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Backend build

on:
push:
branches:
- main
- '[0-9]+.[0-9]+'
pull_request: ~

jobs:
cs-fix:
name: Run code style check
runs-on: "ubuntu-22.04"
strategy:
matrix:
php:
- '8.1'
konradoboza marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/checkout@v4

- name: Setup PHP Action
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
coverage: none
extensions: 'pdo_sqlite, gd'
tools: cs2pr

- uses: ramsey/composer-install@v3
with:
dependency-versions: "highest"

- name: Run code style check
run: composer run-script check-cs -- --format=checkstyle | cs2pr

tests:
name: Tests
runs-on: "ubuntu-22.04"
timeout-minutes: 10

strategy:
fail-fast: false
matrix:
php:
- '7.4'
- '8.3'

steps:
- uses: actions/checkout@v4

- name: Setup PHP Action
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
coverage: none
extensions: pdo_sqlite, gd
tools: cs2pr

- uses: ramsey/composer-install@v3
with:
dependency-versions: "highest"
composer-options: "--prefer-dist --no-progress --no-suggest"

- name: Setup problem matchers for PHPUnit
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"

- name: Run PHPStan analysis
run: composer run-script phpstan

- name: Run test suite
run: composer run-script --timeout=600 test
14 changes: 14 additions & 0 deletions .php-cs-fixer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

return \Ibexa\CodeStyle\PhpCsFixer\InternalConfigFactory::build()->setFinder(
PhpCsFixer\Finder::create()
->in(__DIR__ . '/rules')
->in(__DIR__ . '/tests')
->files()->name('*.php')
);
69 changes: 51 additions & 18 deletions composer.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,54 @@
{
"name": "ibexa/phpstan",
"license": "proprietary",
"type": "ibexa-bundle",
"keywords": [
"ibexa-dxp"
],
"require": {
"php": "^7.4 || ^8.0"
},
"config": {
"sort-packages": true
},
"extra": {
"phpstan": {
"includes": [
"extension.neon"
]
"name": "ibexa/phpstan",
"license": "proprietary",
"type": "ibexa-bundle",
"keywords": [
"ibexa-dxp"
],
"require": {
"php": "^7.4 || ^8.0",
"ibexa/core": "4.6.x-dev",
konradoboza marked this conversation as resolved.
Show resolved Hide resolved
"ibexa/doctrine-schema": "4.6.x-dev"
},
"require-dev": {
"ibexa/code-style": "~2.0.0",
"phpstan/phpstan": "^1.10",
"phpstan/phpstan-phpunit": "^1.3",
"phpstan/phpstan-strict-rules": "^1.6",
"phpstan/phpstan-symfony": "^1.3",
"phpunit/phpunit": "^9"
},
"autoload": {
"psr-4": {
"Ibexa\\PHPStan\\Rules\\": "rules/"
}
},
"autoload-dev": {
"psr-4": {
"Ibexa\\Tests\\PHPStan\\Rules\\": "tests/rules/"
}
},
"scripts": {
"fix-cs": "php-cs-fixer fix --config=.php-cs-fixer.php --show-progress=dots",
"check-cs": "@fix-cs --dry-run",
"test": "phpunit -c phpunit.xml.dist",
"phpstan": "phpstan analyse -c phpstan.neon"
},
"scripts-descriptions": {
"fix-cs": "Automatically fixes code style in all files",
"check-cs": "Run code style checker for all files",
"test": "Run automatic tests",
"phpstan": "Run static code analysis"
},
"config": {
"sort-packages": true,
"allow-plugins": false
},
"extra": {
"phpstan": {
"includes": [
"extension.neon"
]
}
}
}
}
2 changes: 2 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ parameters:
- stubs/Money/Money.stub
- stubs/Money/MoneyFormatter.stub
- stubs/Money/MoneyParser.stub
rules:
- Ibexa\PHPStan\Rules\NoConfigResolverParametersInConstructorRule
6 changes: 6 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
parameters:
level: 8
paths:
- rules
- tests
checkMissingCallableSignature: true
12 changes: 12 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<phpunit
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
bootstrap="vendor/autoload.php"
failOnWarning="true"
colors="true">
<testsuites>
<testsuite name="rules">
<directory>tests/rules</directory>
</testsuite>
</testsuites>
</phpunit>
68 changes: 68 additions & 0 deletions rules/NoConfigResolverParametersInConstructorRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\PHPStan\Rules;

use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface;
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;

/**
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\MethodCall>
*/
final class NoConfigResolverParametersInConstructorRule implements Rule
{
public function getNodeType(): string
{
return Node\Expr\MethodCall::class;
}

/**
* @throws \PHPStan\ShouldNotHappenException
*/
public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Node\Identifier) {
return [];
}

$function = $scope->getFunction();
if ($function !== null && $function->getName() !== '__construct') {
return [];
}

/** @var \PhpParser\Node\Identifier $nodeName */
$nodeName = $node->name;
$methodName = $nodeName->name;

if (
$methodName !== 'getParameter'
&& $methodName !== 'hasParameter'
&& !isset($node->getArgs()[0])
) {
return [];
}

$type = $scope->getType($node->var);
$configResolverInterfaceType = new ObjectType(ConfigResolverInterface::class);
if (!$configResolverInterfaceType->isSuperTypeOf($type)->yes()) {
return [];
}

return [
RuleErrorBuilder
::message('Referring to ConfigResolver parameters in constructor is not allowed due to potential scope change.')
->identifier('Ibexa.NoConfigResolverParametersInConstructor')
->nonIgnorable()
konradoboza marked this conversation as resolved.
Show resolved Hide resolved
->build(),
];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Tests\PHPStan\Rules\Fixtures;

use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface;

final class NoConfigResolverParametersInConstructorFixture
{
private ConfigResolverInterface $configResolver;

public function __construct(ConfigResolverInterface $configResolver)
{
$this->configResolver = $configResolver;

$configResolver->hasParameter('foo');
$configResolver->getParameter('foo');
}

public function foo(): void
{
//this is allowed outside of constructor - no error reported by PHPStan
$this->configResolver->hasParameter('bar');
}
}
43 changes: 43 additions & 0 deletions tests/rules/NoConfigResolverParametersInConstructorRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Tests\PHPStan\Rules;

use Ibexa\PHPStan\Rules\NoConfigResolverParametersInConstructorRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends \PHPStan\Testing\RuleTestCase<\Ibexa\PHPStan\Rules\NoConfigResolverParametersInConstructorRule>
*/
final class NoConfigResolverParametersInConstructorRuleTest extends RuleTestCase
{
protected function getRule(): Rule
{
return new NoConfigResolverParametersInConstructorRule();
}

public function testRule(): void
{
$this->analyse(
[
__DIR__ . '/Fixtures/NoConfigResolverParametersInConstructorFixture.php',
],
[
[
'Referring to ConfigResolver parameters in constructor is not allowed due to potential scope change.',
21,
],
[
'Referring to ConfigResolver parameters in constructor is not allowed due to potential scope change.',
22,
],
]
);
}
}
Loading