Skip to content
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

Static analysis: Using $this in a static method should show an error #528

Merged
Merged
9 changes: 9 additions & 0 deletions fixtures/diagnostics/baselines/this_in_method.php
@@ -0,0 +1,9 @@
<?php

class Foo
{
public function bar()
{
return $this;
}
}
6 changes: 6 additions & 0 deletions fixtures/diagnostics/errors/this_in_function.php
@@ -0,0 +1,6 @@
<?php

function foo()
{
return $this;
}
3 changes: 3 additions & 0 deletions fixtures/diagnostics/errors/this_in_root.php
@@ -0,0 +1,3 @@
<?php

echo $this;
9 changes: 9 additions & 0 deletions fixtures/diagnostics/errors/this_in_static_method.php
@@ -0,0 +1,9 @@
<?php

class Foo
{
public static function bar()
{
return $this;
}
}
19 changes: 19 additions & 0 deletions src/TreeAnalyzer.php
Expand Up @@ -68,6 +68,7 @@ public function __construct(PhpParser\Parser $parser, string $content, DocBlockF
*/
private function collectDiagnostics($node)
{
// Get errors from the parser.
if (($error = PhpParser\DiagnosticsProvider::checkDiagnostics($node)) !== null) {
$range = PhpParser\PositionUtilities::getRangeFromPosition($error->start, $error->length, $this->sourceFileNode->fileContents);

Expand All @@ -92,6 +93,24 @@ private function collectDiagnostics($node)
'php'
);
}

// Check for invalid usage of $this.
if ($node instanceof Node\Expression\Variable && $node->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()) {
$this->diagnostics[] = new Diagnostic(
$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'
);
}
}
}

/**
Expand Down
118 changes: 118 additions & 0 deletions tests/Diagnostics/InvalidThisUsageTest.php
@@ -0,0 +1,118 @@
<?php
declare(strict_types = 1);

namespace LanguageServer\Tests\Diagnostics;

use PHPUnit\Framework\TestCase;
use phpDocumentor\Reflection\DocBlockFactory;
use LanguageServer\{
DefinitionResolver, TreeAnalyzer
};
use LanguageServer\Index\{Index};
use LanguageServer\Protocol\{
Diagnostic, DiagnosticSeverity, Position, Range
};
use function LanguageServer\pathToUri;
use Microsoft\PhpParser\Parser;

class InvalidThisUsageTest extends TestCase
{
/**
* Parse the given file and return diagnostics.
*
* @param string $path
* @return Diagnostic[]
*/
private function collectDiagnostics(string $path): array
{
$uri = pathToUri($path);
$parser = new Parser();

$docBlockFactory = DocBlockFactory::createInstance();
$index = new Index;
$definitionResolver = new DefinitionResolver($index);
$content = file_get_contents($path);

$treeAnalyzer = new TreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri);
return $treeAnalyzer->getDiagnostics();
}

/**
* Assertions about a diagnostic.
*
* @param Diagnostic|null $diagnostic
* @param int $message
* @param string $severity
* @param Range $range
*/
private function assertDiagnostic($diagnostic, $message, $severity, $range)
{
$this->assertInstanceOf(Diagnostic::class, $diagnostic);
$this->assertEquals($message, $diagnostic->message);
$this->assertEquals($severity, $diagnostic->severity);
$this->assertEquals($range, $diagnostic->range);
}

public function testThisInStaticMethodProducesError()
{
$diagnostics = $this->collectDiagnostics(
__DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_method.php'
);

$this->assertCount(1, $diagnostics);
$this->assertDiagnostic(
$diagnostics[0],
'$this can not be used in static methods.',
DiagnosticSeverity::ERROR,
new Range(
new Position(6, 15),
new Position(6, 20)
)
);
}

public function testThisInFunctionProducesError()
{
$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.',
DiagnosticSeverity::ERROR,
new Range(
new Position(4, 11),
new Position(4, 16)
)
);
}

public function testThisInRoot()
{
$diagnostics = $this->collectDiagnostics(
__DIR__ . '/../../fixtures/diagnostics/errors/this_in_root.php'
);

$this->assertCount(1, $diagnostics);
$this->assertDiagnostic(
$diagnostics[0],
'$this can only be used in an object context.',
DiagnosticSeverity::ERROR,
new Range(
new Position(2, 5),
new Position(2, 10)
)
);
}

public function testThisInMethodProducesNoError()
{
$diagnostics = $this->collectDiagnostics(
__DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method.php'
);

$this->assertCount(0, $diagnostics);
}
}