^KT-61039 Fixed
KT-59739 is now open for K1 (but fixed in K2)
Review: https://jetbrains.team/p/kt/reviews/11867/timeline
This partially reverts commit 4f3ecedbca.
Only K1 part is reverted.
Motivation for revert: KT-59739 cannot be properly fixed in K1 because
of the bug it causes - KT-61039
We just accepted that we will have one more "green in K1 -> red in K2"
case
In scope of: KT-22841
Review: https://jetbrains.team/p/kt/reviews/11867/timeline
Reduce complexity by reusing "expect-actual matcher" (namely
`AbstractExpectActualCompatibilityChecker.getCallablesCompatibility`)
The current solution has worse algorithmic complexity. Previously it was
O(n) in the best case, where `n` is a number of members. Now, it's
O(m^2), where `m` is number of members in one overload group. But we
prefer to have worse complexity but reuse expect-actual matcher, number
of elements in one overload group shall not be big on real world
examples.
The previous logic was non-trivial because it compared types with with
double comparison in `equals`.
Motivation: the discovered false negative in test
changeModalityFromAbstractToOpenInOverride.kt
Unfortunately, it can't be fixed in K1, so we will just live with that.
Luckily, we report ABSTRACT_MEMBER_NOT_IMPLEMENTED additionally.
In K2, MODALITY_CHANGED_IN_NON_FINAL_EXPECT_CLASSIFIER_ACTUALIZATION
will be correctly reported as a compilation error
In scope of: KT-22841
Review: https://jetbrains.team/p/kt/reviews/11867/timeline
It was reported because in composite module analysis we have both expect
and actual annotation descriptors, which may differ.
This may lead to bugs in the checker of ACTUAL_ANNOTATIONS_NOT_MATCH_EXPECT
diagnostic.
`fqName` comparison can be safely removed, because we already check
types in `areCompatibleExpectActualTypes`.
Tests:
- typealiasedAnnotation.kt - for annotation use (so it has different
ClassId)
- typealiasedAnnotationAsArgument.kt - when typealiased annotation
used in another annotation as argument
- sourceRetentionAnnotationsWhenTypealias.kt - tests same as
compiler/testData/diagnostics/tests/multiplatform/annotationMatching/sourceRetentionAnnotationsWhenTypealias.kt
No special test added for opt-in, because it's prohibited to have
expect/actual opt-ins by EXPECT_ACTUAL_OPT_IN_ANNOTATION checker.
^KTIJ-26700
In the following scenario, when we search corresponding expect member
for actual `A.B`, we can skip checking compatibility of `B` scope.
```
class A {
class B {
fun foo() {}
}
}
actual typealias AImpl = A
```
This is because:
1. Annotation checker runs no matter if found expect class is compatible
or not.
2. Class always has at most one corresponding `expect` class (unlike for
functions, which may have several overrides), so we are sure that we
found the right member.
^KT-60668
^KT-60936
^KT-61163 Fixed
Review: https://jetbrains.team/p/kt/reviews/11599/timeline
They can be safely skiped because we check overridden descriptors
anyway.
IMO, it's a bug that delegated descriptors are copied with default
params. But it's much harder problem, and my IMO can easily be wrong for
some weird reason.
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
For StrongIncompatible `actual` declaration is considered as overload
and error reports on expected declaration. For WeakIncompatible the
error is reported straight away
Before the refactoring `areCompatibleClassScopes` returned just
`Incompatible`. It is bad because StrongIncompatible isn't possible for
classes (classes can't be overloaded). Now all class incompatibilities
are weak.
The commit has a minor impact on observable behavior (cases where we
reported the compilation problems are still reported but on another
elements):
- We no longer report type parameter class incompatibilities on expect
declaration, we report them only on actuals (it happened because all
WeakIncompatible are reported only on actuals)
- In a sense, Java implicit actualization was the only way to "overload"
classes (it would be a redeclaration compilation problem, so it
doesn't count as a valid "overload"). And since type parameters
incompatibility was StrongIncompatible for classes, we counted them as
"overloads" and didn't report incompatibility problems on Kotlin
class. Now we do report. (see
implicitJavaActualization_multipleActuals)
^KT-60902 Fixed
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
We should prioritize to return STRONG incompatibilities over WEAK
incompatibilities. But this invariant broke in `areCompatibleCallables`,
because `areCompatibleTypeParameters` returns incompatibilities of both
types, and `areCompatibleTypeParameters` is called in WEAK
incompatibilities section.
The fix is to split `areCompatibleTypeParameters` into two functions:
`areStrongIncompatibleTypeParameters` and
`areWeakIncompatibleTypeParameters`. And call each of this function in
appropriate `areCompatibleCallables` sections.
^KT-59665 Fixed
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
It's better to have this logic in common place
(AbstractExpectActualCompatibilityChecker) to avoid missing compilation
errors in the future
This commit fixes:
1. Missing compilation error for actual function with default arguments
for 'actual typealias' KT-59665
2. Missing compilation error for actual function with default arguments
for actual fake-override KT-59665
Alternative solution for KT-59665 is to create a special checker.
"incompatibility" vs "special checker":
Arguments for common incompatibility:
- What if we had a rule that expect and actual default params must
match? If so then it certainly would be an incompatibility.
- Technically, we do the matching of expect and actual params (because
we allow default params in common ancestors of expect and actual
declarations).
- It's hard to check that the actual definition doesn't use default
params because `ExpectedActualResolver.findActualForExpected` filters
out fake-overrides and doesn't return them. It's not clear logic for
me, that I'm afraid to touch.
implicitActualFakeOverride_AbstractMap.kt test breaks if you drop this
weird logic
- WEAK incompatibilities can be considered as "checkers". So it doesn't
matter how it's implemented, as a "incompatibility" or a "checker"
Arguments against common incompatibility:
- Although we match expect and actual declarations to allow default
params in common ancestors of expect and actual declarations, it's
still can be considered that we check that the actual declaration
doesn't have default params. And it doesn't feel right that we check
correctness of the actual declaration in expect-actual matcher.
- ~~It may change the rules of expect actual matching~~ (It's not true,
because ActualFunctionWithDefaultParameters is declared as WEAK
incompatibility)
Many errors are reported in stdlib with these annotations
(SinceKotlin, Deprecated, so on).
But having them only on expect is a valid case. E.g. SinceKotlin added
if some old platform-specific API becomes commonized.
^KT-58551
This implementation only checks annotations set on expect/actual
declarations and requires further refinement (e.g. checking of other
annotation targets, class scopes within typealiases).
^KT-58551
This lets us properly complete the call which fixes some issues with
false-positive type mismatches.
This change doesn't apply to array literals in annotation calls yet
because they are resolved as context-dependent.
This will be adapted in a following commit.
#KT-59581
Nikita Bobko