It became possible to drop it after KT-62590. Now, on a frontend, the
return type check is part of a common AbstractExpectActualChecker logic
Change in nestedAnnotationClassViaActualTypealias.fir.kt aligns the
behaviour with K1. KT-61964
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
Those two functions are copy-paste of each other. They diverged since
the time they were copy-pasted. This commit makes
FirExpectActualMatchingContextImpl.areCompatibleExpectActualTypes
up-to-date.
I didn't update ExpectActualUtilsKt.areCompatibleExpectActualTypes
because I will drop it in the next commit.
Change in `dynamicTypesEqualToAnything` doesn't change any logic (yet.
This change in logic will take effect in the next commit), because:
1. `dynamicTypesEqualToAnything` is only changed in
AbstractExpectActualChecker.getCallablesCheckingIncompatibility. But
AbstractExpectActualChecker.getCallablesCheckingIncompatibility doesn't
check return types on frontend (it only check return types on backend).
2. `dynamicTypesEqualToAnything` is ignored on IR backend
I have no idea what is the difference between `createTypeCheckerState()`
and `actualSession.typeContext`, but it aligns these copy-pasted
versions and makes the tests behave like in K1 (at least
'typeUsageWithUnresolvedReference' and 'kt57320' are affected)
This commit is mainly a preparation for the next commit.
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
- getCallablesStrongIncompatibility is unused (because it moved to the
"matcher" KT-62590)
- `checkClassScopesCompatibility` parameter is always `true` or unused
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
- Migrate ExpectActualCompatibility -> ExpectActualCheckingCompatibility
where the "checker" is expect
- Migrate ExpectActualCompatibility -> ExpectActualMatchingCompatibility
where the "matching" is expect
KT-62590 in progress. A lot of tests start to fail now. I will fix them
in next commits
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
- The Matcher must only do the "matching" (aka "strong compatibility")
- Drop all "checking" related (aka "weak compatibility" related) logic
- `checkClassScopesCompatibility` parameter belongs to the
"expect-actual checker" not to the "expect-actual matcher". Drop it
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
This is a minor optimization.
This commit is a step forward for KT-62590
Conceptually, `checkClassScopesCompatibility` belongs to the
"expect-actual checker". It doesn't belong to the "expect-actual
matcher".
- `checkClassScopesCompatibility` should be set to `true` when you want
to do the "checking"
- `checkClassScopesCompatibility` should be set to `false` when you want
to do the "matching"
`collectActualCallablesMatchingToSpecificExpect` only needs the
"matching"
No tests changed their behaviour
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
Motivation: Make getCallablesCompatibility API slightly nicer. The
function is going to be called from more places after KT-62590 is fixed.
This commit makes fixing KT-62590 easier
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
It should have been WeakIncompatible from the beginning because it's not
possible to overload by return type in Kotlin
This commit is a step forward to fix KT-62591
Unfortunately, the test cannot demonstrate the problem because of
another bug in K2 KT-59887
^KT-62752 Fixed
Review: https://jetbrains.team/p/kt/reviews/12750/timeline
Motivation:
It makes expect-actual matching-checking model more consistent.
expect-actual "matching" is run before FirResolvePhase.BODY_RESOLVE. You
can't know return types, until you run BODY_RESOLVE. That's why the
return type can't be checked during expect-actual matching. But it's
cursed: you have something that have to match by, but, at the same time,
you can't do it.
expect-actual "checking" is run after FirResolvePhase.BODY_RESOLVE.
That's why if we convert ReturnType incompatibility to WeakIncompatible
(which should have been called CheckingIncompatible), then expect-actual
matching model becomes consistent.
We will also be able to get rid of unnecessary
FirActualCallableDeclarationChecker. Because it won't be necessary.
Return types will be checked by common logic of expect-actual "checker"
Unresolved annotation arguments were treated as absent arguments,
which lead to false-positive reports.
Add assert and test for that and fix.
MR: KT-MR-12245
^KT-60671 Fixed
This includes checking of annotatins set on:
- value parameter types
- type parameter bound types
- extension functions receiver types
- function return types
- class super types
Fix in `defaultParams_inheritanceByDelegation_positive.kt`
is needed because of problem in resolution of implicit return types
(KT-62064), which leads to crash in annotation checker, because it
expects resolved return type.
MR: KT-MR-12245
^KT-60671 Fixed
Review: https://jetbrains.team/p/kt/reviews/12279/files
Motivation: make sure that cases like KT-62027 won't happen again
Review: https://jetbrains.team/p/kt/reviews/12279/files
Now it's responsibility of the
`createExpectActualTypeParameterSubstitutor` calller to think about the
case when parameters size isn't equal. You must not be able to create a
substitutor if type parameters sizes are not equal
Improvement in `createExpectActualTypeParameterSubstitutor` API also
improves
`AbstractExpectActualCompatibilityChecker.getCallablesCompatibility`
API.
Because suppose that you accidentally created a redundant wrapping
substitutor => you need to handle the case of not equal type parameters
size on the call site => you start thinking why you should do that on
the call site? It must be a responsibility of
`getCallablesCompatibility` => you realize that you created a redundant
wrapping substitutor
This commit handles subtle situation when K1 represents flexible type
arguments as just T..T?, but K2 does it as T&Any..T?.
This can provoke a type like Captured(*)&Any..Captured(*)?,
and before this commit we couldn't find recursion inside Captured(*)&Any.
This could lead to explosions inside type system and inference errors
#KT-60581 Fixed
It's implemented as IR checker because in K2 constant expressions are
evaluated on backend. FIR diagnostic removed because isn't needed.
"annotationViaActualTypeAlias" test has no `// FIR_IDENTICAL` because
diagnostic reported on entire typealias declaration instead of its name.
This is because in IR+LightTree we have only offsets, so can't navigate
to typealias name element.
^KT-59940 Fixed
There is IDE quick fix which suggests to copy mismatched annotation
from `expect` to `actual` (see KTIJ-26633). It needs to find
`actual` PsiElement where to add annotation. Before previous commit, it
was easy - just get source of `Incompatibility.actualSymbol`.
After previous commit, the problem might be in value parameter, while
`actualSymbol` would contain function symbol. This is solved by adding
new field `Incompatibility.actualAnnotationTargetElement`.
`SourceElementMarker` introduced, because it's needed to be used in
abstract checker. Existing `DeclarationSymbolMarker` doesn't fit
because in next PR for this issue annotations set on types will be reported,
and types are not declarations.
^KT-60671
Comment was added to make it clear why whole declarations reported
in diagnostic instead of value parameter symbols (same will be done
with other targets in subsequent commits).
^KT-60671
The main idea is getting rid of stub types and using just type variables
See more detailed description at docs/fir/delegated_property_inference.md
The problem with stub types is that they need really special treatment
in many places, and on the other hand, there are no clear contracts on
how they should work (that regularly leads to bugs like KT-59529)
^KT-61060 Fixed
^KT-61075 Fixed
^KT-61077 Fixed
^KT-59529 Fixed
^KT-61633 Related
^KT-61618 Related
^KT-61740 Related
^KT-59107 Related
^KT-61747 Related
^KT-61077 Related
^KT-61781 Related
@ImplicitlyActualizedByJvmDeclaration is the only one
OptionalExpectation annotation which works correctly when set only on
`expect`. All other (like @JvmName, @JsName) - not, so warning for them
must be reported.
^KT-61725 Fixed
Rename to "expect" and "actual" annotation.
This will be needed in next commit to make it clear that
only expect annotation value needs special handling.
^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
Currently, there is only attribute `ExpectForActualAttributeKey`
where mapping is stored only for source declarations with `actual`
modifier. But we need mapping of all class members, including classes
which were actualized via `actual typealias` or fake override members.
This data will be needed for the annotation checker in subsequent
commits.
^KT-60668
^KT-60936
Separate callable and class checks to prepare for checking
class scopes. Extract common checks to separate methods.
Also, it is found that in IR checker annotations on type parameters also checked,
because they stored in `matchedExpectToActual`. But it is expected that
only classes and callables are checked. This started to fail because of
added input parameter type checks inside `areAnnotationsCompatible`.
That's why `expectSymbol is IrTypeParameterSymbol` early-return is added.
^KT-60668
^KT-60936
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
Motivation:
- Functions with prefix "are" must return Boolean. And
AbstractExpectActualCompatibilityChecker even already contains some
functions with prefix "are" that return Boolean (e.g.
`areCompatibleCallableVisibilities`,
`areCompatibleSupertypesOneByOne`, etc)
- Unification with functions that are prefixed with "are" and return
Boolean
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)
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
Extract main logic of `areCompatibleCallables` into two functions:
`areStrongIncompatibleCallables` and `areWeakIncompatibleCallables`.
The main point is that `areStrongIncompatibleCallables` &
`areWeakIncompatibleCallables` have very specific return types.
This commit doesn't change any logic. The commit makes the API more
type-safe ensuring that bugs like in previous commit (KT-60902) won't
happen again
^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.
Review: https://jetbrains.team/p/kt/reviews/11039/timeline
This is a preparation refactoring for the following KT-60902 fix and
type-safety refactoring.
This commit doesn't have observable side effects (and can be called pure refactoring) because the following conditions:
1. `expectDeclaration is ConstructorSymbolMarker && actualDeclaration is ConstructorSymbolMarker`
2. `expectDeclaration is FunctionSymbolMarker != actualDeclaration is FunctionSymbolMarker`
can't be both `true` at the same time
^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)
Currently, property `hasSourceAnnotationsErased` returns the opposite
to what is stated in name. Invert it both in implementation and on
call site.
^KT-58551
Nikita Bobko