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

@psalm-internal must not propagate to private functions #9584

Open
scapital opened this issue Mar 30, 2023 · 7 comments
Open

@psalm-internal must not propagate to private functions #9584

scapital opened this issue Mar 30, 2023 · 7 comments

Comments

@scapital
Copy link

https://psalm.dev/r/0657193833

As W\Worker::internalRun is private, so it can not be used not in W\Worker.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/0657193833
<?php

namespace W {
    /**
     * @psalm-internal C\Consumer
     */
    class Worker
    {
        public function run(): int
        {
            return $this->internalRun();
        }
        
        private function internalRun(): int
        {
            return 1;
        }
    }
}

namespace C {
    use W\Worker;

    class Consumer
    {
        public function __construct(
            private Worker $worker,
        ) {
        }
        
        public function run(): void
        {
            $this->worker->run();
        }
    }
}
Psalm output (using commit 8b9ad1e):

ERROR: InternalMethod - 11:27 - The method W\Worker::internalRun is internal to C\Consumer and C\Consumer but called from W\Worker::run

@kkmuffme
Copy link
Contributor

kkmuffme commented Mar 27, 2024

It's possible to extend the class and then it's called from a different namespace, e.g. https://psalm.dev/r/4e660ece0a - therefore I think it does make sense and this behavior should not be changed.

Copy link

psalm-github-bot bot commented Mar 27, 2024

I found these snippets:

https://psalm.dev/r/4e660ece0a
<?php

namespace W {
    /**
     * @psalm-internal C\Consumer
     */
    class Worker
    {
        public function run(): int
        {
            return $this->internalRun();
        }
        
        private function internalRun(): int
        {
            return 1;
        }
    }
}

namespace C {
    use W\Worker;

    class Consumer
    {
        public function __construct(
            private Worker $worker,
        ) {
        }
        
        public function run(): void
        {
            $this->worker->run();
        }
    }
}

namespace D {
    use W\Worker;

    class NewWorker extends Worker
    {
        private function internalRun(): int
        {
            return 1;
        }
    }
}
Psalm output (using commit ef3b018):

ERROR: InternalMethod - 11:27 - The method W\Worker::internalRun is internal to C\Consumer but called from W\Worker::run

ERROR: InternalClass - 41:29 - W\Worker is internal to C\Consumer but called from D\NewWorker

@scapital
Copy link
Author

@kkmuffme

It's possible to extend the class and then it's called from a different namespace, e.g. https://psalm.dev/r/4e660ece0a

If class is internal to some namespace/class, there are not ability to extend it in another namespace, your example has error

W\Worker is internal to C\Consumer but called from D\NewWorker

and that's ok.

therefore I think it does make sense and this behavior should not be changed

Do you mean, it is ok, if some class is marked as internal, it does not has ability to call his own private methods?

Copy link

I found these snippets:

https://psalm.dev/r/4e660ece0a
<?php

namespace W {
    /**
     * @psalm-internal C\Consumer
     */
    class Worker
    {
        public function run(): int
        {
            return $this->internalRun();
        }
        
        private function internalRun(): int
        {
            return 1;
        }
    }
}

namespace C {
    use W\Worker;

    class Consumer
    {
        public function __construct(
            private Worker $worker,
        ) {
        }
        
        public function run(): void
        {
            $this->worker->run();
        }
    }
}

namespace D {
    use W\Worker;

    class NewWorker extends Worker
    {
        private function internalRun(): int
        {
            return 1;
        }
    }
}
Psalm output (using commit 08afc45):

ERROR: InternalMethod - 11:27 - The method W\Worker::internalRun is internal to C\Consumer but called from W\Worker::run

ERROR: InternalClass - 41:29 - W\Worker is internal to C\Consumer but called from D\NewWorker

@kkmuffme
Copy link
Contributor

Do you mean, it is ok, if some class is marked as internal, it does not has ability to call his own private methods?

If it's marked internal to another namespace than itself yes - this basically means that it should not have any private methods in the first place.

It's already possible to get the behavior you want, by putting a 2nd @psalm-internal of the current class. This is more transparent (since it's explicitly declared):

    /**
     * @psalm-internal C\Consumer
     * @psalm-internal W\Worker
     */

https://psalm.dev/r/bea7a0b338

I don't think changing this behavior would be a net-positive, as it would change the behavior intransparently unnecessarily, since the desired behavior can already be achieved with currently available annotations in a transparent way.

Copy link

I found these snippets:

https://psalm.dev/r/bea7a0b338
<?php

namespace W {
    /**
     * @psalm-internal C\Consumer
     * @psalm-internal W\Worker
     */
    class Worker
    {
        public function run(): int
        {
            return $this->internalRun();
        }
        
        private function internalRun(): int
        {
            return 1;
        }
    }
}

namespace C {
    use W\Worker;

    class Consumer
    {
        public function __construct(
            private Worker $worker,
        ) {
        }
        
        public function run(): void
        {
            $this->worker->run();
        }
    }
}
Psalm output (using commit 08afc45):

No issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants