diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index ed27975..e1abeef 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -42,3 +42,13 @@ jobs: - name: Tests run: | vendor/bin/phpunit + + # The SA-overlap gate is matrix-independent — it asks whether PHPStan and + # Psalm both flag a fixture, which doesn't change across PHP versions or + # composer flags. Run it once on the canonical (latest stable + current + # deps) config. On PHP 8.4 + --prefer-lowest, ancient Psalm transitive + # deps emit deprecation warnings that corrupt the JSON output anyway. + - name: SA-overlap gate (fixtures flagged by both PHPStan and Psalm) + if: matrix.php == '8.4' && matrix.composer_flags == '' + run: | + tests/check-sa-overlap.sh diff --git a/Eventjet/ruleset.xml b/Eventjet/ruleset.xml index d8607a3..920d2f9 100644 --- a/Eventjet/ruleset.xml +++ b/Eventjet/ruleset.xml @@ -297,6 +297,10 @@ - - + + + 0 + diff --git a/README.md b/README.md index e73a978..e38ffd3 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,16 @@ # Eventjet Coding Standard +## Scope: formatting, not linting + +This package historically included rules that overlap with what static +analyzers (PHPStan, Psalm) already catch. Going forward we will not add such +rules, and we will progressively disable existing ones — if PHPStan or Psalm +flag a pattern, a CS rule for it is duplicate work. + +Treat this package as a formatter/style standard only. To get the full +benefit, run a static analyzer alongside it; PHP-CS-Fixer and PHPCS are not a +substitute for PHPStan or Psalm. + ## PHP-CS-Fixer ### Basic Usage: Add the following `.php-cs-fixer.dist.php` file to your project's root: diff --git a/composer.json b/composer.json index 499277d..e57d3ec 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,11 @@ "webimpress/coding-standard": "^1.1" }, "require-dev": { - "phpunit/phpunit": "^10.2" + "phpstan/phpstan": "^2.1", + "phpstan/phpstan-phpunit": "^2.0", + "phpstan/phpstan-strict-rules": "^2.0", + "phpunit/phpunit": "^10.2", + "vimeo/psalm": "^6.16" }, "autoload": { "psr-4": { diff --git a/php-cs-fixer-rules.php b/php-cs-fixer-rules.php index 9bb9dc2..f89b42b 100644 --- a/php-cs-fixer-rules.php +++ b/php-cs-fixer-rules.php @@ -77,7 +77,10 @@ 'php_unit_data_provider_static' => true, 'single_quote' => true, 'single_space_around_construct' => true, - 'strict_comparison' => true, + // Loose comparison is lint territory — PHPStan and Psalm both flag it. Kept + // explicitly off so a broader preset (e.g. @PhpCsFixer, @Symfony) can't + // re-enable it silently. + 'strict_comparison' => false, 'trailing_comma_in_multiline' => [ 'elements' => [ 'arrays', diff --git a/tests/check-sa-overlap.sh b/tests/check-sa-overlap.sh new file mode 100755 index 0000000..dd31eb0 --- /dev/null +++ b/tests/check-sa-overlap.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# CI gate: assert that PHPStan ∩ Psalm (over fixtures) equals exactly the set +# of fixtures registered below as "dropped-rule canaries". +# +# Each entry in EXPECTED_REL is a fixture demonstrating a pattern whose CS +# rule we dropped because lint already catches it. Two invariants: +# 1. Each canary MUST be flagged by both PHPStan and Psalm. Missing entries +# mean SA configs drifted, a tool crashed, or the JSON schema changed — +# fail loudly rather than silently letting fixtures through. +# 2. NO other fixture may appear in the intersection. A new fixture flagged +# by both SAs is "lint territory" — drop or rewrite it instead of adding +# a CS rule. +# +# Each canary also lives in tests/fixtures/valid/ so RulesTest::valid asserts +# the CS side: phpcs and php-cs-fixer must NOT flag it (= the CS rule really +# is disabled). +set -uo pipefail + +if ! command -v jq >/dev/null 2>&1; then + echo "ERROR: jq is required but was not found in PATH." >&2 + echo "Install jq (e.g., 'apt-get install jq' or 'brew install jq')." >&2 + exit 1 +fi + +cd "$(dirname "$0")/.." + +EXPECTED_REL=( + tests/fixtures/valid/loose-comparison.php +) + +for f in "${EXPECTED_REL[@]}"; do + if [ ! -f "$f" ]; then + echo "ERROR: registered canary fixture does not exist: $f" >&2 + echo "Update EXPECTED_REL in $(basename "$0") if the fixture was renamed or removed." >&2 + exit 1 + fi +done + +expected="$(for f in "${EXPECTED_REL[@]}"; do realpath "$f"; done | sort -u)" + +phpstan_stderr="$(mktemp "${TMPDIR:-/tmp}/check-sa-overlap.phpstan.XXXXXX")" +psalm_stderr="$(mktemp "${TMPDIR:-/tmp}/check-sa-overlap.psalm.XXXXXX")" +trap 'rm -f "$phpstan_stderr" "$psalm_stderr"' EXIT + +phpstan_json="$(vendor/bin/phpstan analyse \ + --configuration tests/phpstan.neon \ + --error-format=json \ + --no-progress 2>"$phpstan_stderr" || true)" + +psalm_json="$(vendor/bin/psalm \ + --config tests/psalm.xml \ + --output-format=json \ + --no-progress 2>"$psalm_stderr" || true)" + +phpstan_files="$(printf '%s' "$phpstan_json" | jq -r '.files? | keys[]?' 2>>"$phpstan_stderr" | sort -u)" +psalm_files="$(printf '%s' "$psalm_json" | jq -r '.[]?.file_path // empty' 2>>"$psalm_stderr" | sort -u)" + +intersection="$(comm -12 <(printf '%s\n' "$phpstan_files") <(printf '%s\n' "$psalm_files") | sed '/^$/d')" + +missing="$(comm -23 <(printf '%s\n' "$expected") <(printf '%s\n' "$intersection"))" +extra="$(comm -13 <(printf '%s\n' "$expected") <(printf '%s\n' "$intersection"))" + +if [ -n "$missing" ]; then + echo "ERROR: expected canary fixtures missing from PHPStan ∩ Psalm:" + printf '%s\n' "$missing" | sed 's/^/ - /' + echo + echo "PHPStan flagged:" + printf '%s\n' "${phpstan_files:-(nothing)}" + echo + echo "Psalm flagged:" + printf '%s\n' "${psalm_files:-(nothing)}" + echo + echo "PHPStan stderr:" + cat "$phpstan_stderr" 2>/dev/null || echo "(empty)" + echo + echo "Psalm stderr:" + cat "$psalm_stderr" 2>/dev/null || echo "(empty)" + echo + echo "SA configs may have drifted too lax, a tool may have crashed, or the JSON schema changed." + exit 1 +fi + +if [ -n "$extra" ]; then + echo "ERROR: the following fixtures are flagged by BOTH PHPStan and Psalm:" + printf '%s\n' "$extra" | sed 's/^/ - /' + echo + echo "Both major static analyzers already catch these patterns. A CS rule for" + echo "them duplicates lint work users already have. Drop the fixture (and any" + echo "corresponding CS rule), or rewrite it so only one SA catches it." + echo + echo "If this is a deliberate rule drop, also register the fixture in" + echo "EXPECTED_REL in this script." + exit 1 +fi + +echo "OK: PHPStan ∩ Psalm matches the registered canary set exactly." diff --git a/tests/fixtures/invalid/loose-comparison.php b/tests/fixtures/invalid/loose-comparison.php deleted file mode 100644 index f2ca38d..0000000 --- a/tests/fixtures/invalid/loose-comparison.php +++ /dev/null @@ -1,7 +0,0 @@ - $items + */ function foo(array $items): void { foreach ($items as $item) { diff --git a/tests/fixtures/valid/psr-12-import-order.php b/tests/fixtures/valid/psr-12-import-order.php index 5f96381..5370545 100644 --- a/tests/fixtures/valid/psr-12-import-order.php +++ b/tests/fixtures/valid/psr-12-import-order.php @@ -14,5 +14,5 @@ use const E_USER_DEPRECATED; -new stdClass(); -trigger_error('Test', E_USER_DEPRECATED); +$obj = new stdClass(); +trigger_error('Test ' . $obj::class, E_USER_DEPRECATED); diff --git a/tests/phpstan.neon b/tests/phpstan.neon new file mode 100644 index 0000000..f52aae4 --- /dev/null +++ b/tests/phpstan.neon @@ -0,0 +1,24 @@ +# PHPStan config for the test fixtures. +# +# This project is a coding-standard package, not an application. We don't lint +# our sniff/fixer sources with PHPStan (yet). PHPStan runs over the fixtures so +# that if someone adds a fixture demonstrating something PHPStan already catches, +# CI fails — that's the signal "this is lint territory, don't add a CS rule for it". +# +# If we ever want PHPStan to check the production sources too, add a separate +# config at the repository root. + +parameters: + level: max + paths: + - fixtures + # Fixtures are isolated code snippets that routinely declare consts purely + # to exercise a syntactic pattern, with no consumer. classConstant.unused + # fires on that shape and is not a lint bug we want to catch in fixtures. + ignoreErrors: + - identifier: classConstant.unused + +includes: + - ../vendor/phpstan/phpstan-phpunit/extension.neon + - ../vendor/phpstan/phpstan-phpunit/rules.neon + - ../vendor/phpstan/phpstan-strict-rules/rules.neon diff --git a/tests/psalm.xml b/tests/psalm.xml new file mode 100644 index 0000000..4b347bd --- /dev/null +++ b/tests/psalm.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + +