KE2: Extract String.plus and String?.plus calls
#17752
Conversation
| symbol.callableId?.callableName?.asString() == functionName | ||
| return isFunctionMatchingNames( | ||
| symbol, | ||
| CallableId( |
There was a problem hiding this comment.
I think that it would be better to avoid constructing anything from kotlinc if possible, especially if we are aiming to use concurrency, as then we don't have to worry about what side-effects etc there might be. Here I think we can just as easily destruct the CallableId at the other end, as construct it here.
There was a problem hiding this comment.
CallableId and ClassId are tuples of 3 and 2 names. Should we introduce custom data classes instead of reusing the kotlin compiler types? I don't think we'd want to pass 3 + 2 String args instead.
There was a problem hiding this comment.
I've just been looking at how we might handle this problem in general; I'm more interested in cases where we want to extract e.g. a generated function, for which the analysis API doesn't give us the AST and we have to fabricate it. I was hoping that we could have our own CodeQlFunction interface, and have it be implemented by KtFunction using something like delegates. However, it looks like that isn't possible; there have long been discussions and proposals (e.g. https://youtrack.jetbrains.com/issue/KT-505, Kotlin/KEEP#87) but no implementation yet.
Perhaps CallableId and ClassId are simple enough that we should just reuse them? Or at least, do that for now until we come to something like the generated-function problem?
| return false | ||
|
|
||
| if (!isExtension) | ||
| return true |
There was a problem hiding this comment.
I'm a bit confused what's going on here. Should this be if (!isExtension && receiverType == null)?
There was a problem hiding this comment.
I reworked this a bit in an extra commit. My goal was to only use the last two parameters if isExtension==true.
| isExtension: Boolean = false, | ||
| nullability: KaTypeNullability = KaTypeNullability.UNKNOWN, | ||
| extensionReceiverClassName: ClassId? = null | ||
| ): Boolean { |
There was a problem hiding this comment.
I was wondering if this would read more clearly as an extension, so you'd have symbol.isFunctionMatchingNames(name, isExtension, nullability, extensionReceiverClassName)
There was a problem hiding this comment.
I pushed an extra commit that changed the functions to extensions.
No description provided.