diff --git a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php new file mode 100644 index 0000000..6f3cd18 --- /dev/null +++ b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php @@ -0,0 +1,7 @@ +getName() === 'this') { - // Find the first ancestor that's a class method. Return an error - // if there is none, or if the method is static. - $method = $node->getFirstAncestor(Node\MethodDeclaration::class); - if ($method === null || $method->isStatic()) { + // Find the first ancestor that's a class method or anonymous function. + // Return an error if there is none, or if the method or anonymous function is static + // Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/} + $methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class); + if ($methodOrAnnonFunc === null){ $this->diagnostics[] = new Diagnostic( - $method === null - ? "\$this can only be used in an object context." - : "\$this can not be used in static methods.", + "\$this can only be used in an object context or non-static anonymous functions.", Range::fromNode($node), null, DiagnosticSeverity::ERROR, '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? + } + } } } } diff --git a/tests/Diagnostics/InvalidThisUsageTest.php b/tests/Diagnostics/InvalidThisUsageTest.php index 7542918..e8f7249 100644 --- a/tests/Diagnostics/InvalidThisUsageTest.php +++ b/tests/Diagnostics/InvalidThisUsageTest.php @@ -76,11 +76,11 @@ class InvalidThisUsageTest extends TestCase $diagnostics = $this->collectDiagnostics( __DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php' ); - + $this->assertCount(1, $diagnostics); $this->assertDiagnostic( $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, new Range( new Position(4, 11), @@ -98,7 +98,7 @@ class InvalidThisUsageTest extends TestCase $this->assertCount(1, $diagnostics); $this->assertDiagnostic( $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, new Range( new Position(2, 5), @@ -115,4 +115,94 @@ class InvalidThisUsageTest extends TestCase $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) + ) + ); + } }