nescohen commented on a change in pull request #1392:
URL: https://github.com/apache/groovy/pull/1392#discussion_r498608358
##########
File path:
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
##########
@@ -2592,7 +2592,7 @@ public void visitStaticMethodCallExpression(final
StaticMethodCallExpression cal
// in that order
List<Receiver<String>> receivers = new LinkedList<>();
addReceivers(receivers, makeOwnerList(new
ClassExpression(receiver)), false);
- List<MethodNode> mn = null;
+ List<MethodNode> mn = Collections.emptyList();
Receiver<String> chosenReceiver = null;
for (Receiver<String> currentReceiver : receivers) {
mn = findMethod(currentReceiver.getType(), name, args);
Review comment:
Hey Eric, sorry for the late response here. Lots to do at ApacheCon :P
I would say for this one that I can't say for 100% sure that receivers CAN
be empty, but there's a couple of reasons that I would argue setting mn to an
empty list is going to be better here.
1. `receivers` starts as an empty list, so even if there is no actual
codepath where it is empty now, will that be true in the future? An empty list
is still a valid value for that type so someone could easily refactor it to be
empty under some conditions.
2. By setting mn to an empty list we are ensuring that the mn.isEmpty() call
will not throw a NPE, rather it will trigger the appropriate exception of
`addNoMatchingMethodError`.
I'm not sure if I can make this call for you as I am relatively new to the
project, but I think this change will result in more resilient code, while
possibly fixing erroneous codepaths in the now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
