Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improvements to Type Hinting and Object Model #117

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

g105b
Copy link

@g105b g105b commented Feb 15, 2022

This PR fixes #89 - IDEs will not complain about the use of type hints or object inheritance any more.

I have used PHPStan set to level 5 checks to expose any ambiguous/incorrect uses of type hinting. My IDE is happy now!

In order to run the unit tests, PHPUnit is a hard dev
dependency, so I've included it in this commit, and now
I can run the unit tests as part of this PR.
This is for willdurand#89 - to ensure correct type hints are provided
to developers who use IDEs.
Fixes willdurand#89 - IDEs and PHPStan are happy with this implementation
* Depend on PHPUnit for development

In order to run the unit tests, PHPUnit is a hard dev
dependency, so I've included it in this commit, and now
I can run the unit tests as part of this PR.

* Depend on PHPStan for development

This is for willdurand#89 - to ensure correct type hints are provided
to developers who use IDEs.

* Fix object model of AcceptHeader interface

Fixes willdurand#89 - IDEs and PHPStan are happy with this implementation

* Correct return type

* Correct nonexistent Priority class to AcceptHeader

* Improve typehint - allow looser type to be returned

* Improve typehint - more accurate types as parameters

* Improve typehint - more accurate generics as parameters

* Expose script property - was only ever written

* Properly typehint associative array

* Typehint nullable string

* Match typehints of parent method

* Add PHPStan to CI

* Configure PHPUnit versions for different PHP runtimes

* Use real phpunit
@nreynis
Copy link

nreynis commented Mar 14, 2022

You should split this: it really is 3 PR in one:

  • Adding static analysis tooling
  • Exposing script
  • Changing inheritance organization

All those are independent concerns, by tying them all together in one PR, you take the risk that none ever get fixed.

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

Successfully merging this pull request may close these issues.

Negotiator::getBest() return type hint?
2 participants