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

AutoloadSourceLocator should know all symbols from a file once a class is autoloaded from that file #617

Open
ondrejmirtes opened this issue May 28, 2020 · 6 comments

Comments

@ondrejmirtes
Copy link
Contributor

Let's say we have a file that looks like this:

class A
{
}

function b()
{
}

const C = 1;

Once the PHP runtime autoloads the class A, it also knows about b() and C which traditionally cannot be autoloaded and wouldn't be discovered otherwise.

AutoloadSourceLocator should parse the file with A before creating the Reflection and when asked about b() and C, it should report them as known.

I understand if you don't want this change - it makes the behaviour non-deterministic - if the user asks first about b(), it will not be known. But I needed this to make the static reflection's behaviour as close to runtime reflection as possible.

PoC implementation: phpstan/phpstan-src@25c7791#diff-b84fb81e70db3cdb6b4495c80b475a44

@Ocramius
Copy link
Member

Isn't this already handled by looking at loaded files?

I don't understand what is not working as it currently is: specifically test scenario needed.

The AutoloadSourceLocator design only follos the autoloader behaviour, so I think this might be a new source locator based on a different principle?

@ondrejmirtes
Copy link
Contributor Author

Who is handling loaded files in your opinion? Do you mean the case when new \ReflectionFunction($functionName) succeeds and we can read getFileName() from it?

Nope, it's not that case - because the function isn't loaded in runtime.

I think AutoloadSourceLocator is a good place to do it because exactly it follows the autoloading principle, and these functions and constants are discovered only thanks to the autoloading of classes in the same file.

Test scenario is straightforward, you have:

  1. The file with the class and other symbols (https://github.com/phpstan/phpstan-src/blob/master/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/a.php)
  2. The test case: https://github.com/phpstan/phpstan-src/blob/master/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php

doFoo, SOME_CONSTANT, and ANOTHER_CONSTANT are correctly found only because I asked about AFoo first.

I'm not saying if it's a good idea or not, I just wanted to match exactly what PHP does. When AFoo is loaded by the autoloader, other symbols in the file are also known.

@Ocramius
Copy link
Member

The AutoloadSourceLocator (and all other locators) should only report what is "seen" during a lookup to a third party responsible of memoization and lookup heuristic/optimisation.

As mentioned here and elsewhere, there is a missing component that requires some architectural design.

Solving the problem in AutoloadSourceLocator still leaves it open for all other source locators too.

@ondrejmirtes
Copy link
Contributor Author

Yeah, centralization would make sense. Locators would be able to push all found symbols into it.

@Ocramius
Copy link
Member

Correct: something like "traverse and stop when found" with a registry that can be set up independently 👍

@WinterSilence
Copy link
Contributor

WinterSilence commented Jan 20, 2022

@ondrejmirtes it's unreal/wrong in case like as:
file A.php:

class A {}
const C = 1;

file B.php:

class B {}
const C = 2;

If we reflect class A and B, C = 1 or 2? or C declared in global namespace and it's wrong code?

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

No branches or pull requests

3 participants