Now, the calls to method reference are counted in the RFC metrics #58
Conversation
Codecov Report
@@ Coverage Diff @@
## master #58 +/- ##
============================================
- Coverage 75.16% 73.82% -1.34%
+ Complexity 933 887 -46
============================================
Files 42 40 -2
Lines 2158 2124 -34
Branches 256 252 -4
============================================
- Hits 1622 1568 -54
- Misses 388 412 +24
+ Partials 148 144 -4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks, @maykon-oliveira! I gave another review to the code, and I have a few extra comments!
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Counts the number of super methods invocations") | ||
| public void countSuperInvocations() { | ||
| CKClassResult a = report.get("rfc.GO3"); | ||
| Assertions.assertEquals(2, a.getRfc()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Oh, if it's returning 2, it's because we are now able to differentiate it. Maybe the new method name function made the trick.
This is an exceptional test case, as a() is not available, so JDT has to guess things. Suggestion: make the assertion to 2 again, and just rename it to methodCallsToAMethodNotInTheProject, or something like that.
| String method = JDTUtils.getMethodFullName(binding); | ||
| methodInvocations.add(method); | ||
| } | ||
|
|
||
| private String arguments(List<?> arguments) { |
There was a problem hiding this comment.
this method should disappear too!
| import org.eclipse.jdt.core.dom.*; | ||
|
|
||
| import java.util.HashSet; | ||
| import java.util.List; | ||
|
|
||
| public class RFC implements CKASTVisitor, ClassLevelMetric, MethodLevelMetric { | ||
|
|
||
| private HashSet<String> methodInvocations = new HashSet<String>(); | ||
| private final HashSet<String> methodInvocations = new HashSet<String>(); | ||
|
|
||
| public void visit(MethodInvocation node) { | ||
| IMethodBinding binding = node.resolveMethodBinding(); | ||
| count(node.getName() + "/" + arguments(node.arguments()), binding); |
There was a problem hiding this comment.
We can probably use the getMethodFullName here too!
There was a problem hiding this comment.
@mauricioaniche JDTUtils#getMethodFullName not accept null values.
public void visit(MethodInvocation node) {
IMethodBinding binding = node.resolveMethodBinding();
count(node.getName() + "/" + arguments(node.arguments()), binding);
}Here, sometimes binding is null, so I cannot get method full name using JDTUtils#getMethodFullName
JDTUtils#getMethodFullName not accepts MethodInvocation type as argument.
binding is always null when method not belongs to class.
visit(ExpressionMethodReference node)have same behavior
There was a problem hiding this comment.
so, I think you should deal with the null case here, instead of duplicating code.
This is all about getting the name of the method, right? So, suggestion:
IMethodBinding binding = node.resolveMethodBinding();
String methodName = binding!=null ? JDTUtils.getMethodFullName(binding) : JDTUtils.getMethodFullName(node);
What do you think?
There was a problem hiding this comment.
I was thinking about this.
I can to create a overload of JDTUtils#getMethodFullName that accepts MethodInvocation and ExpressionMethodReference types. And we can get rid of duplicating code.
There was a problem hiding this comment.
Makes sense to me! If you need any help, let me know!
…ature/method-reference-rfc
|
Hm, something went wrong |
|
@maykon-oliveira shall I close this one and we reopen when you get back to this ? |
|
Yes, ok |
I disabled one test method, bc no works well.
(@mauricioaniche ) wrote in README.md that RFC fails when a method has overloads with same number of parameters, but different types.
So I disabled this test case, it should be 2, not 1, but I go to improve RFC metric.