-
-
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
Match functions signatures with lambdas on it #4458
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4458 +/- ##
============================================
- Coverage 84.32% 84.16% -0.16%
- Complexity 3298 3314 +16
============================================
Files 473 476 +3
Lines 10591 10908 +317
Branches 1907 2023 +116
============================================
+ Hits 8931 9181 +250
- Misses 675 694 +19
- Partials 985 1033 +48
Continue to review full report at Codecov.
|
e2b1735
to
1576eeb
Compare
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.
Thanks for expanding the supports for Function signatures! My comments were not blocking, they were primarily documenting my time spent on the confusion.
} | ||
} | ||
split.add(result.toString()) | ||
return split.map { it.trim() }.filter { it.isNotBlank() } |
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.
Is it.isNotBlank()
necessary? Without verifying with actual test code, I assume they are only possible at Line 97. I believe the isNotBlank()
and trim()
should be applied when append()
is invoked, which avoids creating more lists.
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.
Fixed, what do you think now?
Related with #4448 but it doesn't fix it yet. The next step is to support extension functions.
Note: If you check the tests you will see that the signature
foo(() -> kotlin.String)
matches a functione like this one:fun foo(a: () -> Unit)
. That's because we don't need to distinguish between this two functions:The compiler doesn't allow to define both at the same time.
If we want to distinguish between those we would introduce a breaking change because until now this signature:
foo(List)
was matchingfun foo(a: List<String>)
but if we enforce to specify the generics that type should be changed.For those two reasons combined I think that it's better to ignore the generics.