diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ad31f47e..3e97e374 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -17,6 +17,14 @@ jobs: fail-fast: false matrix: php-versions: [ '8.0', '8.1', '8.2', '8.3', '8.4', '8.5' ] + dependencies: [ 'highest' ] + include: + # The lowest allowed dependency set needs to be verified only once, and the + # oldest supported PHP is where consumers are most likely to end up with it. + - php-versions: '8.0' + dependencies: 'lowest' + + name: "PHP ${{ matrix.php-versions }} (${{ matrix.dependencies }} deps)" steps: - uses: actions/checkout@v7 @@ -40,13 +48,25 @@ jobs: path: | vendor tools/psalm/vendor - key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} + # composer.lock is not committed, so it cannot take part in the cache key: + # the installed set depends on the PHP version and on the resolution + # strategy, and both must be part of the key to keep the caches separate + key: ${{ runner.os }}-php-${{ matrix.php-versions }}-${{ matrix.dependencies }}-${{ hashFiles('composer.json') }} restore-keys: | - ${{ runner.os }}-php- + ${{ runner.os }}-php-${{ matrix.php-versions }}-${{ matrix.dependencies }}- - name: Install dependencies + if: ${{ matrix.dependencies == 'highest' }} run: composer install --prefer-dist + # Without a committed composer.lock every other job resolves to the newest + # allowed release, so the lower bounds declared in composer.json are never + # exercised. This job installs the other end of those ranges: the set a + # consumer with older pinned dependencies actually gets. + - name: Install lowest allowed dependencies + if: ${{ matrix.dependencies == 'lowest' }} + run: composer update --prefer-dist --prefer-lowest --prefer-stable + # Psalm 6 needs PHP >= 8.1 to run; the analysis target stays 8.0 (phpVersion in psalm.xml) - name: Static Analysis if: ${{ matrix.php-versions == '8.1' }} @@ -54,6 +74,9 @@ jobs: composer install --prefer-dist -d tools/psalm tools/psalm/vendor/bin/psalm --no-cache + # Both dependency sets report coverage: code that supports one version of a + # dependency is dead code under the other one, so a single set can never cover + # all of src. Codecov merges the two reports, and codecov.yml waits for both. - name: Test with coverage if: ${{ matrix.php-versions == '8.0' }} run: ./bin/phpunit -d memory_limit=-1 --coverage-clover clover.xml @@ -67,6 +90,7 @@ jobs: uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: token: ${{ secrets.CODECOV_TOKEN }} + flags: ${{ matrix.dependencies }} phar: runs-on: "ubuntu-22.04" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a9f57122..0081da4a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,3 +50,22 @@ make csfix # run code style fixer make psalm # run static analysis ``` +### Testing against the lowest allowed dependencies + +`composer.lock` is not committed, so `composer install` always gives you the +newest release allowed by `composer.json`. Arkitect is installed as a dependency +of other projects, though, where the resolver often picks something older, so CI +also runs the test suite against the lower bound of every declared constraint. + +To reproduce that job locally: + +```shell +composer update --prefer-lowest --prefer-stable +make test +composer update # restore the newest allowed dependencies +``` + +If a change needs a feature that is not in the lowest allowed version of a +dependency, raise the constraint in `composer.json` instead of working around +it. + diff --git a/codecov.yml b/codecov.yml index 74e42d9d..7fef4984 100644 --- a/codecov.yml +++ b/codecov.yml @@ -29,13 +29,27 @@ comment: layout: "reach, diff, flags, files" behavior: default require_changes: false + # Same reason as codecov.notify.after_n_builds below. + after_n_builds: 2 -# Wait for the (single) coverage upload before computing the status, so the -# check doesn't run against a partial report. +# Wait for both coverage uploads before computing the status, so the check +# doesn't run against a partial report. The build uploads one report per +# dependency set (see the `flags` below): the code supporting one version of a +# dependency is dead code under the other, so only the merge of the two reports +# describes the coverage of src. codecov: require_ci_to_pass: true notify: wait_for_ci: true + after_n_builds: 2 + +# The flags exist to tell the two uploads apart, not to gate them separately: +# neither dependency set covers src on its own, so a per-flag status would be +# meaningless. Nothing is carried forward from older commits either, both +# reports are uploaded on every build. +flag_management: + default_rules: + carryforward: false ignore: - "tests" diff --git a/composer.json b/composer.json index d4c6cc75..c1d6e3a5 100644 --- a/composer.json +++ b/composer.json @@ -23,16 +23,16 @@ ], "require": { "php": "^8.0", - "nikic/php-parser": "~5", + "nikic/php-parser": "^5.3", "ondram/ci-detector": "^4.2", - "phpstan/phpdoc-parser": "^1.2|^2.0", + "phpstan/phpdoc-parser": "^1.20|^2.0", "symfony/console": "^5.4|^6.0|^7.0|^8.0", "symfony/finder": "^5.4|^6.0|^7.0|^8.0", "webmozart/assert": "^1.12|^2.0" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.95", - "mikey179/vfsstream": "^1.6", + "mikey179/vfsstream": "^1.6.8", "phpspec/prophecy": "^1.10", "phpspec/prophecy-phpunit": "^2.5", "phpunit/phpunit": "^9.6|^10.0|^11.0", diff --git a/src/Analyzer/DocblockParserFactory.php b/src/Analyzer/DocblockParserFactory.php index abdf2bd2..51ab383a 100644 --- a/src/Analyzer/DocblockParserFactory.php +++ b/src/Analyzer/DocblockParserFactory.php @@ -12,7 +12,11 @@ class DocblockParserFactory { /** + * Psalm only ever sees one of the two installed versions of phpstan/phpdoc-parser, + * so the constructor calls of the other branch always look wrong to it. + * * @psalm-suppress TooFewArguments + * @psalm-suppress TooManyArguments * @psalm-suppress InvalidArgument */ public static function create(): DocblockParser @@ -20,7 +24,7 @@ public static function create(): DocblockParser $phpDocParser = null; $phpDocLexer = null; - // this if is to allow using v 1.2 or v2 + // this if is to allow using v1 or v2 if (class_exists(ParserConfig::class)) { $parserConfig = new ParserConfig(['lines' => true]); $constExprParser = new ConstExprParser($parserConfig); @@ -30,7 +34,10 @@ public static function create(): DocblockParser } else { $typeParser = new TypeParser(); $constExprParser = new ConstExprParser(); - $phpDocParser = new PhpDocParser($typeParser, $constExprParser); + // on v1 line numbers are opt-in through $usedAttributes, the 5th constructor + // argument: without it every node reports no line and violations would be + // reported on the wrong line + $phpDocParser = new PhpDocParser($typeParser, $constExprParser, false, false, ['lines' => true]); $phpDocLexer = new Lexer(); }