Check for unused parameter when generating factory #99
Conversation
| @@ -416,6 +416,8 @@ private function resolveImplement(ServiceDefinition $def, $name) | |||
| throw new ServiceCreationException("Type hint for \${$param->getName()} in $interface::$methodName() doesn't match type hint in $class constructor."); | |||
| } | |||
| $def->getFactory()->arguments[$arg->getPosition()] = self::literal('$' . $arg->getName()); | |||
| } elseif (!$def->getSetup()) { | |||
There was a problem hiding this comment.
I'm confused, how is setup of service relevant to it's constructor parameters and to parameters of create method of the factory interface?
There was a problem hiding this comment.
When implementing generated factory there are only two ways to use parameters in create method:
a) in constructor of created object.
b) in setup directives.
Or am I wrong?
There was a problem hiding this comment.
@fprochazka When the parameter from create method did not exist in the constructor it failed with no clear exception. I was trying to solve this when #60 was reported but my solution broke some tests - the parameter can be used in setup instead of the constructor. With this condition it should work fine to throw the exception.
There was a problem hiding this comment.
Exception message should be even better :) IMHO typo is in interface, not in constructor, because constructor is primary and interface secondary.
There was a problem hiding this comment.
I will try to implement "Did you mean" feature.
This would be ok?
Unused parameter $baz when implementing method Bad4::create(). Typo in Bad4::create() parameters? Did you mean $bar?
There was a problem hiding this comment.
What about: Unused parameter $baz when implementing method Bad4::create(). Did you mean $bar in Bad4?
Maybe more comples:
Unused parameter $baz when implementing method Bad4::create(). Did you mean Bad4::create($bar)?
There was a problem hiding this comment.
I am not sure, what to choose.
So, @dg, please choose one and I'll update commit.
|
Good job @EdaCZ. 👍 |
|
Improvement suggestion: Add "Did you mean ...?" feature to the exception message. |
|
@dg I've updated commit with "Did you mean feature". Is it ok now? |
|
Great! Thanks |
Check for unused parameter when generating factory
Purpose: throw an Exception when you have typo in the name of parameter in factory method (or constructor of factory product).
Related: #60