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

Declare more precise phpdoc types #993

Merged
merged 8 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
- name: "Install dependencies"
run: |
composer require php-coveralls/php-coveralls:^2.2 --dev --no-update
composer update --no-progress --prefer-dist
COMPOSER_ROOT_VERSION=dev-master composer update --no-progress --prefer-dist
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do / why is it necessary?

Copy link
Contributor Author

@staabm staabm Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this fix, CI errors in PRs of forks. see #993 (comment)

errors beeing fixed can be seen here: https://github.com/nikic/PHP-Parser/actions/runs/8671744331/job/23781260607?pr=993

Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit[9.6.12, ..., 9.6.19] require phpunit/php-code-coverage ^9.2.28 -> satisfiable by phpunit/php-code-coverage[9.2.28, 9.2.29, 9.2.30, 9.2.31].
    - phpunit/phpunit[9.5.16, ..., 9.6.11] require phpunit/php-code-coverage ^9.2.13 -> satisfiable by phpunit/php-code-coverage[9.2.13, ..., 9.2.31].
    - phpunit/phpunit[9.5.10, ..., 9.5.14] require phpunit/php-code-coverage ^9.2.7 -> satisfiable by phpunit/php-code-coverage[9.2.7, ..., 9.2.31].
    - phpunit/phpunit[9.5.0, ..., 9.5.9] require phpunit/php-code-coverage ^9.2.3 -> satisfiable by phpunit/php-code-coverage[9.2.3, ..., 9.2.31].
    - phpunit/phpunit[9.4.0, ..., 9.4.4] require phpunit/php-code-coverage ^9.2 -> satisfiable by phpunit/php-code-coverage[9.2.0, ..., 9.2.31].
    - phpunit/phpunit 9.3.11 requires phpunit/php-code-coverage ^9.1.11 -> satisfiable by phpunit/php-code-coverage[9.1.11, ..., 9.2.31].
    - phpunit/phpunit[9.3.8, ..., 9.3.10] require phpunit/php-code-coverage ^9.1.5 -> satisfiable by phpunit/php-code-coverage[9.1.5, ..., 9.2.31].
    - phpunit/phpunit[9.3.4, ..., 9.3.7] require phpunit/php-code-coverage ^9.1.1 -> satisfiable by phpunit/php-code-coverage[9.1.1, ..., 9.2.31].
    - phpunit/phpunit[9.3.0, ..., 9.3.3] require phpunit/php-code-coverage ^9.0 -> satisfiable by phpunit/php-code-coverage[9.0.0, ..., 9.2.31].
    - phpunit/phpunit[9.0.0, ..., 9.2.6] require php ^7.3 -> your php version (8.3.6) does not satisfy that requirement.
    - phpunit/php-code-coverage[9.0.0, ..., 9.1.2] require nikic/php-parser ^4.7 -> satisfiable by nikic/php-parser[v4.7.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.1.3, ..., 9.2.2] require nikic/php-parser ^4.8 -> satisfiable by nikic/php-parser[v4.8.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.2.3, ..., 9.2.6] require nikic/php-parser ^4.10.2 -> satisfiable by nikic/php-parser[v4.10.2, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage 9.2.7 requires nikic/php-parser ^4.12.0 -> satisfiable by nikic/php-parser[v4.12.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.2.8, ..., 9.2.15] require nikic/php-parser ^4.13.0 -> satisfiable by nikic/php-parser[v4.13.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.2.16, ..., 9.2.24] require nikic/php-parser ^4.14 -> satisfiable by nikic/php-parser[v4.14.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.2.25, ..., 9.2.29] require nikic/php-parser ^4.15 -> satisfiable by nikic/php-parser[v4.15.0, ..., 4.x-dev] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - phpunit/php-code-coverage[9.2.30, ..., 9.2.31] require nikic/php-parser ^4.18 || ^5.0 -> satisfiable by nikic/php-parser[dev-master, v4.18.0, v4.19.0, v4.19.1, 4.x-dev, v5.0.0alpha1, ..., 5.0.x-dev (alias of dev-master)] from composer repo (https://repo.packagist.org/) but nikic/php-parser is the root package and cannot be modified. See https://getcomposer.org/dep-on-root for details and assistance.
    - Root composer.json requires phpunit/phpunit ^9.0 -> satisfiable by phpunit/phpunit[9.0.0, ..., 9.6.19].

Error: Your requirements could not be resolved to an installable set of packages.

without thix fix composer does not realize what the version of the package of the checked-out project is

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I guess I caused this with b43758e and previously this probably just resolved to PHPUnit 8 instead...

- name: "Tests"
run: "php vendor/bin/phpunit --coverage-clover build/logs/clover.xml"
- name: Coveralls
Expand Down Expand Up @@ -49,7 +49,7 @@ jobs:
ini-file: "development"
tools: composer:v2
- name: "Install dependencies"
run: "composer update --no-progress --prefer-dist ${{ matrix.flags }}"
run: "COMPOSER_ROOT_VERSION=dev-master composer update --no-progress --prefer-dist ${{ matrix.flags }}"
- name: "PHPUnit"
run: "php vendor/bin/phpunit"
test_old_73_80:
Expand All @@ -66,7 +66,7 @@ jobs:
ini-file: "development"
tools: composer:v2
- name: "Install PHP 8 dependencies"
run: "composer update --no-progress --prefer-dist"
run: "COMPOSER_ROOT_VERSION=dev-master composer update --no-progress --prefer-dist"
- name: "Tests"
run: "test_old/run-php-src.sh 7.4.33"
test_old_80_70:
Expand All @@ -83,7 +83,7 @@ jobs:
ini-file: "development"
tools: composer:v2
- name: "Install PHP 8 dependencies"
run: "composer update --no-progress --prefer-dist"
run: "COMPOSER_ROOT_VERSION=dev-master composer update --no-progress --prefer-dist"
- name: "Tests"
run: "test_old/run-php-src.sh 8.3.0RC2"
phpstan:
Expand Down
1 change: 1 addition & 0 deletions lib/PhpParser/Node.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ interface Node {
/**
* Gets the type of the node.
*
* @psalm-return non-empty-string
* @return string Type of the node
*/
public function getType(): string;
Expand Down
12 changes: 11 additions & 1 deletion lib/PhpParser/Node/Identifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
* Represents a non-namespaced name. Namespaced names are represented using Name nodes.
*/
class Identifier extends NodeAbstract {
/** @var string Identifier as string */
/**
* @psalm-var non-empty-string
* @var string Identifier as string
*/
public string $name;

/** @var array<string, bool> */
Expand All @@ -25,6 +28,10 @@ class Identifier extends NodeAbstract {
* @param array<string, mixed> $attributes Additional attributes
*/
public function __construct(string $name, array $attributes = []) {
if ($name === '') {
throw new \InvalidArgumentException('Identifier name cannot be empty');
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


$this->attributes = $attributes;
$this->name = $name;
}
Expand All @@ -36,6 +43,7 @@ public function getSubNodeNames(): array {
/**
* Get identifier as string.
*
* @psalm-return non-empty-string
* @return string Identifier as string.
*/
public function toString(): string {
Expand All @@ -45,6 +53,7 @@ public function toString(): string {
/**
* Get lowercased identifier as string.
*
* @psalm-return non-empty-string
* @return string Lowercased identifier as string
*/
public function toLowerString(): string {
Expand All @@ -63,6 +72,7 @@ public function isSpecialClassName(): bool {
/**
* Get identifier as string.
*
* @psalm-return non-empty-string
* @return string Identifier as string
*/
public function __toString(): string {
Expand Down
11 changes: 10 additions & 1 deletion lib/PhpParser/Node/Name.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
use PhpParser\NodeAbstract;

class Name extends NodeAbstract {
/** @var string Name as string */
/**
* @psalm-var non-empty-string
* @var string Name as string
*/
public string $name;

/** @var array<string, bool> */
Expand Down Expand Up @@ -33,6 +36,7 @@ public function getSubNodeNames(): array {
/**
* Get parts of name (split by the namespace separator).
*
* @psalm-return non-empty-list<string>
* @return string[] Parts of name
*/
public function getParts(): array {
Expand Down Expand Up @@ -103,6 +107,7 @@ public function isRelative(): bool {
* Returns a string representation of the name itself, without taking the name type into
* account (e.g., not including a leading backslash for fully qualified names).
*
* @psalm-return non-empty-string
* @return string String representation
*/
public function toString(): string {
Expand All @@ -113,6 +118,7 @@ public function toString(): string {
* Returns a string representation of the name as it would occur in code (e.g., including
* leading backslash for fully qualified names.
*
* @psalm-return non-empty-string
* @return string String representation
*/
public function toCodeString(): string {
Expand All @@ -123,6 +129,7 @@ public function toCodeString(): string {
* Returns lowercased string representation of the name, without taking the name type into
* account (e.g., no leading backslash for fully qualified names).
*
* @psalm-return non-empty-string
* @return string Lowercased string representation
*/
public function toLowerString(): string {
Expand All @@ -142,6 +149,7 @@ public function isSpecialClassName(): bool {
* Returns a string representation of the name by imploding the namespace parts with the
* namespace separator.
*
* @psalm-return non-empty-string
* @return string String representation
*/
public function __toString(): string {
Expand Down Expand Up @@ -237,6 +245,7 @@ public static function concat($name1, $name2, array $attributes = []) {
*
* @param string|string[]|self $name Name to prepare
*
* @psalm-return non-empty-string
* @return string Prepared name
*/
private static function prepareName($name): string {
Expand Down
5 changes: 5 additions & 0 deletions test/PhpParser/Node/IdentifierTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
namespace PhpParser\Node;

class IdentifierTest extends \PHPUnit\Framework\TestCase {
public function testConstructorThrows() {
self::expectException(\InvalidArgumentException::class);
new Identifier('');
}

public function testToString() {
$identifier = new Identifier('Foo');

Expand Down