[nbauma109]

Parses and writes the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes (JVMS 4.7.20), which Attribute.readAttribute returns as Unknown today.

Why

Annotations written on types (JSR 308) are stored apart from the declaration annotations: JSpecify's @Nullable in void put(@Nullable Object key), List<@Nullable String>, <K extends @Nullable Object>. A user of BCEL can only get them by parsing the bytes of an Unknown attribute itself: target type, variable-size target info, type path, then the annotation with its element values. For example, a decompiler built on BCEL loses every @Nullable of a library such as Guava, whose packages are @NullMarked, and the recompiled sources are rejected by Error Prone.

Changes

• Const: ATTR_RUNTIME_VISIBLE_TYPE_ANNOTATIONS = 29, ATTR_RUNTIME_INVISIBLE_TYPE_ANNOTATIONS = 30, KNOWN_ATTRIBUTES = 31, and the two attribute names.
• TypeAnnotations (abstract, Iterable<TypeAnnotationEntry>, like Annotations), RuntimeVisibleTypeAnnotations, RuntimeInvisibleTypeAnnotations.
• TypeAnnotationEntry: the target type (TARGET_* constants for the table 4.7.20-A), the target info as stored in the class file (its size depends on the target type; the localvar_target table has a variable size), the type path (getTypePathLength, getTypePathKind, getTypePathArgumentIndex), and the AnnotationEntry read by the existing code. An undefined target type throws ClassFormatException; the constructor rejects a type path that is not a sequence of pairs of bytes (at most 255 steps) and a target info whose size does not match the target type. dump writes what read read, and copy(ConstantPool) makes a deep copy bound to the given constant pool.
• Attribute.readAttribute, Visitor.visitTypeAnnotations (a default method: existing Visitor implementations are unaffected), DescendingVisitor (which also visits the AnnotationEntry of each entry), EmptyVisitor, StringRepresentation, and an entry in changes.xml.

The target info is exposed as bytes, not as one class per kind of target, to keep the API small. The meaning of its bytes is documented on getTargetInfo(): index of the type parameter, of the bound, or of the formal parameter.

Tests

TypeAnnotationsTest reads src/test/resources/typeannotations/TypeAnnotated.class, compiled with javac --release 8 from the .java file next to it. It covers a field, the type path of a type argument, an invisible annotation, a method return and a parameter, the bound of a class type parameter, dump then re-read, copy (independent mutation, other constant pool), the visitor callbacks and their stack context, and StringRepresentation. More tests cover the localvar_target table (read then dump gives the same bytes), an undefined target type, a malformed type path and a target info of the wrong size.

The default mvn goal (tests, rat, japicmp, checkstyle, pmd, spotbugs, javadoc) passes.

🤖 Generated with Claude Code

[nbauma109]

@garydgregory The 28-ea job of this pull request fails for a reason unrelated to the change: master fails it too, since JaCoCo 0.8.15 cannot instrument Java 28 class files (Unsupported class file major version 72). #538 fixes it. Once #538 is merged and this branch is rebased onto master, 28-ea is expected to pass.

On my fork every other job passes (21 of 22, all operating systems and Java 8 to 27): nbauma109#103

[garydgregory]

@nbauma109
Please comment on or address these the following.

Copying

RuntimeVisibleTypeAnnotations.copy(ConstantPool) and RuntimeInvisibleTypeAnnotations.copy(ConstantPool) do not fulfill their documented contract. Both methods promise a deep copy, but return a shallow clone, ignore the supplied constant pool, and share the original annotation array and mutable AnnotationEntry objects. Changing shared contents through the copy can therefore change the original. Existing ordinary-annotation classes have a similar defect, but repeating it here also causes a regression: these attributes previously used Unknown.copy(ConstantPool), which copies its bytes and installs the supplied pool. The new methods should copy mutable contents independently and bind their constant-pool references to the supplied pool. Those references are described in JVMS §4.7.20; the deep-copy requirement comes from BCEL’s Javadoc. The current test compares strings using the original pool, so it needs additional checks for independent mutation and a different pool.

Visting

DescendingVisitor.visitTypeAnnotations(TypeAnnotations) leaves the new visitor support incomplete. It visits the attribute but does not visit the contained AnnotationEntry objects, unlike DescendingVisitor.visitAnnotation(Annotations). With the new fixture, there are five type-annotation attribute callbacks and zero annotation-entry callbacks, despite six annotations being present. Previously, these annotations were inside Unknown attributes and were not traversed either, so this is missing functionality rather than a regression. The new method should traverse the wrapped annotation entries, with tests checking their callbacks and visitor stack context. The contained annotation structure is defined in JVMS 4.7.20; the traversal expectation comes from BCEL’s existing visitor behavior.

TypeAnnotationEntry

The new TypeAnnotationEntry API accepts malformed input, and some new Javadoc is inaccurate. The public constructor documents typePath as a pair of bytes per step, but accepts a one-byte array. dump(DataOutputStream) then writes a path length of zero followed by that extra byte, shifting the following annotation fields. Dumping succeeds, but reading the result throws EOFException. This affects invalid caller input to the new API; valid parsed entries round-tripped successfully. Validation should reject malformed arrays before writing. JVMS 4.7.20.2 defines the two-byte path entries, and 4.7.20.1 defines the target-info layouts.

Javadoc

The new Javadoc also needs corrections: getTypePathLength() says an empty path identifies the “whole type,” but for nested types it identifies the outermost part where an annotation is allowed; TARGET_CLASS_EXTENDS omits interface extends clauses; and the new attribute class summaries omit expression types and type-parameter declarations. These cases are described in JVMS 4.7.20.

Could you address each point, or explain the intended BCEL contract if any of these behaviors is intentional?

[nbauma109]

@garydgregory All four points are addressed in 601010f.

Copying. copy(ConstantPool) of both attributes now sets the given constant pool on the copy and replaces the entries with deep copies (TypeAnnotationEntry.copy(ConstantPool), which rebuilds each entry and its AnnotationEntry against the given pool, keeping the indexes as Unknown.copy does). The test checks the pool of the attribute and of the copied annotations, that the entries and annotation objects differ, that changing the copy (adding an element value pair, replacing its table) leaves the original unchanged, and a copy of the invisible attribute.

Visiting. DescendingVisitor.visitTypeAnnotations visits the AnnotationEntry of each entry, while the attribute is on the stack. The test counts 5 attribute callbacks and 6 annotation entry callbacks on the fixture, and checks through predecessor() that each annotation is visited within its attribute class.

TypeAnnotationEntry. The constructor throws IllegalArgumentException for a type path that is not a sequence of pairs of bytes or has more than 255 steps, and for a target info whose size does not match the target type (the localvar_target table is checked against its length); an undefined target type still throws ClassFormatException. Tests cover each case, and the 255-step limit being accepted.

Javadoc. getTypePathLength() now says an empty path is the outermost part of the type where an annotation is allowed (@A Outer.Inner, whereas Outer.@A Inner needs a step); TARGET_CLASS_EXTENDS covers the extends of an interface; the attribute summaries list the declarations, the supertypes, the type parameters and their bounds, and the expressions and local variables of a Code attribute, and the classes that hold the attributes.

The default mvn goal passes.