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

Better typehint selection for methods used in several examples #1

Open
stof opened this issue Feb 6, 2014 · 2 comments
Open

Better typehint selection for methods used in several examples #1

stof opened this issue Feb 6, 2014 · 2 comments

Comments

@stof
Copy link

stof commented Feb 6, 2014

This continues the discussion started by @docteurklein in phpspec/phpspec#230 (comment) as he made a valid argument:

Take the following spec:

function it_does_something_with_a_token(ArrayToken $token)
{
    $token->getId()->willReturn(1234);

    $this->foo($token)->shouldReturn(1234);
}

function it_does_something_special_with_object_token(ObjectToken $token)
{
    $token->getId()->willReturn(null);

    $this->foo($token)->shouldReturn(3);
}

In this case, both examples will ask for the generation of foo. But applyignt he current logic will typehint it as ArrayToken or ObjectToken (depending of which example wins) and the other example will explode with a fatal error.
But the spec here allows you to know that none of these typehints are the expected one. In such case, it should look for all cases asking for generation, and find the common type (by looking at the common ancestor class in the inheritance hierarchy or a common interface)

@ciaranmcnulty
Copy link
Owner

There shouldn't be a scenario where multiple specs are asking for method generation (http://butunclebob.com/ArticleS.UncleBob.TheThreeRulesOfTdd).

I'd be interested to hear more how you'd approach this though, currently phpspec handles each exception sequentially in a listener so I can't quite see a way to get the sort of aggregated view you're talking about without hooking into and changing the Listener behaviour substantially.

@ciaranmcnulty
Copy link
Owner

I've thought more about this, we in fact would be able to handle this case but I'm not sure which would be a better approach:

a) Derive the common base class (if existing) and type hint that
b) Bail out completely and generate no type hints

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

No branches or pull requests

2 participants