Motivation: missing visibility modifier is an error in visibility modifiers list, so we should highlight this list.
Including a name in the range is convenient for using alt+enter (you don't have to move cursor from name to fun/class/val keyword)
Also change NO_EXPLICIT_RETURN_TYPE_IN_API_MODE diagnostic range to 'declaration name' to match corresponding IDE inspection.
Fix stylistic problems and typos after review
Change CLI flag to -Xexplicit-api=strict|warning. 'Disable' state and 'mode' suffix are left out as implementation details.
Change intention title to 'make X public explicitly'
Do not report 'no explicit visibility' on property accessors
Set DECLARATION_SIGNATURE as a range for report
Rename internal diagnostic from _MIGRATION to _WARNING
It will use the same SpecifyTypeExplicitlyIntention as in other places; to reuse check logic, some parts of code were moved from corresponding inspection (PublicApiImplicitTypeInspection) into ApiModeDeclarationChecker.
Also disable RedundantVisibilityModifierInspection when API mode is on.
As for SAM wrappers, the bytecode sequence
new A
dup
new B
dup
invokespecial B.<init>
invokespecial A.<init>
breaks the inliner, so instead we do
new B
dup
invokespecial B.<init>
store x
new A
dup
load x
invokespecial A.<init>
Otherwise, the cached instances cannot be reused for different wrapped
types. Also, if the wrapped type is regenerated during inlining, the
inliner would produce a call to a nonexistent constructor that takes the
regenerated type as an argument.
To avoid bytecode sequences like
new _1Kt$sam$i$java_lang_Runnable$0
dup
new _1Kt$f$1
dup
invokespecial _1Kt$f$1.<init>()V
invokespecial _1Kt$sam$i$java_lang_Runnable$0.<init>(...)V
as the different order of `new` and `<init>` confuses the inliner.
Consider the following constraint system (from the test example):
Nothing? <: V1
F!! <: V2
Inv<V1> <: S
Inv<V2> <: S
Where V1, V2, S are type variables, and F has nullable upper bound.
Type variable fixation order should be: V2 -> V1 -> S, and the problem
was that previously after fixation of type variable V2 we were trying
to fix S (before V1), so we had the following constraints on S:
Inv<F!!> <: S
Inv<V1> <: S
=> S were fixed to Inv<F!!>
And after this V1 was fixed to F!! which is contradictory as Nothing?
is not a subtype of F!!.
#KT-33033 Fixed
#KT-30297 Fixed
#KT-32168 Fixed
#KT-27722 Fixed (actually, it was fixed with addition of DefNotNullTypes, and now test was added to save this behavior)
#KT-32345 Fixed
Namely, anonymous objects defined in lambdas that have all captured
variables as loose fields instead of a single reference to the parent.
The question is, when a lambda inside an inline function defines an
anonymous object, and that object is not regenerated during codegen for
the inline function itself, but then has to be regenerated at call site
anyway, do we use an outer `this` or loose capture fields? For example,
before KT-28064:
inline fun f1(g: () -> Unit) = object { g() }
// -> f1$1 { $g: () -> Unit }
inline fun f2(g: () -> Unit) = f1 { object { g() } }
// -> f2$$inlined$f1$1 { $g: () -> Unit }
// f2$$inlined$f1$1$lambda$1 { this$0: f2$$inlined$f1$1 }
inline fun f3(g: () -> Unit) = f2 { object { g() } }
// -> f3$$inlined$f2$1 { $g: () -> Unit }
// f3$$inlined$f2$1$1 { this$0: f3$$inlined$f2$1 }
// f3$$inlined$f2$1$1$lambda$1 { this$0: f3$$inlined$f2$1$1 }
After KT-28064:
inline fun f2(g: () -> Unit) = f1 { object { g() } }
// -> f2$$inlined$f1$1 { $g: () -> Unit }
// f2$1$1 { $g: () -> Unit }
inline fun f3(g: () -> Unit) = f2 { object { g() } }
// -> f3$$inlined$f2$1 { $g: () -> Unit }
// f3$$inlined$f2$2 { ??? }
// f3$1$1 { $g: () -> Unit }
Should `???` be `this$0: f3$$inlined$f2$1` or `$g: () -> Unit`? This
commit chooses the latter for KT-28064 bytecode and keeps `this$0` when
inlining the old bytecode.
Here we want getting trailing comma in order to check its correctness
and then to report diagnostics. Now, note that if we have stub for
some PSI element, it means that we're definitely not in the compiler as
there are no stubs and we're definitely not in the current source file,
because in the current file we have full PSI tree in IDE.
Stubs are required, for example, for decompiled code and there is no
need to report any diagnostics about trailing comma there.
Also, because we don't have stubs for destructuring declarations,
one check for trailing commas is left without similar guard (
see resolveLocalVariablesFromDestructuringDeclaration method)
This commit fixes several IDE-tests:
MultiFileJvmBasicCompletionTestGenerated.testDoNotCompleteWithConstraints
MultiFileJvmBasicCompletionTestGenerated.testInImport
MultiFileJvmBasicCompletionTestGenerated.testInImportedFunctionLiteralParameter
MultiFileJvmBasicCompletionTestGenerated.testMoreSpecificExtensionGeneric
MultiFileJvmBasicCompletionTestGenerated.testNoGenericFunDuplication
MultiFileJvmBasicCompletionTestGenerated.testNotImportedExtensionFunction2
To avoid overload resolution ambiguity error in sources where everything
from both kotlin.reflect and kotlin.reflect.full is imported with a
star-import
Execution failed for task ':idea:idea-test-framework:test'.
> Could not resolve all files for configuration ':idea:idea-test-framework:testRuntimeClasspath'.
> Could not find org.jetbrains.intellij.deps.completion:completion-ranking-kotlin:0.0.2.
...
Required by:
project :idea:idea-test-framework > project :idea
It looks like when resolving this dependency gradle doesn't check the list of repositories in :idea module.
Steven Schäfer