Allow to double Throwable, or an interface that extends Throwable by ciaranmcnulty · Pull Request #412 · phpspec/prophecy

ciaranmcnulty · 2018-07-30T16:04:42Z

The use case is explained in #410

When doubling something like the following:

interface VendorProblem extends Throwable {}

Prophecy was trying to generate something like:

class DoubleOfFoo extends stdClass implements MyTraversable

This is not allowed by PHP. To fix, there is a ClassPatch that sets the class being extended to Exception instead of stdClass

… setting the doubled class to Exception

stof · 2018-07-30T16:07:27Z

src/Prophecy/Doubler/ClassPatch/ThrowablePatch.php

+     * @param string $type
+     * @return bool
+     */
+    private function isThrowable($type)


Do we really need that many private methods ?

I tend to write this way but if you don't like it I can combine a few of them? Maybe it'll look better without the unnecessary docblocks

stof · 2018-07-30T16:07:48Z

src/Prophecy/Doubler/ClassPatch/ThrowablePatch.php

+    private function checkItCanBeDoubled(ClassNode $node)
+    {
+        $className = $node->getParentClass();
+        if ($className != 'stdClass') {


should use strict comparison

stof · 2018-07-30T16:08:02Z

src/Prophecy/Doubler/ClassPatch/ThrowablePatch.php

+    }
+
+    /**
+     * @param ClassNode $node


useless phpdoc. It duplicates the typehint

Wasn't sure what the project policy was :-) will strip a few out in other places too then

stof · 2018-07-30T16:19:04Z

Tests are failing on PHP 5.x. The specs need to be skipped on 5.x when they expect the Throwable interface to exist

dkarlovi · 2018-07-31T07:26:56Z

fixtures/ThrowableInterface.php

+
+interface ThrowableInterface extends \Throwable
+{
+}


Newline missing.

dkarlovi · 2018-07-31T07:27:12Z

spec/Prophecy/Doubler/ClassPatch/ThrowablePatchSpec.php

+
+        $this->shouldThrow('Prophecy\Exception\Doubler\ClassCreatorException')->duringApply($node);
+    }
+}


Newline missing.

dkarlovi · 2018-07-31T07:28:41Z

Hopefully, this allows symfony/symfony#26702 to be re-applied. /cc @dunglas @nicolas-grekas

ciaranmcnulty · 2018-07-31T09:03:07Z

@stof can you take another look please?

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: #26702 #27420 #27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

@dunglas

This PR was merged into the 4.2-dev branch. Discussion ---------- Mark ExceptionInterfaces throwable #2 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This has been reverted in beta of 4.1 because of lack of support in prophecy, which has been fixed since then (incl. release). Can be merged again. References: symfony/symfony#26702 symfony/symfony#27420 symfony/symfony#27419 phpspec/prophecy#412 ping @dunglas @ciaranmcnulty @dkarlovi @Wirone @teohhanhui @stof @nicolas-grekas @ondrejmirtes Commits ------- 17c3675226 Mark ExceptionInterfaces throwable

Allow to double Throwable, or an interface that extends Throwable, by…

ba64baf

… setting the doubled class to Exception

stof reviewed Jul 30, 2018

View reviewed changes

ciaranmcnulty added 2 commits July 30, 2018 17:13

Be stricter about comparison

61ae33c

Remove a level of calls in ThrowablePatch

83c02a6

Skip specs if on PHP 5

733b5d0

ciaranmcnulty force-pushed the bugfix/doubling-throwable branch from fe39721 to 733b5d0 Compare July 30, 2018 16:41

dkarlovi reviewed Jul 31, 2018

View reviewed changes

dkarlovi mentioned this pull request Jul 31, 2018

Making ExtensionInterfaces extending \Throwable is a BC break symfony/symfony#27419

Closed

Missing newlines

d09fbb4

dunglas mentioned this pull request Jul 31, 2018

Mark ExceptionInterfaces throwable symfony/symfony#26702

Merged

ciaranmcnulty merged commit e5a9420 into phpspec:master Aug 5, 2018

Taluu mentioned this pull request Aug 5, 2018

Cannot prophesize and interface extending \Throwable #410

Open

ostrolucky mentioned this pull request Aug 29, 2018

Mark ExceptionInterfaces throwable #2 symfony/symfony#28307

Merged

Jean85 added a commit to Jean85/client-common that referenced this pull request Oct 31, 2018

Require at least phpspec/prophecy 1.8 due to phpspec/prophecy#412

2dac8f0

Jean85 added a commit to Jean85/client-common that referenced this pull request Oct 31, 2018

Require at least phpspec/prophecy 1.8 due to phpspec/prophecy#412

22c3573

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to double Throwable, or an interface that extends Throwable#412

Allow to double Throwable, or an interface that extends Throwable#412
ciaranmcnulty merged 5 commits intophpspec:masterfrom
ciaranmcnulty:bugfix/doubling-throwable

ciaranmcnulty commented Jul 30, 2018

Uh oh!

stof Jul 30, 2018

Uh oh!

ciaranmcnulty Jul 30, 2018

Uh oh!

stof Jul 30, 2018

Uh oh!

stof Jul 30, 2018

Uh oh!

ciaranmcnulty Jul 30, 2018

Uh oh!

stof commented Jul 30, 2018

Uh oh!

dkarlovi Jul 31, 2018

Uh oh!

dkarlovi Jul 31, 2018

Uh oh!

dkarlovi commented Jul 31, 2018

Uh oh!

ciaranmcnulty commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ciaranmcnulty commented Jul 30, 2018

Uh oh!

stof Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

ciaranmcnulty Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

stof Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

stof Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

ciaranmcnulty Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

stof commented Jul 30, 2018

Uh oh!

dkarlovi Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

dkarlovi Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

dkarlovi commented Jul 31, 2018

Uh oh!

ciaranmcnulty commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants