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

Conversation

MaartenStaa
Copy link
Contributor

Closes #476

Two scenarios produce errors, either $this is used in a static method (as described in the issue), or when it's used outside of a class method altogether, e.g.:

<?php

class A
{
    public static function foo()
    {
        return $this; // error here
    }

    public function bar()
    {
        return $this; // no error here (of course)
    }
}

function foo()
{
    return $this; // error here
}

echo $this; // error here

@codecov
Copy link

codecov bot commented Nov 15, 2017

Codecov Report

Merging #528 into master will increase coverage by 0.47%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #528      +/-   ##
============================================
+ Coverage     78.91%   79.38%   +0.47%     
- Complexity      832      837       +5     
============================================
  Files            56       56              
  Lines          1968     1979      +11     
============================================
+ Hits           1553     1571      +18     
+ Misses          415      408       -7
Impacted Files Coverage Δ Complexity Δ
src/TreeAnalyzer.php 94.64% <100%> (+0.58%) 54 <0> (+5) ⬆️
src/DefinitionResolver.php 84.05% <0%> (+1.12%) 302% <0%> (ø) ⬇️
src/Cache/FileSystemCache.php 27.27% <0%> (+4.54%) 8% <0%> (ø) ⬇️

@felixfbecker
Copy link
Owner

Could you add a test for this?

@jens1o
Copy link
Contributor

jens1o commented Nov 16, 2017

Should this be included in the parser instead?

@MaartenStaa
Copy link
Contributor Author

MaartenStaa commented Nov 16, 2017

@jens1o see #476, I asked the same question.

edit: as php -l itself does not produce a parse error either for this, it makes sense that the parser would not produce an error diagnostic.

Copy link
Owner

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome

@felixfbecker felixfbecker merged commit 4f672c2 into felixfbecker:master Nov 19, 2017
@MaartenStaa MaartenStaa deleted the error-on-this-in-static-context branch November 20, 2017 21:37
cgxxv pushed a commit to cgxxv/php-language-server that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants