Tolerate $this usage in anonymous functions
Alternative solution for issue 536. Gives warning "$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check." for $this var usage. And hints developer to check class of $this before usage. Class check can be added in form "if ($this instanceof [class])" {/*use $this*/}. Note: if class check added no type checking is done its in language server it just hides the warning.pull/543/head
parent
ff746a836d
commit
47d6424c98
|
@ -0,0 +1,7 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
$dummy = function(){
|
||||||
|
if($this instanceof StdClass){
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
};
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo
|
||||||
|
{
|
||||||
|
public function bar()
|
||||||
|
{
|
||||||
|
return function(){
|
||||||
|
return $this;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,13 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo
|
||||||
|
{
|
||||||
|
public function bar()
|
||||||
|
{
|
||||||
|
return function(){
|
||||||
|
if($this instanceof StdClass){
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,13 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo
|
||||||
|
{
|
||||||
|
public static function bar()
|
||||||
|
{
|
||||||
|
return function(){
|
||||||
|
if($this instanceof StdClass){
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,5 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
$dummy = static function(){
|
||||||
|
return $this;
|
||||||
|
};
|
|
@ -0,0 +1,5 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
$dummy = function(){
|
||||||
|
return $this;
|
||||||
|
};
|
|
@ -0,0 +1,5 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
$dummy = function(){
|
||||||
|
return $this;
|
||||||
|
};
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo
|
||||||
|
{
|
||||||
|
public static function bar()
|
||||||
|
{
|
||||||
|
return function(){
|
||||||
|
return $this;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
|
@ -96,19 +96,66 @@ class TreeAnalyzer
|
||||||
|
|
||||||
// Check for invalid usage of $this.
|
// Check for invalid usage of $this.
|
||||||
if ($node instanceof Node\Expression\Variable && $node->getName() === 'this') {
|
if ($node instanceof Node\Expression\Variable && $node->getName() === 'this') {
|
||||||
// Find the first ancestor that's a class method. Return an error
|
// Find the first ancestor that's a class method or anonymous function.
|
||||||
// if there is none, or if the method is static.
|
// Return an error if there is none, or if the method or anonymous function is static
|
||||||
$method = $node->getFirstAncestor(Node\MethodDeclaration::class);
|
// Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/}
|
||||||
if ($method === null || $method->isStatic()) {
|
$methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class);
|
||||||
|
if ($methodOrAnnonFunc === null){
|
||||||
$this->diagnostics[] = new Diagnostic(
|
$this->diagnostics[] = new Diagnostic(
|
||||||
$method === null
|
"\$this can only be used in an object context or non-static anonymous functions.",
|
||||||
? "\$this can only be used in an object context."
|
|
||||||
: "\$this can not be used in static methods.",
|
|
||||||
Range::fromNode($node),
|
Range::fromNode($node),
|
||||||
null,
|
null,
|
||||||
DiagnosticSeverity::ERROR,
|
DiagnosticSeverity::ERROR,
|
||||||
'php'
|
'php'
|
||||||
);
|
);
|
||||||
|
} else if ($methodOrAnnonFunc instanceof Node\MethodDeclaration){
|
||||||
|
$method = $methodOrAnnonFunc;
|
||||||
|
if ($method->isStatic()) {
|
||||||
|
$this->diagnostics[] = new Diagnostic(
|
||||||
|
"\$this can not be used in static methods.",
|
||||||
|
Range::fromNode($node),
|
||||||
|
null,
|
||||||
|
DiagnosticSeverity::ERROR,
|
||||||
|
'php'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
} else if ($methodOrAnnonFunc instanceof Node\Expression\AnonymousFunctionCreationExpression){
|
||||||
|
$anonFunc = $methodOrAnnonFunc;
|
||||||
|
if ($anonFunc->staticModifier){
|
||||||
|
$this->diagnostics[] = new Diagnostic(
|
||||||
|
"\$this can not be used in static anonymous functions.",
|
||||||
|
Range::fromNode($node),
|
||||||
|
null,
|
||||||
|
DiagnosticSeverity::ERROR,
|
||||||
|
'php'
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
// IfStatementNode must be in AnonymousFunctionCreationExpression so get what was first
|
||||||
|
$ifStatement = $node->getFirstAncestor(Node\Statement\IfStatementNode::class, Node\Expression\AnonymousFunctionCreationExpression::class);
|
||||||
|
// naive check for Node\Expression\BinaryExpression $ifStatement->expression "$this instanceof [class]":
|
||||||
|
// checks
|
||||||
|
// leftOperand Node\Expression\Variable is current node ($this node) or is any $this node in ifStatement
|
||||||
|
// operator kind PhpParser\TokenKind $ifStatement->expression->operator->kind equal to InstanceOfKeyword
|
||||||
|
// for information: class is in PhpParser\Node\QualifiedName $ifStatement->expression->rightOperand
|
||||||
|
if (!($ifStatement
|
||||||
|
&& $ifStatement instanceof Node\Statement\IfStatementNode
|
||||||
|
&& ($expression = $ifStatement->expression)
|
||||||
|
&& $expression instanceof Node\Expression\BinaryExpression
|
||||||
|
&& ($expression->leftOperand = $node
|
||||||
|
|| $expression->leftOperand instanceof Node\Expression\Variable && $expression->leftOperand->getName() === 'this')
|
||||||
|
&& $expression->operator->kind === PhpParser\TokenKind::InstanceOfKeyword
|
||||||
|
)){
|
||||||
|
if (($method = $node->getFirstAncestor(Node\MethodDeclaration::class))===null || $method->isStatic()){
|
||||||
|
$this->diagnostics[] = new Diagnostic(
|
||||||
|
"\$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.",
|
||||||
|
Range::fromNode($node),
|
||||||
|
null,
|
||||||
|
DiagnosticSeverity::WARNING,
|
||||||
|
'php'
|
||||||
|
);
|
||||||
|
}//else we should warn here that invoker can bind $this to object of any class. Should we?
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -76,11 +76,11 @@ class InvalidThisUsageTest extends TestCase
|
||||||
$diagnostics = $this->collectDiagnostics(
|
$diagnostics = $this->collectDiagnostics(
|
||||||
__DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php'
|
__DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php'
|
||||||
);
|
);
|
||||||
|
|
||||||
$this->assertCount(1, $diagnostics);
|
$this->assertCount(1, $diagnostics);
|
||||||
$this->assertDiagnostic(
|
$this->assertDiagnostic(
|
||||||
$diagnostics[0],
|
$diagnostics[0],
|
||||||
'$this can only be used in an object context.',
|
'$this can only be used in an object context or non-static anonymous functions.',
|
||||||
DiagnosticSeverity::ERROR,
|
DiagnosticSeverity::ERROR,
|
||||||
new Range(
|
new Range(
|
||||||
new Position(4, 11),
|
new Position(4, 11),
|
||||||
|
@ -98,7 +98,7 @@ class InvalidThisUsageTest extends TestCase
|
||||||
$this->assertCount(1, $diagnostics);
|
$this->assertCount(1, $diagnostics);
|
||||||
$this->assertDiagnostic(
|
$this->assertDiagnostic(
|
||||||
$diagnostics[0],
|
$diagnostics[0],
|
||||||
'$this can only be used in an object context.',
|
'$this can only be used in an object context or non-static anonymous functions.',
|
||||||
DiagnosticSeverity::ERROR,
|
DiagnosticSeverity::ERROR,
|
||||||
new Range(
|
new Range(
|
||||||
new Position(2, 5),
|
new Position(2, 5),
|
||||||
|
@ -115,4 +115,94 @@ class InvalidThisUsageTest extends TestCase
|
||||||
|
|
||||||
$this->assertCount(0, $diagnostics);
|
$this->assertCount(0, $diagnostics);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testThisInMethodInAnonymousFunctionWithNoCheckProducesNoError()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(0, $diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInMethodInAnonymousFunctionWithCheckProducesNoError()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(0, $diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInAnonymousFunctionWithCheckProducesNoError()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/baselines/this_in_anonymous_function_check.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(0, $diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInStaticMethodInAnonymousFunctionWithCheckProducesWarning()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(0, $diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInStaticAnonymousFunctionProducesError()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_anonymous_function.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(1, $diagnostics);
|
||||||
|
$this->assertDiagnostic(
|
||||||
|
$diagnostics[0],
|
||||||
|
'$this can not be used in static anonymous functions.',
|
||||||
|
DiagnosticSeverity::ERROR,
|
||||||
|
new Range(
|
||||||
|
new Position(3, 11),
|
||||||
|
new Position(3, 16)
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInAnonymousFunctionWithNoCheckProducesWarning()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/warnings/this_in_anonymous_function.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(1, $diagnostics);
|
||||||
|
$this->assertDiagnostic(
|
||||||
|
$diagnostics[0],
|
||||||
|
'$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
|
||||||
|
DiagnosticSeverity::WARNING,
|
||||||
|
new Range(
|
||||||
|
new Position(3, 11),
|
||||||
|
new Position(3, 16)
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testThisInStaticMethodInAnonymousFunctionWithNoCheckProducesWarning()
|
||||||
|
{
|
||||||
|
$diagnostics = $this->collectDiagnostics(
|
||||||
|
__DIR__ . '/../../fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertCount(1, $diagnostics);
|
||||||
|
$this->assertDiagnostic(
|
||||||
|
$diagnostics[0],
|
||||||
|
'$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
|
||||||
|
DiagnosticSeverity::WARNING,
|
||||||
|
new Range(
|
||||||
|
new Position(7, 19),
|
||||||
|
new Position(7, 24)
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue