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

Bump minimum required version to 7.4.1+ #571

Closed
Ocramius opened this issue May 6, 2020 · 10 comments · Fixed by #623
Closed

Bump minimum required version to 7.4.1+ #571

Ocramius opened this issue May 6, 2020 · 10 comments · Fixed by #623
Assignees
Labels
dependencies Pull requests that update a dependency file enhancement
Milestone

Comments

@Ocramius
Copy link
Member

Ocramius commented May 6, 2020

No description provided.

@Ocramius Ocramius added enhancement dependencies Pull requests that update a dependency file labels May 6, 2020
@Ocramius Ocramius modified the milestones: 4.2.0, 4.3.0 May 6, 2020
@dereuromark
Copy link

For what reason?
There are many libraries that depend on a library that uses this one here. And those need to work still in php73 and some even php72. That would just prevent them from getting those bugfixes.

If there is no pressing reason it is always best for libraries to have a reasonable frame of versions supported.

@asgrim
Copy link
Member

asgrim commented May 20, 2020

For what reason?

The maintainers give their time and support for free, so if we feel we should bump the minimum PHP version, then we will 😄 Remember, because this is open source software (see https://github.com/Roave/BetterReflection/blob/master/LICENSE for details), you are more than free to fork it and support your own fork which maintains compatibility with PHP 7.2 ❤️ thanks!

@Ocramius
Copy link
Member Author

Few reasons to be added:

  • we want to be able to use newer tooling (typed properties are a big one here)
  • we want to maintain smaller dependency ranges to ensure maximum stability for a certain target release
  • we want downstream to get the best possible version we can deliver, and we work to our best effort when we work with the tools that we consider to be best
  • we do this for fun in our free time: might as well use what we want to work with

@kukulich
Copy link
Collaborator

You can assign me to this.

@Ocramius
Copy link
Member Author

@kukulich do we want to bump to 7.4.1+ with 4.3.0 or 4.4.0? I would probably release 4.3.0 within today (with the current fixes) and then bump from there, if you agree

@kukulich
Copy link
Collaborator

@Ocramius I have the same idea so I agree to release 4.3.0 with current PHP requirement :)

@ondrejmirtes Do you have any unreported bugs? :)

@ondrejmirtes
Copy link
Contributor

I plan to go through the commits to my fork and some classes I copied over to phpstan-src to see what remains.

  1. This big one is still open: AutoloadSourceLocator::locateClassByName can crash with "Class MemoizingParser not found" #573

  2. Another one (that I haven't reported yet) is that AutoloadSourceLocator doesn't support class aliases. This is how I initially implemented it: https://github.com/phpstan/phpstan-src/blob/c0cb3a69266d1530b5710016eb7eb471e03f9d02/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php I think it can no longer extend the abstract parent class. This part is important: phpstan/phpstan-src@c0cb3a6#diff-b84fb81e70db3cdb6b4495c80b475a44R110-R123

  3. One more thing that I recently did is that to match PHP behaviour, AutoloadSourceLocator also has to know about other symbols in an autoloaded file. Let's say that there's class A, function B and constant C in a file. If you ask for class A, PHP suddenly also knows about function B and constant C, which traditionally cannot be autoloaded, but PHP will know about them after class A is loaded. So what I'm doing is that AutoloadSourceLocator is memoizing all the found symbols in a file after reading it, and responds to them when asked about them later. It's a little bit undeterministic behaviour but I really wanted to match the behaviour 1:1. It's fine by me if you don't want it.

  4. Another thing that @kukulich plans to tackle is to parse @SInCE annotation and filter out stuff in the PhpStorm source stubber based on the given PHP version. But that can wait until later.

@Ocramius
Copy link
Member Author

Could you please report those as separate issues? That way, we can at least get work started/more eyes on it (if needed)

@ondrejmirtes
Copy link
Contributor

Yeah, of course, I now remembered a few more, mainly related to performance :)

@Ocramius Ocramius modified the milestones: 4.3.0, 4.4.0 May 27, 2020
@ondrejmirtes
Copy link
Contributor

I've reported all the issues I currently know about :) There's a lot of them but they make BetterReflection work in PHPStan really well :)

@Ocramius Ocramius self-assigned this May 28, 2020
Ocramius added a commit that referenced this issue May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants