diff --git a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php deleted file mode 100644 index 6f3cd18..0000000 --- a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php +++ /dev/null @@ -1,7 +0,0 @@ -getName() === 'this') { - // 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){ + // 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()) { $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), 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 e8f7249..7542918 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 or non-static anonymous functions.', + '$this can only be used in an object context.', 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 or non-static anonymous functions.', + '$this can only be used in an object context.', DiagnosticSeverity::ERROR, new Range( new Position(2, 5), @@ -115,94 +115,4 @@ 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) - ) - ); - } }