adds null protection in case of classes have no package. #12
Conversation
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
|
|
||
| final String name = aPackage.getName(); | ||
|
|
||
| if (name == null) { |
|
🐛 forgot null protection in several other places, such as Recommend adding a regression test. Should be easy. |
|
I know that there are other places, in fact I try to touch as less lines as possible and I fixes the problem I had now. But now that you mention this I will fix as well. |
|
@jglick now I am not sure what you mean by doing a regression test for checking nulls, it comes to my mind a unit test but nothing more |
Well, you hit an NPE somehow, obviously. So you use |
| @@ -163,7 +171,17 @@ public boolean permitsMethod(Method method, Object receiver, Object[] args) { | |||
|
|
|||
| @Override | |||
| public boolean permitsConstructor(@Nonnull Constructor<?> constructor, @Nonnull Object[] args) { | |||
| return constructor.getDeclaringClass().getPackage().getName().equals(ORG_APACHE_MAVEN_MODEL); | |||
| if (constructor == null) { | |||
There was a problem hiding this comment.
🐜 obviously not possible, given the annotation.
|
🐝 |
|
Looks good, just needs a test or two. |
|
The problem is that I dont know why the NPE was thrown with a simple read,
|
|
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
|
@rsandell I am trying to reproduce in a simple test the error that a package is null, but it is not possible,I have tried several things like executing code from external groovy script, using |
|
ok, thank you for trying. But as Jesse said, you did get the exception somehow in your own pipeline scripts, so it should be reproducible :). |
adds null protection in case of classes have no package.
@reviewbybees