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

False positives when using method_exists() with a static method #1267

Closed
josephzidell opened this issue Jul 22, 2018 · 27 comments · Fixed by phpstan/phpstan-src#1969
Closed
Labels
Milestone

Comments

@josephzidell
Copy link
Contributor

josephzidell commented Jul 22, 2018

Summary of a problem or a feature request

When using method_exists() with a static method, it is giving a false positive error. The PHP documentation clearly highlights this as a valid use-case.

Code snippet that reproduces the problem

https://phpstan.org/r/ddb2e9099834cf325a7917bcd50b42e2

Actual output

Call to an undefined static method Greetings::waveGoodbye().

Expected output

Nothing. Should have no errors

Similar to #323

@ondrejmirtes ondrejmirtes added this to the 0.10 milestone Jul 22, 2018
@ondrejmirtes
Copy link
Member

I've been thinking about this and I can't think of any use case for this - why isn't the method defined on Greetings? I don't think you can define it dynamically.

@josephzidell
Copy link
Contributor Author

josephzidell commented Jul 22, 2018 via email

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jul 22, 2018 via email

@josephzidell
Copy link
Contributor Author

Models in an MVC framework. A model can define this method, allowing special functionality. Any design pattern, really.

@ondrejmirtes
Copy link
Member

Can you show me some example code? I don' see how you can optionally define a static method. The method either exists or it doesn't

@josephzidell
Copy link
Contributor Author

I'm calling this from the parent class, and checking if the child class defined the method.

class Model
{
	public function checkForExcitingFunctionality()
	{
		if (method_exists(static::class, 'exciting')) {
			static::exciting();
		}
	}
}

// boring class
class ProductModel extends Model
{
}

// exciting class
class ImageModel extends Model
{
	public static function exciting()
	{
		echo 'Ooh hoo!!';
	}
}

@ondrejmirtes
Copy link
Member

I see. The example with static::class is different one than the one you gave first, and makes more sense. Thanks.

@josephzidell
Copy link
Contributor Author

Any update on this?

@szepeviktor
Copy link
Contributor

This could also happen in a trait.

if (method_exists(__CLASS__, 'getConstants')) {
    $statuses = self::getConstants('STATUS_');
}

@dereuromark
Copy link
Contributor

Any update?
We also face this issue for valid PHP code:

    protected function assertInitializeExists(string $widgetClassName): void
    {
        if (!method_exists($widgetClassName, 'initialize')) { ... }
    }

@ondrejmirtes
Copy link
Member

@dereuromark Can you post full example to phpstan.org? This one does not yield any errors.

@dereuromark
Copy link
Contributor

Maybe because of missing doc block? I will try to provide one later.

@dereuromark
Copy link
Contributor

    /**
     * @param string $widgetClassName
     *
     * @return void
     */
    protected function assertInitializeExists(string $widgetClassName)
    {
        if (!method_exists($widgetClassName, 'initialize')) {
            throw new \Exception(sprintf(
                'Widget %s needs to define and implement custom initialize() method with its custom widget input parameters.',
                $widgetClassName
            ));
        }
    }

is the exact method throwing that error

  Line   src/Spryker/Yves/Kernel/Widget/WidgetFactory.php                    
 ------ -------------------------------------------------------------------- 
  72     Call to function method_exists() with string and 'initialize' will  
         always evaluate to false.     

And PHP docs state it can be object or fqcn class name string, and this code works fine :)

@josephzidell
Copy link
Contributor Author

bump

@ondrejmirtes
Copy link
Member

@josephzidell What does "bump" mean? Either send a pull request with a fix, or wait. Issue is urgent when a lot of people have the same problem, not just because one guy says "bump".

@szepeviktor
Copy link
Contributor

send a pull request with a fix, or wait

Hard to decide!

@marcospassos
Copy link

Similar issue here:

// Call to function method_exists() with string and '__invoke' will always evaluate to false.
if (\method_exists($this->name, '__invoke')) {
}

@ondrejmirtes
Copy link
Member

There has been a fix of the method_exists error issue: phpstan/phpstan-src#12

But PHPStan will still complain about Call to an undefined static method Greetings::waveGoodbye(). inside the condition. Unfortunately there's not an easy way to represent this in the type system.

@josephzidell
Copy link
Contributor Author

I've moved on from PHP and am not following this anymore. @ondrejmirtes, should we keep this open for others?

@ondrejmirtes
Copy link
Member

Yes, please keep this open, a bug is still a bug.

@keichinger
Copy link

I may have found another example where PhpStan is complaining that a certain method doesn't exist, though my example may be a bit extreme 😅

https://phpstan.org/r/eed4b23c-b6e7-424f-87fa-9f88ac7150e3

In this particular case, I've got a very generic Trait that may or may not be used within a class-inheritance. The actual code works flawlessly, it's just that PhpStan is (rightfully) getting confused here :D

Without knowing the internals of PhpStan, I can imagine that fixing this particular case may be an extreme case of „This code is making sure that it's safe to call said method though parent:: doesn't know about it” 😅

@ondrejmirtes
Copy link
Member

@keichinger Your problem is unrelated, please open a separate issue.

@justlevine
Copy link

Exact same use case here.

Ive got an abstract class ObjectType, whose children can optionally implement various interfaces.

/**
 * Class - ObjectType
 */
abstract class ObjectType {
 	...
	 protected static function get_type_config() : array {
		$config = parent::get_type_config();

		$config['fields'] = static::get_fields();

		if ( method_exists( static::class, 'get_connections' ) ) { // children that implement TypeWithConnections.php
			$config['connections'] = static::get_connections();
		}

		if ( method_exists( static::class, 'get_interfaces' ) ) { // children that implement TypeWithInterfaces.php
			$config['interfaces'] = static::get_interfaces(); 
		}

		return $config;
	}
}

/**
 * Class - Post
 */
class  Post extends ObjectType implements TypeWithConnections {
	...
	public static function get_interfaces() : array {
		return [ 'some config array' ];
	}
}

@acrobat
Copy link

acrobat commented Jul 4, 2022

I've also a similar problem related to a BC layer needed in our library to support multiple versions of symfony. Although it's not related to static methods in this case

https://phpstan.org/r/c3ccc112-1178-415a-b3e6-dc0e67f8978e

@ondrejmirtes
Copy link
Member

@acrobat This makes sense: Call to an undefined method ExceptionEvent::getException(). - you're never checking for existence of that method.

@acrobat
Copy link

acrobat commented Jul 5, 2022

Hmm good point actually, nevermind 😅

@github-actions
Copy link

github-actions bot commented Dec 7, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants