icedream
/
php-language-server
1
0
Fork 0
Code Releases Activity

Revert "Tolerate $this usage in anonymous functions" 

This reverts commit 47d6424c98.
pull/543/head
alWerewolf 2017-11-30 18:10:38 +03:00
parent 47d6424c98
commit ed32ac7659
10 changed files with 10 additions and 217 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
View File

@ -1,7 +0,0 @@
<?php
$dummy = function(){
if($this instanceof StdClass){
return $this;
}
};

11
fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
View File

@ -1,11 +0,0 @@
<?php
class Foo
{
public function bar()
{
return function(){
return $this;
};
}
}

13
fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
View File

@ -1,13 +0,0 @@
<?php
class Foo
{
public function bar()
{
return function(){
if($this instanceof StdClass){
return $this;
}
};
}
}

13
fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
View File

@ -1,13 +0,0 @@
<?php
class Foo
{
public static function bar()
{
return function(){
if($this instanceof StdClass){
return $this;
}
};
}
}

5
fixtures/diagnostics/errors/this_in_static_anonymous_function.php
View File

@ -1,5 +0,0 @@
<?php
$dummy = static function(){
return $this;
};

5
fixtures/diagnostics/warnings/this_in_anonymous_function.php
View File

@ -1,5 +0,0 @@
<?php
$dummy = function(){
return $this;
};

5
fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
View File

@ -1,5 +0,0 @@
<?php
$dummy = function(){
return $this;
};

11
fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
View File

@ -1,11 +0,0 @@
<?php
class Foo
{
public static function bar()
{
return function(){
return $this;
};
}
}

61
src/TreeAnalyzer.php
View File

@ -96,66 +96,19 @@ 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 or anonymous function. // Find the first ancestor that's a class method. Return an error
// Return an error if there is none, or if the method or anonymous function is static // if there is none, or if the method is static.
// Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/} $method = $node->getFirstAncestor(Node\MethodDeclaration::class);
$methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class); if ($method === null || $method->isStatic()) {
if ($methodOrAnnonFunc === null){
$this->diagnostics[] = new Diagnostic( $this->diagnostics[] = new Diagnostic(
"\$this can only be used in an object context or non-static anonymous functions.", $method === null
? "\$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?
}
}
} }
} }
} }

94
tests/Diagnostics/InvalidThisUsageTest.php
View File

@ -80,7 +80,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 or non-static anonymous functions.', '$this can only be used in an object context.',
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 or non-static anonymous functions.', '$this can only be used in an object context.',
DiagnosticSeverity::ERROR, DiagnosticSeverity::ERROR,
new Range( new Range(
new Position(2, 5), new Position(2, 5),
@ -115,94 +115,4 @@ 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)
)
);
}
} }