-
-
Notifications
You must be signed in to change notification settings - Fork 793
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
Fix false-positive on ExplicitCollectionElementAccessMethod #4400
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4400 +/- ##
============================================
- Coverage 84.35% 84.35% -0.01%
- Complexity 3277 3279 +2
============================================
Files 473 473
Lines 10357 10361 +4
Branches 1827 1829 +2
============================================
+ Hits 8737 8740 +3
Misses 668 668
- Partials 952 953 +1
Continue to review full report at Codecov.
|
cf19942
to
27e9a7a
Compare
when { | ||
function == null -> false | ||
!function.isOperator -> false | ||
else -> !(function.isFromJava && function.valueParameters.size > 2) |
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.
Here you check for the parameter list size to be bigger than 2, but in the test case underneath you assert the following.
does not report if the function has more than 3 arguments and it's defined in java
What system behavior can the user of this rule expect in the mentioned case?
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.
Here you check for the parameter list size to be bigger than 2, but in the test case underneath you assert the following.
does not report if the function has more than 3 arguments and it's defined in java
I fixed the description of the test to:
does not report if the function has 3 or more arguments and it's defined in java
What system behavior can the user of this rule expect in the mentioned case?
(I'm not sure if I understood what you are asking) The idea is to ignore the cases where the a function set
is called and if it is defined in java and java 3 or more arguments. The full context is in the linked issue.
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'm referring to the logical and numerical difference between the two statements.
code: args > 2
test: args > 3
Thanks for fixing the test. However, I think the code snippet in the test should then have exactly 3 parameters to assert the boundary (e.g. 3).
Excuse me for being a bit picky here.
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.
Done.
This fixes the issue raised con #4288 implementing the solution proposed in this comment: #4288 (comment):
In this case I'm just checking for the
set
case because I think that all the cases forget
should work as expected.fixes #4288