The existing reference shortener does not use import alias when it
shortens a symbol. Instead, it adds a new import directive for the
symbol that is already imported. This commit updates reference shortener
to let it reuse the existing import alias rather than adding a new one:
1. When shortening a symbol, check whether the symbol is already
imported.
2. If it is already imported by an import alias, keep the symbol
reference expression and the import alias as a string together in
`ShortenCommand`.
The actual PSI update (shortening) based on the ShortenCommand is done
by IntelliJ.
^KTIJ-27205
Similar to K1 KDocLinkResolutionService used by Fe10KDocReference (to
support additional KDoc resolution), this commit adds K2 counterpart
AdditionalKDocResolutionProvider and uses it for KDocReferenceResolver.
^KT-62187
This commit sets a list of `KtElement` as the return type of
`ShortenCommand::invokeShortening()`. It allows us to take the result
i.e., shortened PSIs. This can be used, for example, when we want to
run code-format only on the shortened PSI after running the reference
shortener.
^KT-57636 Fixed
The existing K2 reference shortener collects all the PSI elements to
shorten. As a result, it possibly shortens duplicated PSI elements. For
example,
```
// FILE: main.kt
package a.b.c
fun test(n: Int) {
return if (<expr>x.y.z.Outer.Inner.VALUE0 > x.y.z.Outer.Inner.VALUE1</expr>) 1
else n
}
// FILE: values.kt
package x.y.z
class Outer {
object Inner {
val VALUE0 = 13
val VALUE1 = 17
}
}
```
for the above code, the existing K2 reference shortener tried to shorten
- x.y.z.Outer.Inner -> Inner
- x.y.z.Outer.Inner.VALUE0 -> VALUE0
- x.y.z.Outer.Inner -> Inner
- x.y.z.Outer.Inner.VALUE1 -> VALUE1
`x.y.z.Outer.Inner` is included in the list to shorten twice.
When it actually shortens the PSI elements, it shortens only
- x.y.z.Outer.Inner.VALUE0 -> VALUE0
- x.y.z.Outer.Inner.VALUE1 -> VALUE1
but it imports all of
- x.y.z.Outer.Inner
- x.y.z.Outer.Inner.VALUE0
- x.y.z.Outer.Inner.VALUE1
As a result, it has unnecessary additional import directives.
This commit fixes the issue by avoiding duplicated shortening for a
single PSI element.
For the following example, when we run the reference shortener, it
drops `a.b.c` qualifier, because it matches "FOURTH".
```
package a.b.c
fun <T, E, D> foo(a: T, b: E, c: D) = a.hashCode() + b.hashCode() + c.hashCode() // FIRST
fun <E> E.foo() = hashCode() // SECOND
object Receiver {
fun <T, E, D> foo(a: T, b: E, c: D) = a.hashCode() + b.hashCode() + c.hashCode() // THIRD
fun foo(a: Int, b: Boolean, c: String) = a.hashCode() + b.hashCode() + c.hashCode() // FOURTH
fun test(): Int {
fun foo(a: Int, b: Boolean, c: Int) = a + b.hashCode() + c // FIFTH
return <expr>a.b.c.foo(1, false, "bar")</expr>
}
}
```
As shown in the above example, when SHORTEN_IF_ALEADY_IMPORTED option is
given from a user, the reference shortener has to check whether it can
drop the qualifier without changing the referenced symbol and if it is
possible to do that without adding a new import directive, it deletes
the qualifier.
It needs two steps:
1. Collect all candidate symbols matching the signature e.g., function
arguments / type arguments
2. Determine whether the referenced symbol has the highest reference
priority when we drops the qualifier depending on scopes
This commit uses `AllCandidatesResolver(shorteningContext.analysisSession.useSiteSession).
getAllCandidates( .. fake FIR call/property-access ..)` for step1.
For step2, we use a heuristic based on scopes of candidates. If a
candidate symbol is under the same scope with the target expression, it
has a `FirLocalScope` which has the high priority. So when we have a
candidate under a `FirLocalScope` and the actual referenced symbol is
different from the candidate, we must avoid dropping its qualifier
because the shortening will change its semantics i.e., reference.
The order of scopes depending on their scope types is:
1. FirLocalScope
2. FirClassUseSiteMemberScope / FirNestedClassifierScope
3. FirExplicitSimpleImportingScope
4. FirPackageMemberScope
5. others
Note that for "others" the above rule can be wrong. Please update it if
you find other scopes that have a priority higher than the specified
scopes.
One of non-trivial parts is the priority among multiple
FirClassUseSiteMemberScope and FirNestedClassifierScope. They are
basically scopes for class declarations. We decide their priorities
based on the distance of class declaration from the target expression.
Note that we take a strict approach to reject all false positive. For
example, when we are not sure, we don't shorten it to avoid changing its
semantics.
TODO: One corner case is handling receivers. We have to update
```
private fun shortenIfAlreadyImported(
firQualifiedAccess: FirQualifiedAccess,
calledSymbol: FirCallableSymbol<*>,
expressionInScope: KtExpression,
): Boolean
```
The current implementation cannot handle the following example:
```
package foo
class Foo {
fun test() {
// It references FIRST. Removing `foo` lets it reference SECOND.
<caret>foo.myRun {
42
}
}
}
inline fun <R> myRun(block: () -> R): R = block() // FIRST
inline fun <T, R> T.myRun(block: T.() -> R): R = block() // SECOND
```
Tests related to TODO:
- analysis/analysis-api/testData/components/referenceShortener/referenceShortener/receiver2.kt
- analysis/analysis-api/testData/components/referenceShortener/referenceShortener/receiver3.kt
FirReferenceResolveHelper internally checks whether the referenced class
id matches the qualifed access or not. If they do not match, it reports
an error. When the companion object has the same name as the class,
resolving a qualified expression access to a member of the companion
object causes an error because of the mismatch e.g.,
```
package my.sample
class Test {
fun a() {
my.sample.<caret>Test.say()
}
companion object Test {
fun say() {}
}
}
```
This commit fixes the issue.
TODO: When the companion object has a name difference from class, it
does not report an error but the resolution result is wrong in FIR. See
KT-56167.
---
Commentary from rebaser: the issue mentioned in this code is
fixed in 71a368e06e, so the actual
fix is omitted, and only test data is preserved
The existing `KtModifierListOwner.addModifierList(newModifierList)`
function adds a visibility keyword (e.g., `private`) in front of the
`context(..)` modifier, which generates wrong code. This commit fixes
it.
^KTIJ-23728 fixed
For the following example:
```
fun foo(bar: Int) {
<expr>if (bar == 4) return "Four"
else return "Int"</expr>
}
```
AA FE1.0 `isUsedAsExpression` returns `false`.
Since the current AA FIR `isUsedAsExpression` returns `true` for the
above example, this commit fixes it.
KtWhenExpression.getMissingCases() reports the same missing cases even
when the KtWhenExpression has an else branch. Without a description, it
is difficult to be aware of it.
For an array access expression e.g., `foo[3]`,
KtFirExpressionTypeProvider first tries
`getReturnTypeForArrayStyleAssignmentTarget(..)` method that determines
the type of the array access expression when it is in the form of "set"
the value. However, the method also returns a non-null result for an
array access expression that reads its value i.e., "get". In this case,
the result of `getKtType()` becomes wrong. This commit lets the method
return a null for the type of "get" array access expression.
ReadWriteAccessCheckerFirImpl class is ReadWriteAccessChecker
implementation for FIR. We register it for both stand-alone and test
analsys. It is used by KotlinFirReferenceContributor.kt to check the
synthetic Java property.
FirSimpleNameReference class handles the references to the Java
synthetic property. It causes an issue in `processReference(..)` method
of `MethodTextOccurrenceProcessor` when the property is used by
read/write. Since read/write has two results of the reference resolution
i.e., getter for read and setter for write, `ref.solve()` in the method
returns `null` because it uses `resolve()` method in
PsiPolyVariantReferenceBase that returns null when it has multiple
results. This commit separates FirSimpleNameReference objects for
read/write to fix the issue.
The existing code does not set the list of bases that
FirLightInterfaceClassSymbol extends for "extendsList". It collects only
the set of interfaces for "extendsList" of FirLightInterfaceClassSymbol.
However, interfaces can "extend" other classes and/or interfaces, but
they cannot "implement" other interfaces. Therefore, we have to includes
all interfaces and classes that the child interface extends in
the "extendsList". Additionally, this commit adds `private fun
PsiClass.hasSuper(..): Boolean` to FirLightClassBase that returns
whether one of recursive super classes of the PsiClass is `baseClass`
or not. This commit lets `isInheritor()` method use
`PsiClass.hasSuper()`.
`fun getResolvedToPsi(analysisSession: KtAnalysisSession): Collection<PsiElement>`
of `KtFirReference` resolves the target of the reference. When the
target of reference to a property is a getter or setter, we have to
check the getter/setter if the target for the property does not
exist. This commit adds the fallback logic i.e., resolving the
getter/setter of the property in
`fun getResolvedToPsi(analysisSession: KtAnalysisSession): Collection<PsiElement>`
of `KtFirSimpleNameReference`.
Jaebaek Seo