Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Tyson Andre
    @TysonAndre
    You'd have to write a plugin for it (AnalyzeFunctionCallCapability would help). tool/pdep is somewhat related, but not what you wanted - it dumps a graph of the dependencies of classes on other classes. Dumping method/property relationships might be within the scope of what pdep could eventually do, but I'm not the owner of pdep
    Tyson Andre
    @TysonAndre
    A more efficient approach would be enable the reference tracking setting or dead code detection and to use the references to classes to build the graph. That would get you everything except internal elements such as strlen, etc
        /**
         * @override
         */
        public function addReference(FileRef $file_ref): void
        {
            if (Config::get_track_references()) {
                // Currently, we don't need to track references to PHP-internal methods/functions/constants
                // such as PHP_VERSION, strlen(), Closure::bind(), etc.
                // This may change in the future.
                if ($this->isPHPInternal()) {
                    return;
                }
                if ($file_ref instanceof Context && $file_ref->isInFunctionLikeScope() && $file_ref->getFunctionLikeFQSEN() === $this->fqsen) {
                    // Don't track functions calling themselves
                    return;
                }
                $this->reference_list[$file_ref->__toString()] = $file_ref;
            }
        }
    Tyson Andre
    @TysonAndre

    I asked about pdep, but I doubt it's going to get implemented any time soon.

    Also, it wouldn't track calls to functions from themselves, etc., because this is focused on dead code detection

    Ace Pace
    @acepace
    awesome ty
    Ace Pace
    @acepace
    @TysonAndre so I've been busy and got a lot of my plugin working

    I noticed phan doesn't know how to parse callbacks of the form
    call_user_func(array('B', 'parent::who'));
    Where B is a class extending class A both defining Who.

    any idea how I can hack this in?

    or less hack, more properly :)
    Tyson Andre
    @TysonAndre
    @acepace phan/phan#1854 - See ArrayShapeType::asFunctionInterfaceOrNull, possibly ContextNode->getFunctionLikeFromDynamicExpression, and possibly CallableParamPlugin
    There's hundreds of open issues and it was low priority relative to them to support that syntax. I'm open to PRs with tests
    Ace Pace
    @acepace
    thanks!
    Patrick Allaert
    @patrickallaert

    Hello,

    With the method below:

    private function getDocumentRoot(): string
    {
        return realpath(__DIR__ . "/web/");
    }

    Phan returns the following error:

    PhanPossiblyFalseTypeReturn Returning realpath((__DIR__ . '/web/')) of type false|string but getDocumentRoot() is declared to return string (false is incompatible)

    It is an entirely valid error: realpath() can return string or false, while getDocumentRoot() is declared to return a string exclusively.

    My question is: how to address this issue in the state of the art?

    The options I thought about are the following:

    1) Specific Exception:

    private function getDocumentRoot(): string
    {
        $result = realpath(__DIR__ . "/web/");
    
        if (!is_string($result)) {
            throw new \RuntimeException("Cannot compute document root");
        }
    
        return $result;
    }

    While this makes the Phan error dissappear, it just changes the way to catch the exception from \TypeError to \RuntimeException. Somehow it feels non natural to remap the \TypeError to something else, this leads to the next option:

    2) Throwing \TypeError manually:

    private function getDocumentRoot(): string
    {
        $result = realpath(__DIR__ . "/web/");
    
        if (!is_string($result)) {
            throw new \TypeError("Cannot compute document root");
        }
    
        return $result;
    }

    As option 1), it "solves" the reported issue, however it re-implements the natural behaviour of PHP itself: it really feels like boilerplate code.

    3) Acknowledging that a TypeError can occur:

    /**
     * @throws \TypeError
     */
    private function getDocumentRoot(): string
    {
        return realpath(__DIR__ . "/web/");
    }

    While potentially no additional code involved, it could "mark" that a \TypeError is possible and should not be reported as something not handled. However, using @throws does not make the issue dissappear.

    4) Suppressing the issue locally:

    /**
     * @suppress PhanPossiblyFalseTypeReturn
     */
    private function getDocumentRoot(): string
    {
        return realpath(__DIR__ . "/web/");
    }

    This is supressing the issue, however, I feel that @suppress is more like a workaround to tackle edge cases while this seems pretty common. It also feels more like silencing it over declaring an intention.

    I'd like to know how you usually deal with that situation and what the best practices are.

    Tyson Andre
    @TysonAndre

    While potentially no additional code involved, it could "mark" that a \TypeError is possible and should not be reported as something not handled. However, using @throws does not make the issue dissappear.

    I don't plan to implement any support for making @throws hide phan errors/notices - In general, in code being analyzed, it's also possible to throw a TypeError explicitly. It's also unclear what @throws Error and @throws Throwable would/should do since they're parent types.

    Again, depending on the specifics of the situation, I've implemented 1/2/4 in the past to work around notices. To add, I'd personally change 4. to add a description such as @suppress PhanPossiblyFalseTypeReturn in abnormal situtations where PHP can't find the directory or the directory was removed, this will throw a TypeError, and there's no great way to handle this (or bad entries in the realpath cache, or nfs wierdness)

    You could also add a helper function/method called realpathOrThrow to a file/class with type-safe utilities - that's what Phan does for StringUtil::jsonEncode()
    Patrick Allaert
    @patrickallaert

    To add, I'd personally change 4. to add a description such as @suppress PhanPossiblyFalseTypeReturn in abnormal situtations where PHP can't find the directory or the directory was removed, this will throw a TypeError, and there's no great way to handle this (or bad entries in the realpath cache, or nfs wierdness)

    I also opted for 4), the extra comment is indeed probably worth it!

    Thank you @TysonAndre for your opinion. I also opted for option 4) as being the most practical, the extra comment can indeed make it more clear!
    I wanted to be sure I wasn't missing another option.
    Patrick Allaert
    @patrickallaert

    Hi!

    FYI, I filled an issue on PHPStorm's tracker: https://youtrack.jetbrains.com/issue/WI-54573
    It is about the lack of understanding:

    /**
     * @return iterable<Foo>
     */

    The IDE doesn't understand that syntax: looping over the results, it doesn't understand that the inner variable's type is Foo, same with: Generics<Foo>.

    Does anyone have a workaround in the mean time? For now it means commenting with /** @var Foo $item */ every loops, but that is duplication and I hate those markers :-(

    Tyson Andre
    @TysonAndre
    1. PHPStorm supports Generator|Foo[] syntax, which phan doesn't because it uses Generator<Foo> - so you can use @return Generator|Foo[] and @phan-return Generator<Foo>, but that's another type of repetition
    2. You could suppress PHPStorm's warnings, but that doesn't work well on projects/files that don't have other static analyzers set up and phpstorm may have its own checks you'd wish to keep using. I forget how that's done
    3. I'm not sure what PHPStorm's plans are for templates in general, and haven't seen anything in their announcements. I don't work on it and primarily use Vim.
    Patrick Allaert
    @patrickallaert
    Thank you for your answer @TysonAndre ! I dislike Generator|Foo[] because that syntax implies that a native array of Foo could possibly be returned. At least I see all alternatives and didn't think about combining @return and @phan-return at the same time! Thanks again!
    Adrian
    @adrian-enspired
    how would i annotate an array of arrays of a type? is there something like array[type[]] or do i just stick with array[] and document elsewhere?
    Adrian
    @adrian-enspired

    also saw an error i don't understand:

    PhanTypeInvalidRightOperandOfBitwiseOp Invalid operator: right operand of & is bool (expected int|string)

    the line in question is like $x & $key, where $key is an array key (so int|string) and is also asserted to be an int. any idea why phan thinks it's bool?

    Phan 3.2.0
    php-ast version 1.0.6
    PHP version used to run Phan: 7.4.3
    Tyson Andre
    @TysonAndre
    There's many ways to write arrays in union types. See https://github.com/phan/phan/wiki/About-Union-Types. type[][] if all keys are unknown, there's also more elaborate types such as list<array<string, type>>
    I can't tell you why without a self-contained snippet. Maybe it thinks it's an iterable (e.g. not enough information to infer array), so $x is initially the empty type, and there was code treating $x like a boolean prior to that
    Or even the file in question. It's possible the union type caching is the issue, but more likely it's something about the code in question
    Adrian
    @adrian-enspired

    thanks for the link! i knew there was something for it... should have looked harder.

    regarding the PhanTypeInvalidRightOperandOfBitwiseOp question, this reproduces it for me

    function example(int $a, array $bs) {
      foreach ($bs as $b => $s) {
        $x = ($a & $b === $a);
      }
    }
    i can add assert(is_int($b)) before the op and it still errors.
    Tyson Andre
    @TysonAndre
    Operator precedence of & and === is a common source of confusion. That gets parsed as $a & ($b === $a).
    phan ±0198a2479⚡ » php internal/dump_fallback_ast.php --php-ast-native '$x = ($a & $b === $a);'
    AST_STMT_LIST [] #1
            0 => AST_ASSIGN [] #1
                    var => AST_VAR [] #1
                            name => x
                    expr => AST_BINARY_OP [BINARY_BITWISE_AND] #1
                            left => AST_VAR [] #1
                                    name => a
                            right => AST_BINARY_OP [BINARY_IS_IDENTICAL] #1
                                    left => AST_VAR [] #1
                                            name => b
                                    right => AST_VAR [] #1
                                            name => a
    So your code is buggy and should be $x = ($a & $b) === $a;
    PHP bases the precedence on C, which is the same for those operators
    Adrian
    @adrian-enspired
    oh holy cow : [ i even had the parenthesis on a similar statement in another function.
    thanks. sorry to bother.
    Tyson Andre
    @TysonAndre
    exclude_file_regex should be @(^(\./)?src/)@ since it's probably checking against ./src/
    Vladimir Belozyorov
    @VBelozyorov
    Thank you! phan -D output has confused me a little bit
    Adrian
    @adrian-enspired

    phan 2.7.3 is emittingPhanEmptyFQSENInClasslike with code like

    class C {
      /** @var string Fully Qualified Classname. */
      const FQCN = '';
    
      public function f() {
        if (! empty(static::FQCN)) {
          $fqcn = static::FQCN;
          new $fqcn();
        }
      }
    }

    shouldn't it be obvious to phan that $fqcn is a non-empty string?

    Tyson Andre
    @TysonAndre
    phan/phan#2495 is similar but not the cause - Phan currently doesn't infer anything about constants, class constants, function_exists(), etc in the local scope. Just variables, $this->prop, SomeClass::$static_prop, but not $other->prop
    The 2.x release line isn't supported, but 3.2.6 would have the same false positive
    Filed phan/phan#4286 to track the suggestion
    Adrian
    @adrian-enspired
    trait T { 
      /**
       * @phan-suppress PhanUndeclaredConstantOfClass
       */
      public function f() { echo static::X; }
    }
    
    class C { use T; }
    is there a way I can suppress the warning when analyzing the trait, but still see it when analyzing the class that uses it?
    Tyson Andre
    @TysonAndre

    Currently, no - in abstract classes declaring a class constant with /** @phan-abstract */ public const X = ''; would help detect classes that didn't override the constant, but that doesn't help in your use case because traits can't have class constants.

    • I think you might be able to do something with @mixin for a stub interface (e.g. .phan/stubs) with a abstract class constant but I don't know if that even works now and wouldn't rely on it even if it did work.
    • Phan only analyzes trait methods on the declaring trait right now, and there's no way to suppress something only on the declaring context

    There were discussions of allowing traits to have class constants or require implementing interfaces in php itself in some future php version

    Abstract methods would be how I typically would work around that, or to just keep suppressing it
    Eddie Kohler
    @kohler
    Hi all! Thanks for phan. Question: Is there a way to mark a class that inherits from, say, IteratorAggregate as being a kind of Iterator<T> for a specific T? For example, in the following code, I’d like Phan to emit a deprecation error on lines 19 and 26, but it only does so on line 26.
    <?php
    class X {
        /** @deprecated */
        public $x;
    }
    
    class Y implements IteratorAggregate {
        /** @var list<X> */
        public $xs;
        /** @return Iterator<X> */
        function getIterator() {
            return new ArrayIterator($this->xs);
        }
    }
    
    /** @param Y $y */
    function f1($y) {
        foreach ($y as $x) {
            echo $x->x;
        }
    }
    
    /** @param Y|Iterator<X> $y */
    function f2($y) {
        foreach ($y as $x) {
            echo $x->x;
        }
    }
    Tyson Andre
    @TysonAndre
    I initially thought that was a special case of phan/phan#824 or phan/phan#3238 but it wasn't - it's now fixed for that particular case in phan/phan@716c242 (see previous issues for remaining work)
    Adrian
    @adrian-enspired
    should covariant return types be expected to work with phan ^4.0 ? I found phan/phan#3795 but unclear if there was any movement on it.
    Adrian
    @adrian-enspired
    also seems there's a setting allow_method_param_type_widening but nothing like allow_method_return_type_narrowing
    Vladimir Belozyorov
    @VBelozyorov

    Hello! I have a question about

    input:22: PhanTypeNoPropertiesForeach Class \ActiveRecord was passed to foreach, but it does not extend Traversable and doesn't have any declared properties. (This check excludes dynamic properties)

    In this snippet

    If I remove ActiveRecord from union type on line 5, why Phan doesn't warn about passing null|int|string|bool to foreach? Why only non-traversable object instance bothers it?

    As you already guessed, in real code return type of that find() method depends on its arguments. Can I help Phan to define type in each case more precisely except of add exclusive @phan-var for each call of find()?

    Tyson Andre
    @TysonAndre

    PhanTypeMismatchForeach detects the case where there are no valid types, but if some of the types are valid, it doesn't warn. I don't believe that --strict-type-checking or similar options currently cover array|null - there's probably hundreds of issues for which a warning about being partially invalid could start being emitted

    https://github.com/phan/phan#readme

    Phan is a static analyzer for PHP that prefers to minimize false-positives.

    Vladimir Belozyorov
    @VBelozyorov
    But it warns about PhanTypeNoPropertiesForeachin the initial snippet while there is valid ActiveRecord[] in that union type, is it intentional?