Phpinspectionsea: Unused constructor dependencies: static context must be inspected

Created on 24 Mar 2017  路  9Comments  路  Source: kalessil/phpinspectionsea

Find a bug.

  class UserInfo {

    private $name;

    public function __construct(string $name) {
      $this->name = $name; // Get error here
    }

    public static function getPrinter() {
      return function (UserInfo $userInfo) {
        return $userInfo->name;  // But property is used here.
      };
    }

  }

  $user = new UserInfo('funivan');
  $printer = UserInfo::getPrinter();
  echo $printer($user);

If we change public static function getPrinter() to public function getPrinter() then inspection works fine )

bug / false-positive fixed

All 9 comments

That's a very confusing design: the return (string)$userInfo should be used here (with __toString implementation of course).

I don't do this often, but I think we should leave as it is now but reflect this case and solution in docs.

Makes no difference, except private static function initPlaceholderFormatters() is private.
I need to think, handling this will complicate the inspector a lot...

@kalessil agree, this is very specific case. The most simple way to add documentation)

Why when you set it as non-static it works fine? Maybe some if() is causing this issue. Static should be allowed to work with cases like that.

@rentalhost : that's a trick (argh!!!) for accessing non-public properties:

  • the lambdas created inside the class, so they have access to all internals (incl. all private things);
  • the lambdas dispatched to an outer consumer, which calls the lambda without side-effects;

It's VERY dirty hacking, alternative is to use decorators

Wow, it's very hard to understand what this code does. After rerereading that I finally get. But still is hard, maybe because of this strange pattern (or antipattern, I don't know now, I am confused haha).

In all case, I guess that you could make it works on inspection.
PHP allows this kind of thing, after all.

In this case, I guess that only need get all anonymous functions on class that make references to own class (_write on line 111_), then treats like a common class method and check the property references. I guess that it should be done like (warning: pseudo-code):

Collections.addAll(methodsToCheck, constructor.getContainingClass().getOwnMethods());

for (PhpClass trait : constructor.getContainingClass().getTraits()) {
    Collections.addAll(methodsToCheck, trait.getOwnMethods());
}
// ^^^ current code. ^^^

// Check all collected methods to find anonymous functions.
for (Method methodToCheck: methodsToCheck) {
    Function[] anonymousFunctions = PSAPI.findAnonymousFunctions(methodToCheck);
    if (anonymousFunction.size() > 0) {
        // We found something.
        // Check if it refers to own class.
        for (Function anonymousFunction: anonymousFunctions) {
            Parameter[] functionParameters = anonymousFunction.getParameters();
            for (Parameter functionParameter = functionParameters) {
                if (functionParameter.getType() === clazz) {
                    // Once we found something, we can jump to next anonymous function iteration.
                    // Is possible that we have other references on it.
                    methodsToCheck.add(anonymousFunction);
                    continue 2;
                }
            }
        }
    }
}

// vvv current code. vvv

The approach from @rentalhost should be sufficient, I'll give it a try.

Fixed, I had only to remove static method checks - main logic is not changed.

Was this page helpful?
0 / 5 - 0 ratings