-
-
Notifications
You must be signed in to change notification settings - Fork 352
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
review feature: Add a new method to add a method argument to a specific position. #1422
Conversation
add a test? |
Yep it's planned. |
Revapi Analysis resultsOld API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-20170623.143645-61 New API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-SNAPSHOT Detected changes: 1. Change 1
|
@@ -49,6 +49,12 @@ | |||
<C extends CtAbstractInvocation<T>> C addArgument(CtExpression<?> argument); | |||
|
|||
/** | |||
* Adds an argument expression to the invocation at the specified position | |||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shifts the element currently at that position (if any) and any subsequent elements to the right (adds one to their indices).
|
||
secondArg = launcher.getFactory().Core().createLiteral().setValue(12); | ||
|
||
invocBar.addArgument(1, secondArg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, this should fail with an exception because it's not compatible with the method signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not agree with you.
Spoon is not designed to check to validity of the transformation, it is not realistic to have always consistant model.
we can create a scanner that check the consistant of the model but it is not the responsibility of the setters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, this should fail with an exception because it's not compatible with the method signature.
If you do that, we should do it in all methods, especially if we create addParameter(int position, ...)
in CtExecutable: then you'll have an egg-chicken problem, if you use a transformation on the executable and on the invocations, there will be a "not compatible transformation" somewhere.
So I agree with Thomas: we should not do check there, but we should document the fact that those transformation are not checked against the executable.
Revapi Analysis resultsOld API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-20170625.224633-64 New API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-SNAPSHOT Detected changes: 1. Change 1
|
but it is not the responsibility of the setters.
I agree
we should document the fact that those transformation are not checked against the executable.
Yes, starting by the Javadoc of this method.
|
I changed the javadoc, but in fact we can do the same kind of warning for any |
Revapi Analysis resultsOld API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-20170625.224633-64 New API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-SNAPSHOT Detected changes: 1. Change 1
|
No description provided.