-
Notifications
You must be signed in to change notification settings - Fork 39
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
KotlinParseException on string concatenation for if expression #1190
Conversation
889be47
to
118de7a
Compare
Codecov Report
@@ Coverage Diff @@
## master #1190 +/- ##
============================================
- Coverage 84.52% 84.49% -0.04%
+ Complexity 2515 2511 -4
============================================
Files 103 103
Lines 7055 7054 -1
Branches 1902 1902
============================================
- Hits 5963 5960 -3
Misses 306 306
- Partials 786 788 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
val myTest25 = "String ${valueStr?.value}" |
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.
two issues contained 2 tests. Or they are the same?
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.
This is one problem caused by the lack of an IfExpression check.
@@ -155,7 +156,8 @@ class StringConcatenationRule(configRules: List<RulesConfig>) : DiktatRule( | |||
val rvalueTextNew = rvalueText?.trim('"') | |||
return "$lvalueText$rvalueTextNew" | |||
} else if (binaryExpressionPsi.isRvalueCallExpression() || binaryExpressionPsi.isRvalueDotQualifiedExpression() || | |||
binaryExpressionPsi.isRvalueOperation() || binaryExpressionPsi.isRvalueSafeQualifiedExpression()) { |
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.
Do we need this branch at all? Or can it be the default behaviour (like the last branch of if
) when right side is a general KtExpression? I don't like that we need to add so many types here, I bet it will fire once again on something else.
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.
Yes, I agree.
Fixed
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.
What is the reason to leave separate branch for call expression?
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.
ReferenceExpression is always a child for the CallExpression. That's why the CallExpression requires a separate condition.
example
- -foo() [CallExpression]
- -- foo [ReferenceExpression]
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.
But then isRvalueReferenceExpression()
will be false and we'll get in .isRvalueExpression()
condition, won't we?
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.
3aad270
to
a4e4d51
Compare
a4e4d51
to
7d89624
Compare
What's done:
closes
KotlinParseException
in [STRING_CONCATENATION] #1076closes One more
org.cqfn.diktat.ruleset.utils.KotlinParseException: Text is not valid
#1141