cucumber / gherkin

A parser and compiler for the Gherkin language.
MIT License
160 stars 48 forks source link

[php] Configure Psalm 6 default settings findUnusedBaselineEntry, findUnusedCode #234

Closed olleolleolle closed 3 months ago

olleolleolle commented 3 months ago

🤔 What's changed?

Psalm's offering news, and our CI had not configured those settings.

This PR fixes a warning that outputs that we should be explicit about this setting.

First, I picked enabling it as the solution. (And, then I continued on that path.)

Psalm in CI: found some issues: 9 errors found ``` ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ Error: src/AstNode.php:38:37: UnusedParam: Param #1 is never referenced in this method (see https://psalm.dev/135) Error: src/GherkinParser.php:31:31: UnusedProperty: Cannot find any references to private property Cucumber\Gherkin\GherkinParser::$predictableIds (see https://psalm.dev/150) Error: src/GherkinParser.php:46:21: PossiblyUnusedMethod: Cannot find any calls to method Cucumber\Gherkin\GherkinParser::parseString (see https://psalm.dev/087) Error: src/Location.php:15:21: PossiblyUnusedMethod: Cannot find any calls to method Cucumber\Gherkin\Location::getColumn (see https://psalm.dev/087) Error: src/Parser/ParserTrait.php:76:16: UnusedReturnValue: The return value for this private method is never used (see https://psalm.dev/272) Error: src/ParserException/UnexpectedEofException.php:16:31: PossiblyUnusedProperty: Cannot find any references to property Cucumber\Gherkin\ParserException\UnexpectedEofException::$receivedToken (see https://psalm.dev/149) Error: src/ParserException/UnexpectedEofException.php:17:31: PossiblyUnusedProperty: Cannot find any references to property Cucumber\Gherkin\ParserException\UnexpectedEofException::$expectedTokenTypes (see https://psalm.dev/149) Error: src/ParserException/UnexpectedEofException.php:18:32: PossiblyUnusedProperty: Cannot find any references to property Cucumber\Gherkin\ParserException\UnexpectedEofException::$stateComment (see https://psalm.dev/149) Error: src/ParserException/UnexpectedTokenException.php:19:32: PossiblyUnusedProperty: Cannot find any references to property Cucumber\Gherkin\ParserException\UnexpectedTokenException::$stateComment (see https://psalm.dev/149) ------------------------------ 9 errors found ------------------------------ Psalm can automatically fix 3 of these issues. Run Psalm again with --alter --issues=PossiblyUnusedMethod,MissingParamType --dry-run to see what it can fix. ------------------------------ Checks took 3.46 seconds and used 110.339MB of memory Psalm was able to infer types for 100% of the codebase Error: Process completed with exit code 2. ```

OK, so perhaps "enable" wasn't the right way, let's see if disabling will lead to nicer output? I learned a little, and placed some Psalm annotations in the code, in order to tell Psalm "these properties on the exceptions, they're expected, across all our implementations".

That got us a shorter list of Psalm offenses.

Psalm in CI: 5 errors found ``` ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ Error: src/AstNode.php:38:37: UnusedParam: Param #1 is never referenced in this method (see https://psalm.dev/135) Error: src/GherkinParser.php:31:31: UnusedProperty: Cannot find any references to private property Cucumber\Gherkin\GherkinParser::$predictableIds (see https://psalm.dev/150) Error: src/GherkinParser.php:46:21: PossiblyUnusedMethod: Cannot find any calls to method Cucumber\Gherkin\GherkinParser::parseString (see https://psalm.dev/087) Error: src/Location.php:15:21: PossiblyUnusedMethod: Cannot find any calls to method Cucumber\Gherkin\Location::getColumn (see https://psalm.dev/087) Error: src/Parser/ParserTrait.php:76:16: UnusedReturnValue: The return value for this private method is never used (see https://psalm.dev/272) ------------------------------ 5 errors found ------------------------------ ```

Upon thinking about those issues reported, I sort of think that it can absolutely be that it's part of the API to have some unused parameters (in code like AST handling callbacks). So, perhaps they're all @psalm-api.

⚡️ What's your motivation?

CI output should be terse, clear and focused. This change hopes to make it less noisy.

🏷️ What kind of change is this?

olleolleolle commented 3 months ago

There!