Skip to content

Refactor to support strong typing. - #42

Merged
florianeckerstorfer merged 1 commit into
cocur:masterfrom
nreynis:feature/strong-typing
Sep 15, 2019
Merged

Refactor to support strong typing.#42
florianeckerstorfer merged 1 commit into
cocur:masterfrom
nreynis:feature/strong-typing

Conversation

@nreynis

@nreynis nreynis commented Sep 14, 2019

Copy link
Copy Markdown
Contributor

I've added strong typing everywhere it was possible.
I also:

  • removed all magic strings for FQCN
  • fixed namespaces in tests comment doc (phpStorm is now happy and successfully identify the object types)
  • I initially did the same thing for @covers annotations but reverted it because the FQCN is required

Adding strong typing made me spot two bugs that I fixed in the meantime:

  • splice wasn't returning this and therefore wasn't fluent (also added doc for this one)
  • test for sortKeysWithDefaultSortingAndReverseOption had a bug, you were passing null assuming it would take the default value. This is not how PHP handles that, default values are provided only if you pass nothing at all. Passing an explicit null will give you a null (now a TypeError as the param isn't nullable). As you were passing the second argument you've got no choice: you're forced to pass the first.

@nreynis

nreynis commented Sep 14, 2019

Copy link
Copy Markdown
Contributor Author

Seems like getFluentTypeErrorForMockedTrait isn't robust enough to pass on lower versions of phpUnit. I'll fix that this weekend.

@nreynis
nreynis force-pushed the feature/strong-typing branch from 3f5fd00 to f50fbdc Compare September 14, 2019 20:29
@nreynis

nreynis commented Sep 14, 2019

Copy link
Copy Markdown
Contributor Author

Fixed tests and squashed commits

@nreynis
nreynis force-pushed the feature/strong-typing branch from f50fbdc to cc51fdd Compare September 14, 2019 20:40
More reliable pattern for TypeError detection
@nreynis
nreynis force-pushed the feature/strong-typing branch from cc51fdd to 0ab5c8c Compare September 14, 2019 20:46
@florianeckerstorfer

Copy link
Copy Markdown
Member

Awesome, thank you so much for the work

@florianeckerstorfer
florianeckerstorfer merged commit 7736894 into cocur:master Sep 15, 2019
@nreynis

nreynis commented Sep 16, 2019

Copy link
Copy Markdown
Contributor Author

You're welcome.

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

Successfully merging this pull request may close these issues.

2 participants