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

PHP 8.4 compatibility, switch from Psalm to PHPStan #189

Merged
merged 11 commits into from
Dec 8, 2024
12 changes: 9 additions & 3 deletions .github/workflows/php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@ jobs:
symfony-version: "^5.4"
- php-version: "8.3"
symfony-version: "^5.4"
- php-version: "8.4"
symfony-version: "^5.4"
- php-version: "8.1"
symfony-version: "^6.4"
- php-version: "8.2"
symfony-version: "^6.4"
- php-version: "8.3"
symfony-version: "^6.4"
- php-version: "8.4"
symfony-version: "^6.4"
- php-version: "8.2"
symfony-version: "^7.0"
symfony-version: "^7.2"
- php-version: "8.3"
symfony-version: "^7.0"
symfony-version: "^7.2"
- php-version: "8.4"
symfony-version: "^7.2"

runs-on: ubuntu-latest

Expand Down Expand Up @@ -60,7 +66,7 @@ jobs:
run: ./typoscript-lint

- name: Run type checker
run: ./vendor/bin/psalm
run: ./vendor/bin/phpstan analyse

- name: Run unit tests
run: ./vendor/bin/phpunit --testdox
Expand Down
7 changes: 2 additions & 5 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
"ext-dom": "*"
},
"require-dev": {
"phpunit/phpunit": "^10.5.11",
"phpunit/phpunit": "^10.5.11 || ^11.5.0",
"mikey179/vfsstream": "^1.6.11",
"vimeo/psalm": "^5.22.2",
"phpstan/phpstan": "^2.0.3",
"phpspec/prophecy-phpunit": "^2.0.2"
},
"scripts": {
Expand All @@ -47,9 +47,6 @@
}
},
"autoload-dev": {
"files": [
"vendor/phpunit/phpunit/src/Framework/Assert/Functions.php"
],
"psr-4": {
"Helmich\\TypoScriptLint\\Tests\\Functional\\": "tests/functional",
"Helmich\\TypoScriptLint\\Tests\\Unit\\": "tests/unit"
Expand Down
5 changes: 5 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
parameters:
level: 10
reportUnmatchedIgnoredErrors: false
paths:
- src
7 changes: 6 additions & 1 deletion src/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ class Application extends SymfonyApplication

public function __construct(Container $container)
{
// TODO: Drop these and declare const type once we move to PHP 8.3
assert(is_string(static::APP_NAME));
assert(is_string(static::APP_VERSION));

$this->container = $container;
parent::__construct(static::APP_NAME, static::APP_VERSION);
}
Expand Down Expand Up @@ -69,11 +73,12 @@ public function getVersion(): string
continue;
}

/** @var object{packages: object{name: string, version: string}[]} $data */
$data = json_decode($contents, null, 512, JSON_THROW_ON_ERROR);
$packages = array_values(
array_filter(
$data->packages,
fn(\stdClass $package): bool => $package->name === "helmich/typo3-typoscript-lint"
fn(object $package): bool => $package->name === "helmich/typo3-typoscript-lint"
)
);

Expand Down
4 changes: 4 additions & 0 deletions src/Linter/Configuration/ConfigurationLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* @license MIT
* @package Helmich\TypoScriptLint
* @subpackage Linter\Configuration
*
* @phpstan-import-type LinterConfigurationArray from LinterConfiguration
*/
class ConfigurationLocator
{
Expand Down Expand Up @@ -50,6 +52,7 @@ public function loadConfiguration(
): LinterConfiguration {
$configs = [$this->loader->load('typoscript-lint.dist.yml')];
foreach ($possibleConfigurationFiles as $configurationFile) {
/** @var array<string, mixed> $loadedConfig */
$loadedConfig = $this->loader->load($configurationFile);

// Simple mechanism to detect tslint config files ("ts" as in "TypeScript", not "Typoscript")
Expand All @@ -64,6 +67,7 @@ public function loadConfiguration(

$configuration ??= new LinterConfiguration();

/** @var LinterConfigurationArray $processedConfiguration */
$processedConfiguration = $this->processor->processConfiguration(
$configuration,
$configs
Expand Down
11 changes: 7 additions & 4 deletions src/Linter/Configuration/YamlConfigurationLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* @package Helmich\TypoScriptLint
* @subpackage Linter\Configuration
*
* @psalm-suppress MethodSignatureMismatch
* @property FileLocatorInterface $locator
*/
class YamlConfigurationLoader extends FileLoader
{
Expand Down Expand Up @@ -46,18 +46,21 @@ public function __construct(FileLocatorInterface $locator, YamlParser $yamlParse
* @param mixed $resource The resource
* @param string|null $type The resource type
*
* @return array
* @return array<string, mixed>
*
* @psalm-suppress MethodSignatureMismatch
*/
public function load(mixed $resource, ?string $type = null): array
{
assert(is_string($resource));
try {
/** @var string $path */
$path = $this->locator->locate($resource);
$file = $this->filesystem->openFile($path);

return $this->yamlParser->parse($file->getContents());
/** @var array<string, mixed> $out */
$out = $this->yamlParser->parse($file->getContents());
return $out;
} catch (FileLocatorFileNotFoundException $error) {
return [];
}
Expand All @@ -73,7 +76,7 @@ public function load(mixed $resource, ?string $type = null): array
*
* @psalm-suppress MethodSignatureMismatch
*/
public function supports(mixed $resource, string $type = null): bool
public function supports(mixed $resource, ?string $type = null): bool
{
return is_string($resource)
&& in_array(pathinfo($resource, PATHINFO_EXTENSION), ['yml', 'yaml']);
Expand Down
44 changes: 33 additions & 11 deletions src/Linter/LinterConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,34 @@
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;

/**
* @phpstan-type LinterSniffConfigurationArray array{
* disabled?: bool,
* parameters?: mixed,
* }
* @phpstan-type LinterConfigurationArray array{
* paths?: string[],
* filePatterns?: string[],
* excludePatterns?: string[],
* sniffs: array<string, LinterSniffConfigurationArray>
* }
* @phpstan-type SniffConfigurationArray array{
* class: class-string,
* parameters?: mixed,
* }
*/
class LinterConfiguration implements ConfigurationInterface
{

private array $configuration = [];
/**
* @var LinterConfigurationArray
*/
private array $configuration = ['sniffs' => []];

/**
* @param LinterConfigurationArray $configuration
* @return void
*/
public function setConfiguration(array $configuration): void
{
$this->configuration = $configuration;
Expand Down Expand Up @@ -38,7 +61,7 @@ public function getPaths(): array
*/
public function getFilePatterns(): array
{
return $this->configuration['filePatterns'] ?: [];
return $this->configuration['filePatterns'] ?? [];
}

/**
Expand All @@ -48,11 +71,11 @@ public function getFilePatterns(): array
*/
public function getExcludePatterns(): array
{
return $this->configuration['excludePatterns'] ?: [];
return $this->configuration['excludePatterns'] ?? [];
}

/**
* @return array
* @return SniffConfigurationArray[]
*/
public function getSniffConfigurations(): array
{
Expand All @@ -62,6 +85,7 @@ public function getSniffConfigurations(): array
continue;
}

/** @var class-string $class */
$class = class_exists($class) ? $class : 'Helmich\\TypoScriptLint\\Linter\\Sniff\\' . $class . 'Sniff';

$configuration['class'] = $class;
Expand All @@ -76,24 +100,22 @@ public function getSniffConfigurations(): array
* @return TreeBuilder The tree builder
* @codeCoverageIgnore FU, I'm not going to test this one!
*
* @psalm-suppress TooManyArguments
* @psalm-suppress TooFewArguments
* @psalm-suppress UndefinedMethod
* @psalm-suppress DeprecatedMethod
* @noinspection PhpUndefinedMethodInspection
* @noinspection PhpParamsInspection
*/
public function getConfigTreeBuilder(): TreeBuilder
{
// @phpstan-ignore-next-line
if (method_exists(TreeBuilder::class, 'getRootNode')) {
$treeBuilder = new TreeBuilder('typoscript-lint');
$root = $treeBuilder->getRootNode();
} else {
$treeBuilder = new TreeBuilder();
$root = $treeBuilder->root('typoscript-lint');
$treeBuilder = new TreeBuilder(); // @phpstan-ignore-line
$root = $treeBuilder->root('typoscript-lint'); // @phpstan-ignore-line
}

/** @psalm-suppress PossiblyUndefinedMethod */
// This is a mess; I won't bother teaching PHPStan how to interpret this
// @phpstan-ignore-next-line
$root
->children()
->arrayNode('paths')
Expand Down
10 changes: 8 additions & 2 deletions src/Linter/ReportPrinter/CheckstyleReportPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ public function __construct(OutputInterface $output)
public function writeReport(Report $report): void
{
try {
$xml = $this->buildReportXMLDocument($report);
$this->output->write($xml->saveXML());
$xmlDocument = $this->buildReportXMLDocument($report);
$xmlOutput = $xmlDocument->saveXML();

if ($xmlOutput === false) {
throw new PrinterException('error while rendering XML output');
}

$this->output->write($xmlOutput);
} catch (DOMException $error) {
throw new PrinterException('Could not generate checkstyle report: ' . $error->getMessage(), $error);
}
Expand Down
4 changes: 4 additions & 0 deletions src/Linter/Sniff/AbstractSyntaxTreeSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
abstract class AbstractSyntaxTreeSniff implements SyntaxTreeSniffInterface
{

/**
* @param array<mixed, mixed> $parameters
* @phpstan-ignore constructor.unusedParameter (is defined in interface)
*/
public function __construct(array $parameters)
{
}
Expand Down
7 changes: 7 additions & 0 deletions src/Linter/Sniff/DeadCodeSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class DeadCodeSniff implements TokenStreamSniffInterface

public const ANNOTATION_COMMENT = '/^\s*([a-z0-9]+=(.*?))(;\s*[a-z0-9]+=(.*?))*\s*$/';

/**
* @param array<mixed, mixed> $parameters
* @phpstan-ignore constructor.unusedParameter (defined in interface)
*/
public function __construct(array $parameters)
{
}
Expand All @@ -27,13 +31,16 @@ public function __construct(array $parameters)
*/
public function sniff(array $tokens, File $file, LinterConfiguration $configuration): void
{
assert(is_string(static::ANNOTATION_COMMENT));

foreach ($tokens as $token) {
if (!($token->getType() === TokenInterface::TYPE_COMMENT_ONELINE
|| $token->getType() === TokenInterface::TYPE_COMMENT_MULTILINE)) {
continue;
}

$commentContent = preg_replace(',^\s*(#|/\*|/)\s*,', '', $token->getValue());
assert($commentContent !== null);

if (preg_match(static::ANNOTATION_COMMENT, $commentContent)) {
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/Linter/Sniff/Inspection/TokenInspections.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ trait TokenInspections
*/
private static function isOperator(TokenInterface $token): bool
{
return static::isUnaryOperator($token) || static::isBinaryOperator($token);
return self::isUnaryOperator($token) || self::isBinaryOperator($token);
}

/**
Expand Down Expand Up @@ -75,7 +75,7 @@ private static function isWhitespace(TokenInterface $token): bool
*/
private static function isWhitespaceOfLength(TokenInterface $token, int $length): bool
{
return static::isWhitespace($token) && strlen(trim($token->getValue(), "\n")) == $length;
return self::isWhitespace($token) && strlen(trim($token->getValue(), "\n")) == $length;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Linter/Sniff/NestingConsistencySniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public function __construct(array $parameters)
{
parent::__construct($parameters);

if (array_key_exists('commonPathPrefixThreshold', $parameters)) {
if (array_key_exists('commonPathPrefixThreshold', $parameters) && is_int($parameters['commonPathPrefixThreshold'])) {
$this->commonPathPrefixThreshold = $parameters['commonPathPrefixThreshold'];
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/Linter/Sniff/OperatorWhitespaceSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class OperatorWhitespaceSniff implements TokenStreamSniffInterface
{
use TokenInspections;

/**
* @param array<mixed, mixed> $parameters
* @phpstan-ignore constructor.unusedParameter (defined in interface)
*/
public function __construct(array $parameters)
{
}
Expand Down
3 changes: 3 additions & 0 deletions src/Linter/Sniff/SniffInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@

interface SniffInterface
{
/**
* @param array<mixed, mixed> $parameters
*/
public function __construct(array $parameters);
}
2 changes: 0 additions & 2 deletions src/Linter/Sniff/SniffLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ class SniffLocator
*
* @return SniffInterface[]
* @throws Exception
*
* @psalm-return array<int, SniffInterface>
*/
private function loadSniffs(LinterConfiguration $configuration): array
{
Expand Down
Loading
Loading