NPE in NewlinesRule: #1050
NPE in NewlinesRule: #1050
Conversation
### What's done: * fixed bug in NewlinesRule Closes #1047
Cheshiriks
force-pushed
the
feature/npe-array-access-exp
branch
from
2dfeaab
to
c16fbfe
Compare
Codecov Report
@@ Coverage Diff @@
## master #1050 +/- ##
============================================
+ Coverage 83.40% 83.43% +0.02%
Complexity 2447 2447
============================================
Files 102 102
Lines 6122 6120 -2
Branches 1833 1832 -1
============================================
Hits 5106 5106
+ Misses 273 272 -1
+ Partials 743 742 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| lintMethod( | ||
| """ | ||
| |fun bar(): String { | ||
| | val a = it.responseBody!!.name |
There was a problem hiding this comment.
This code basically didn't trigger an NPE before
| | val a = it.responseBody!!.name | |
| | val a = it.responseBody!![0].name |
There was a problem hiding this comment.
Yes. I removed this for a test and forgot to return
| @@ -461,7 +462,7 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule( | |||
|
|
|||
| if (psi.children.isNotEmpty() && !psi.isFirstChildElementType(DOT_QUALIFIED_EXPRESSION) && | |||
| !psi.isFirstChildElementType(SAFE_ACCESS_EXPRESSION)) { | |||
| val firstChild = psi.firstChild | |||
| val firstChild = if (psi.isFirstChildElementType(ARRAY_ACCESS_EXPRESSION)) psi.firstChild.firstChild else psi.firstChild | |||
There was a problem hiding this comment.
Is that just workaround?
I looked at this bug before and think, that the main problem is in these lines:
For now I didn't catch the logic of this if statement - in which cases do we need this, even more it doesn't covered by any test.
Probably, we should modify it or get rid of it at all.
There was a problem hiding this comment.
Yes, I deleted this piece and. everything continued to work
There was a problem hiding this comment.
Yes, it should work, however did you understand the reason, why did we need this before, for which cases?
I can't imagine any other examples for now, which will satisfy this condition, instead of it.responseBody!![0].name, for which it doesn't work
There was a problem hiding this comment.
Don't get how it should fix anything?
If we had an NPE on the further manipulations val firstChild or psi.firstChild, you should do null-checks and create special logic for it. Not simply hardcode ARRAY_ACCESS_EXPRESSION
### What's done: * fixed bug in NewlinesRule Closes #1047
| @@ -527,6 +527,18 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | |||
| ) | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
create fixer tests (where WRONG_NEWLINES is fixing the code in such case) to prove that you are right
There was a problem hiding this comment.
@Cheshiriks where are tests for fixers?
| """ | ||
| |fun bar(): String { | ||
| | val a = it.responseBody!![0].name | ||
| |} |
There was a problem hiding this comment.
please add positive scenarios (that will trigger warnings)
### What's done: * fixed bug in NewlinesRule Closes #1047
What's done:
Closes NPE in NewlinesRule #1047