Skip to content

PHPStan Level 3 #385

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

Merged
merged 1 commit into from
Nov 21, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ cache:
- validation/frameworks

before_script:
- if [[ $STATIC_ANALYSIS = true ]]; then composer require phpstan/phpstan --no-update; fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required in --dev now as PHPStan is a PHAR

- composer install
- set -e # Stop on first error.
- phpenv config-rm xdebug.ini || true
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"php": ">=7.2"
},
"require-dev": {
"phpunit/phpunit": "^8.5.15"
"phpunit/phpunit": "^8.5.15",
"phpstan/phpstan": "^1.8"
},
"license": "MIT",
"authors": [
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
parameters:
level: 2
level: 3
paths:
- src/
ignoreErrors:
Expand Down
3 changes: 3 additions & 0 deletions src/Node.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ public function getRoot() : Node {
while ($node->parent !== null) {
$node = $node->parent;
}

/** @var SourceFileNode $node */
return $node;
}

Expand Down Expand Up @@ -613,6 +615,7 @@ public function getNamespaceDefinition() {
$namespaceDefinition = null;
}

/** @var NamespaceDefinition|null $namespaceDefinition */
return $namespaceDefinition;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could add a runtime check for stuff like this or refactor but it's probably a risky move considering the number of similar changes needed across the whole code base.

}

Expand Down
2 changes: 1 addition & 1 deletion src/Node/EnumCaseDeclaration.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class EnumCaseDeclaration extends Node {
/** @var Token */
public $caseKeyword;

/** @var QualifiedName */
/** @var Token */
public $name;

/** @var Token|null */
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/AssignmentExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

class AssignmentExpression extends BinaryExpression {

/** @var Expression */
/** @var Expression|Token */
public $leftOperand;

/** @var Token */
Expand Down
4 changes: 2 additions & 2 deletions src/Node/Expression/BinaryExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@

class BinaryExpression extends Expression {

/** @var Expression */
/** @var Expression|Token */
public $leftOperand;

/** @var Token */
public $operator;

/** @var Expression */
/** @var Expression|Token */
public $rightOperand;

const CHILD_NAMES = [
Expand Down
3 changes: 0 additions & 3 deletions src/Node/Expression/PrefixUpdateExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ class PrefixUpdateExpression extends UnaryExpression {
/** @var Token */
public $incrementOrDecrementOperator;

/** @var Variable */
public $operand;

const CHILD_NAMES = [
'incrementOrDecrementOperator',
'operand'
Expand Down
3 changes: 2 additions & 1 deletion src/Node/Expression/SubscriptExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Microsoft\PhpParser\Node\Expression;

use Microsoft\PhpParser\MissingToken;
use Microsoft\PhpParser\Node\Expression;
use Microsoft\PhpParser\Token;

Expand All @@ -17,7 +18,7 @@ class SubscriptExpression extends Expression {
/** @var Token */
public $openBracketOrBrace;

/** @var Expression */
/** @var Expression|MissingToken */
public $accessExpression;

/** @var Token */
Expand Down
3 changes: 2 additions & 1 deletion src/Node/Expression/UnaryExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
namespace Microsoft\PhpParser\Node\Expression;

use Microsoft\PhpParser\Node\Expression;
use Microsoft\PhpParser\Token;

class UnaryExpression extends Expression {
/** @var UnaryExpression|Variable */
/** @var Expression|Variable|Token */
public $operand;

const CHILD_NAMES = [
Expand Down
3 changes: 0 additions & 3 deletions src/Node/Expression/UnaryOpExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ class UnaryOpExpression extends UnaryExpression {
/** @var Token */
public $operator;

/** @var UnaryExpression */
public $operand;

const CHILD_NAMES = [
'operator',
'operand'
Expand Down
3 changes: 2 additions & 1 deletion src/Node/FunctionReturnType.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Microsoft\PhpParser\Node;

use Microsoft\PhpParser\MissingToken;
use Microsoft\PhpParser\Token;

trait FunctionReturnType {
Expand All @@ -14,6 +15,6 @@ trait FunctionReturnType {
// TODO: This may be the wrong choice if ?type can ever be mixed with other types in union types
/** @var Token|null */
public $questionToken;
/** @var DelimitedList\QualifiedNameList|null */
/** @var DelimitedList\QualifiedNameList|null|MissingToken */
public $returnTypeList;
}
3 changes: 2 additions & 1 deletion src/Node/MissingMemberDeclaration.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Microsoft\PhpParser\Node;

use Microsoft\PhpParser\MissingToken;
use Microsoft\PhpParser\ModifiedTypeInterface;
use Microsoft\PhpParser\ModifiedTypeTrait;
use Microsoft\PhpParser\Node;
Expand All @@ -20,7 +21,7 @@ class MissingMemberDeclaration extends Node implements ModifiedTypeInterface {
/** @var Token|null needed along with typeDeclaration for what looked like typed property declarations but was missing VariableName */
public $questionToken;

/** @var DelimitedList\QualifiedNameList|null */
/** @var DelimitedList\QualifiedNameList|null|MissingToken */
public $typeDeclarationList;

const CHILD_NAMES = [
Expand Down
3 changes: 2 additions & 1 deletion src/Node/NamespaceUseClause.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@

namespace Microsoft\PhpParser\Node;

use Microsoft\PhpParser\MissingToken;
use Microsoft\PhpParser\Node;
use Microsoft\PhpParser\Node\DelimitedList;
use Microsoft\PhpParser\Token;

class NamespaceUseClause extends Node {
/** @var QualifiedName */
/** @var QualifiedName|MissingToken */
public $namespaceName;
/** @var NamespaceAliasingClause */
public $namespaceAliasingClause;
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Statement/InlineHtml.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class InlineHtml extends StatementNode {
public $scriptSectionStartTag;

/**
* @var ExpressionStatement|null used to represent the expression echoed by `<?=` while parsing.
* @var EchoStatement|null used to represent the expression echoed by `<?=` while parsing.
*
* This should always be null in the returned AST,
* and is deliberately excluded from CHILD_NAMES.
Expand Down
3 changes: 2 additions & 1 deletion src/Node/Statement/NamespaceDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Microsoft\PhpParser\Node\Statement;

use Microsoft\PhpParser\MissingToken;
use Microsoft\PhpParser\Node\QualifiedName;
use Microsoft\PhpParser\Node\StatementNode;
use Microsoft\PhpParser\Token;
Expand All @@ -17,7 +18,7 @@
class NamespaceDefinition extends StatementNode {
/** @var Token */
public $namespaceKeyword;
/** @var QualifiedName|null */
/** @var QualifiedName|null|MissingToken */
public $name;
/** @var CompoundStatementNode|Token */
public $compoundStatementOrSemicolon;
Expand Down
44 changes: 26 additions & 18 deletions src/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ private function parseClassDeclaration($parentNode) : Node {
return $classNode;
}

private function parseClassMembers($parentNode) : Node {
private function parseClassMembers($parentNode) : ClassMembersNode {
$classMembers = new ClassMembersNode();
$classMembers->openBrace = $this->eat1(TokenKind::OpenBraceToken);
$classMembers->classMemberDeclarations = $this->parseList($classMembers, ParseContext::ClassMembers);
Expand Down Expand Up @@ -804,7 +804,7 @@ private function parseAttributeGroups($parentNode): array
}

/**
* @return DelimitedList\AttributeElementList
* @return DelimitedList\AttributeElementList|null
*/
private function parseAttributeElementList(AttributeGroup $parentNode) {
return $this->parseDelimitedList(
Expand Down Expand Up @@ -1638,13 +1638,14 @@ private function isParameterStartFn() {
}

/**
* @param string $className (name of subclass of DelimitedList)
* @template TDelimitedList of DelimitedList
* @param class-string<TDelimitedList> $className (name of subclass of DelimitedList)
* @param int|int[] $delimiter
* @param callable $isElementStartFn
* @param callable $parseElementFn
* @param Node $parentNode
* @param bool $allowEmptyElements
* @return DelimitedList|null instance of $className
* @return TDelimitedList|null instance of $className
*/
private function parseDelimitedList($className, $delimiter, $isElementStartFn, $parseElementFn, $parentNode, $allowEmptyElements = false) {
// TODO consider allowing empty delimiter to be more tolerant
Expand Down Expand Up @@ -1994,7 +1995,7 @@ private function parseWhileStatement($parentNode) {
/**
* @param Node $parentNode
* @param bool $force
* @return Node|MissingToken|array - The expression, or a missing token, or (if $force) an array containing a missed and skipped token
* @return Expression|MissingToken|array - The expression, or a missing token, or (if $force) an array containing a missed and skipped token
*/
private function parseExpression($parentNode, $force = false) {
$token = $this->getCurrentToken();
Expand All @@ -2020,7 +2021,7 @@ private function parseExpressionFn() {

/**
* @param Node $parentNode
* @return Expression
* @return UnaryExpression|MissingToken|Variable|ThrowExpression
*/
private function parseUnaryExpressionOrHigher($parentNode) {
$token = $this->getCurrentToken();
Expand Down Expand Up @@ -3212,9 +3213,16 @@ private function parseMemberAccessExpression($expression):MemberAccessExpression
private function parseScopedPropertyAccessExpression($expression, $fallbackParentNode): ScopedPropertyAccessExpression {
$scopedPropertyAccessExpression = new ScopedPropertyAccessExpression();
$scopedPropertyAccessExpression->parent = $expression->parent ?? $fallbackParentNode;

if ($expression instanceof Node) {
$expression->parent = $scopedPropertyAccessExpression;
$scopedPropertyAccessExpression->scopeResolutionQualifier = $expression; // TODO ensure always a Node

// scopeResolutionQualifier does not accept `Node` but
Copy link
Contributor

@TysonAndre TysonAndre Sep 25, 2022

Choose a reason for hiding this comment

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

LGTM overall for latest commits; they addressed review comments/nits.


In followup PRs, that should hopefully be addressed by making parameter types more precise as well, assuming that phpstan can narrow a mix of subclasses of Node and definite non-subclassess to just the expected subclasses Expression|QualifiedName (haven't checked)

(Strict typing overall is useful so we don't accidentally write to an undeclared property, which would be a notice in 8.2 since the classes declared here aren't using #[\AllowDynamicProperties], though I don't need AllowDynamicProperties for my own use case)

// `Expression|QualifiedName|Token`. I'm not sure if we can depend
// on that being the case.
//
// @phpstan-ignore-next-line
$scopedPropertyAccessExpression->scopeResolutionQualifier = $expression;
}

$scopedPropertyAccessExpression->doubleColon = $this->eat1(TokenKind::ColonColonToken);
Expand Down Expand Up @@ -3362,18 +3370,18 @@ private function parseClassConstDeclaration($parentNode, $modifiers) {
}

private function parseEnumCaseDeclaration($parentNode) {
$classConstDeclaration = new EnumCaseDeclaration();
$classConstDeclaration->parent = $parentNode;
$classConstDeclaration->caseKeyword = $this->eat1(TokenKind::CaseKeyword);
$classConstDeclaration->name = $this->eat($this->nameOrKeywordOrReservedWordTokens);
$classConstDeclaration->equalsToken = $this->eatOptional1(TokenKind::EqualsToken);
if ($classConstDeclaration->equalsToken !== null) {
$enumCaseDeclaration = new EnumCaseDeclaration();
$enumCaseDeclaration->parent = $parentNode;
$enumCaseDeclaration->caseKeyword = $this->eat1(TokenKind::CaseKeyword);
$enumCaseDeclaration->name = $this->eat($this->nameOrKeywordOrReservedWordTokens);
$enumCaseDeclaration->equalsToken = $this->eatOptional1(TokenKind::EqualsToken);
if ($enumCaseDeclaration->equalsToken !== null) {
// TODO add post-parse rule that checks for invalid assignments
$classConstDeclaration->assignment = $this->parseExpression($classConstDeclaration);
$enumCaseDeclaration->assignment = $this->parseExpression($enumCaseDeclaration);
}
$classConstDeclaration->semicolon = $this->eat1(TokenKind::SemicolonToken);
$enumCaseDeclaration->semicolon = $this->eat1(TokenKind::SemicolonToken);

return $classConstDeclaration;
return $enumCaseDeclaration;
}

/**
Expand Down Expand Up @@ -3446,7 +3454,7 @@ private function parseQualifiedNameCatchList($parentNode) {
return $result;
}

private function parseInterfaceDeclaration($parentNode) {
private function parseInterfaceDeclaration($parentNode): InterfaceDeclaration {
$interfaceDeclaration = new InterfaceDeclaration(); // TODO verify not nested
$interfaceDeclaration->parent = $parentNode;
$interfaceDeclaration->interfaceKeyword = $this->eat1(TokenKind::InterfaceKeyword);
Expand All @@ -3456,7 +3464,7 @@ private function parseInterfaceDeclaration($parentNode) {
return $interfaceDeclaration;
}

private function parseInterfaceMembers($parentNode) : Node {
private function parseInterfaceMembers($parentNode) : InterfaceMembers {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably dozens of other places we can do this, but figure PHPStan will pick this up on level 4 or 5

$interfaceMembers = new InterfaceMembers();
$interfaceMembers->openBrace = $this->eat1(TokenKind::OpenBraceToken);
$interfaceMembers->interfaceMemberDeclarations = $this->parseList($interfaceMembers, ParseContext::InterfaceMembers);
Expand Down