When making KtLightNullabilityAnnotation after test org.jetbrains.kotlin.idea.caches.resolve.IdeLightClassTestGenerated.NullabilityAnnotations#testJvmOverloads
started failing the wrong assumption was made that for
@JvmOverloads-generated overloads their last parameter is always nullable
(see removed isNullableInJvmOverloads function) and that lead to a bug,
namely KT-28556.
The actual problem of this test started failing was incorrect definition
of kotlinOrigin in KtLightParameter for case of JvmOverloads:
wrong KtParameter was being chosen before
This commit fixes the issue by propagating how actually generated
parameters in codegen relate to source KtParameters'
^KT-28556 Fixed
Extract Java-specific code into module 'frontend.java' and use already
existing JavaAnnotationArgument facilities to determine the default
parameter value in an actual Java annotation class.
Annotation arguments are not yet supported because to create an instance
of AnnotationValue, we need a descriptor, which is not available at this
point.
#KT-22704 In Progress
#KT-28077 Open
This commit restores the heuristic removed in 9df02b2366, but only
enables it in case the class wasn't found with the name mapped initially
according to the InnerClasses attribute values. This helps to support
class files which do not have the InnerClasses attribute for all nested
referenced classes, such as those generated by Groovy.
Note that in theory it's still possible for this code to behave
incorrectly, for example a reference to a class `C` nested in a
_top-level_ class named `A$B` will not be loaded by this code correctly.
This is a lower-priority issue and it will need to be fixed later.
#KT-27874 Fixed
The only place where a name with dots was passed to
ClassifierResolutionContext.mapInternalNameToClassId was in
BinaryClassSignatureParser.parseParameterizedClassRefSignature, where
for some reason names can be both of internal (`a/b$c`) and canonical
(`a/b.c`) form. Supporting both names in mapInternalNameToClassId was
confusing because JVM internal names cannot contain dots. Replace dots
with dollars in parseParameterizedClassRefSignature to get rid of the
dot-name handling in mapInternalNameToClassId and rely on the correct
(simplified) InnerClasses-based name resolution later instead.
Also inline mapDescToClassId to its only usage.
This is an addition to c1ab08c8ce where KT-26582 was fixed. The test
testClassLiteralArguments in JvmRuntimeDescriptorLoaderTestGenerated
failed before this change but went unnoticed because it wasn't run on CI
(see the previous commit)
Preface: for Groovy traits with fields, the Groovy compiler generates
synthetic "$Trait$FieldHelper" classes which posed several problems to
our class file reader, caused by the fact that the contents of the
InnerClasses attribute broke some assumptions about how names on the JVM
are formed and used.
For a trait named `A`, the Groovy compiler will additionally generate a
synthetic class file `A$Trait$FieldHelper` with the following in the
InnerClasses attribute:
InnerClasses:
public static #15= #2 of #14; //FieldHelper=class A$Trait$FieldHelper of class A
i.e. the simple name of the class is `FieldHelper`, the name of its
outer class is `A`, but the full internal name is `A$Trait$FieldHelper`,
which is surprising considering that the names are usually obtained by
separating the outer and inner names via the dollar sign.
Another detail is that in some usages of this synthetic class, the
InnerClasses attribute was missing at all. For example, if an empty
class `B` extends `A`, then there's no InnerClasses attribute in `B`'s
class file, which is surprising because we might decode the same name
differently depending on the class file we encounter it in.
In this change, we attempt to treat these synthetic classes as top-level
by refusing to read "invalid" InnerClasses attribute values (they are
not technically invalid because they still conform to JVMS), fixing the
problem of "unresolved supertypes" error which occurred when these
classes were used as supertypes in a class file in a dependency.
1) In ClassifierResolutionContext.mapInternalNameToClassId, do not use
the ad-hoc logic (copy-pasted from intellij-core) to determine class
id heuristically from the internal name. For $Trait$FieldHelper
classes this logic attempted to replace all dollar signs with dots,
which was semantically incorrect: dollars there were used as
synthetic characters, not as a separator between outer and inner
classes.
2) In isNotTopLevelClass (Other.kt), only consider "valid" InnerClasses
attribute values, where the full name of the class is obtained by
separating the outer name and the inner name with a dollar character.
This way, we'll be able to treat class files with invalid attribute
values as top-level and avoid breaking any other assumptions in the
class file loader.
3) In BinaryJavaClass.visitInnerClass, record all valid InnerClasses
attribute values present in the class file, not just those related to
the class in question itself. This is needed now because previously,
the removed heuristics (see p.1) transformed mentioned inner class
names to class ids correctly >99% of the time. Now that the
heuristics are gone, we'll use the information present in the class
file to map names correctly and predictably. According to JVMS, this
attribute should contain information about all inner classes
mentioned in the class file, and this is true at least for class
files produced by javac.
#KT-18592 Fixed
Only invariant array projections and non-null element types will be
supported soon (see KT-26568), so it makes no sense to store the
complete type in KClassValue. What we need is only the ClassId of the
class, and the number of times it's wrapped into kotlin/Array, which is
exactly what ClassLiteralValue represents.
This change helps in decoupling annotation values from
descriptors/types. The only constant value that depends on descriptors
is now AnnotationValue.
#KT-26582 Fixed
There is a trade-off between robustness of check and accuracy of the
diagnostic: the previous version, which works on generation, was too
fragile and lead to false-positives. Now we check on state machine
generation. However, since we do not have PSI for call, we can only
report diagnostic on whole suspend function or suspend lambda.
Additionally, the state machine is generated on crossinline suspend
lambdas regeneration and thus we do not have the PSI for the lambda as
well!
#KT-27130 Fixed
#KT-27258 Open
Declare AnalysisFlags in module 'frontend', and JvmAnalysisFlags in
module 'frontend.java', to avoid leaking Java-related logic to common
compiler code
* The members of Result are isSuccess, isFailure, exceptionOrNull, getOrNull
* The rest of API is implemented via inline-only extensions
* There are two internal functions to hide detailed mechanics of an internal
Result.Failure class: createFailure and throwOnFailure
* Result.toString is explicit: either Success(v) or Failure(x)
See KT-26538
Otherwise we're not trying to load annotations on the parameter of the
property setter in MemberDeserializer.loadProperty.
Note that after this commit, we could now also assume that if
getter/setter is default, it has no annotations, and thus use
Annotations.EMPTY for default getter/setter in loadProperty. However,
this would cause reflection to work incorrectly on classes compiled by
an older Kotlin compiler, so we'll still try to load annotations on
default accessors for an indefinite time.
#KT-25499 Fixed
Add PropertyDescriptor.backingField/delegateField to store annotations
on the field directly in an otherwise almost empty descriptor instance,
instead of storing them with use-sites in the corresponding property
descriptor. Instead of AnnotationWithTarget, create AnnotationDescriptor
instances in AnnotationSplitter. Change DescriptorRenderer to render
annotations on "related" declarations when needed, with the explicit
use-site target if applicable.
Most changes in diagnostic test data are related to the fact that
annotations which are known to have an incompatible use-site to the
declaration they're applied at (such as `@param:`-annotation on a
function), are now not loaded at all. It's fine because the code is
erroneous, so it doesn't really matter how do we load annotations with
invalid targets (some of this logic is also changed freely in subsequent
commits). Some changes are also explained by the fact that for example
an annotation on the property which is only applicable to FIELD is now
rendered with an explicit use-site target `@field:`, regardless of
whether it did have that use-site target syntactically or not.
Basically, after this change there's no point in calling
Annotations.getUseSiteTargetedAnnotations/getAllAnnotations anymore
because it's easier and more intuitive to just use Annotations of the
corresponding descriptor -- the backing / delegate field (introduced in
this commit) or the extension receiver / setter parameter (related
behavior was fixed in previous commits). Usages of
use-site-target-related methods will be refactored out in subsequent
commits
Mikhael Bogdanov