[2.x] fix: Classfile fallback for the remaining ClassToAPI reflection sites#1711
Open
rkophs wants to merge 3 commits into
Open
[2.x] fix: Classfile fallback for the remaining ClassToAPI reflection sites#1711rkophs wants to merge 3 commits into
rkophs wants to merge 3 commits into
Conversation
When a class references a transitive dependency that is missing from the
analysis classpath (e.g. an optional/provided dep), the JVM throws
NoClassDefFoundError / TypeNotPresentException / ClassNotFoundException
the moment ClassToAPI calls c.getMethods, c.getGenericReturnType,
c.getAnnotations, or c.getTypeParameters. This aborts API extraction for
the whole class and breaks incremental compilation downstream.
Keep reflection as the primary path (zero behavior change on the happy
path) and fall back to a classfile-based extractor when reflection
throws. The fallback parses:
- Field and method descriptors -> erased api.Type (DescriptorParser)
- RuntimeVisible/Invisible Annotations and ParameterAnnotations
(JVMS 4.7.16 / 4.7.18) -> api.Annotation + per-parameter annotations
- Exceptions attribute (JVMS 4.7.5) -> @throws annotations
Each fallback trigger logs a warning. Inherited members carry their
parent's (Class, ClassFile) so attribute byte indices resolve against
the correct constant pool. Main-class detection moves to the classfile
unconditionally (strictly more reliable; no transitive-dep resolution).
Generic Signature parsing (JVMS 4.7.9) is deferred; the fallback uses
erased descriptor types and an empty type-parameter list when reflection
on a generic method/class fails.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ParserSpecification: cover the new ClassFile members directly
- methodExceptions against Thread.sleep (JDK Exceptions attribute)
- FieldOrMethodInfo predicates against JDK (constructor, varargs, static
final field)
- all element_value kinds in RuntimeVisibleAnnotations via an
inline-compiled fixture (string, int, char, double, float, long, byte,
short, enum, class, array, nested annotation)
- parameter annotations indexed by position, including an unannotated
middle slot
- bridge/synthetic detection on the erasure bridge of a generic class
ClassToAPISpecification: exercise the reflection-fallback path end-to-end
by compiling against a Missing dep, then loading via a classloader that
excludes it
- fallback on missing method return type
- fallback on missing method parameter type
- fallback on missing field type
- regression: inherited member annotations read against the parent's
constant pool, not the child's (the bug caught during the refactor —
without this, AttributeInfo byte indices either return garbage or
trip the entry.tag == ConstantUTF8 assertion in toUTF8)
- @throws annotations emitted on classfile-fallback methods
- warn-on-fallback observable via a recording Logger
- documented limitation: missing-type class annotations are silently
dropped by modern JDKs' getAnnotations(), so the fallback never
triggers in that specific case; pinned as a "(limitation)" test
ClassToAPI: TODOs documenting the deferred work
- allSuperTypesSafe: two-layer recovery (raw getSuperclass/getInterfaces
retry, then classfile superClassName/interfaceNames) — current empty
return drops inheritance edges from cmap.inherited and parent types
from the Structure
- classTypeParametersSafe: parse the Signature attribute (JVMS 4.7.9)
to recover type parameters — currently degrades to empty
- classAnnotationsSafe: switch class annotation reading off reflection
entirely so the silently-dropped case is recoverable
Also restores the pre-existing "TODO: over time, ClassToAPI should switch
the majority of access to the classfile parser" comment on
classFileForClass that the earlier rewrite removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
✅ Signed |
Member
|
Thanks for the contribution!
With this change, were you able to get the build going? |
eed3si9n
reviewed
Jun 7, 2026
Comment on lines
+200
to
+203
| cmap.log.warn( | ||
| s"Reflection failed introspecting ${c.getName} " + | ||
| s"(${e.getClass.getSimpleName}: ${e.getMessage}); falling back to classfile parser" | ||
| ) |
Member
There was a problem hiding this comment.
I don't think we should show warning, if the build user can't take actions to correct it.
eed3si9n
reviewed
Jun 7, 2026
Comment on lines
-175
to
+176
| if ( | ||
| c.getMethods.exists(meth => | ||
| meth.getName == "main" && | ||
| Modifier.isStatic(meth.getModifiers) && | ||
| meth.getParameterTypes.length == 1 && | ||
| meth.getParameterTypes.head == classOf[Array[String]] && | ||
| meth.getReturnType == java.lang.Void.TYPE | ||
| ) | ||
| ) { | ||
| if (classFileForClass(c).methods.exists(_.isMain)) { |
Member
There was a problem hiding this comment.
Does this introduce classFileForClass(c) for all classes, which I think is a happy path?
eed3si9n
reviewed
Jun 7, 2026
Comment on lines
+353
to
+356
| cmap.log.warn( | ||
| s"Reflection failed reading annotations on ${c.getName} " + | ||
| s"(${e.getClass.getSimpleName}: ${e.getMessage}); falling back to classfile parser" | ||
| ) |
Member
There was a problem hiding this comment.
Same here about warning that user might not take actions.
eed3si9n
requested changes
Jun 7, 2026
eed3si9n
left a comment
Member
There was a problem hiding this comment.
I feel like this PR combines a few changes. Would it possible to chunk it into cohesive pieces, both for ease of reviewing, but also so we can revert if we find issues later down the line?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Set as a DRAFT first to see if this is the direction the community wants to go.
Summary
Extends #1660 to the remaining reflection call sites in
ClassToAPI. Continues chipping away at sbt/sbt#117 (the oldest open sbt bug, July 2011) whereClassToAPI's reflection on a class triggersNoClassDefFoundError/TypeNotPresentExceptionwhen a referenced type lives in an optional dep that isn't on the analysis classpath.#1660 fixed the inner-class case by reading the
InnerClassesattribute directly. This PR applies the same pattern to the residual sites — methods, constructors, fields, annotations, and exceptions — by keeping reflection as the primary path and falling back to classfile parsing when reflection throwsLinkageError | TypeNotPresentException | ClassNotFoundException. The reflection path is unchanged, so the happy path has zero risk of behavioral regression.Why now
Our org is migrating onto Mill + Zinc; #1660 cleared the inner-class crashes but we still hit
ClassToAPIaborts on classes whose method return / parameter types reference optional deps (e.g. Spring'scompileOnly+ JBoss VFS pattern), and on methods declaring checked exceptions whose types aren't on the classpath.Approach
structure(c, enclPkg, cmap)becomes a dispatcher:The same wrap pattern is applied at
c.getAnnotationsandc.getTypeParametersintoDefinitions0. The fallback emits awarnso this rare path is observable in incremental compilation logs.Changes
ClassFile.scalaParsedAnnotation/ParsedAnnotationArgcase classes. Trait methodsannotations(attrs),parameterAnnotations(attrs),methodExceptions(attrs).AttributeInfopredicates forExceptions/RuntimeVisible+Invisible ParameterAnnotations.FieldOrMethodInfopredicates (isPrivate/isProtected/isFinal/isAbstract/isVarArgs/isBridge/isSynthetic/isConstructor/isStaticInit/isMain) and the correspondingACC_*constants.Parser.scalaelement_valueserializer covers all 13 kinds including nested annotations and arrays.ClassToAPI.scalastructure()dispatcher +structureFromClassfile.classAnnotationsSafeandclassTypeParametersSafewrappers at the class level.cf*helpers convertFieldOrMethodInfotoapi.Def/Val/Vardirectly. Inherited members carry the parent's(Class, ClassFile)so attribute byte indices resolve against the correct constant pool (a bug caught mid-refactor — without this, reads either return garbage or trip theentry.tag == ConstantUTF8assertion intoUTF8). Main-class detection moves to the classfile unconditionally (strictly more reliable; no transitive-dep resolution).DescriptorParser.scala(new)xsbti.api.Typewithout loading the referenced classes. MirrorsClassToAPI.PrimitiveRefs(I→scala.Int, etc.) so fallback-produced types hash-compatibly with the reflection path.ParserSpecification.scalamethodExceptionsagainstThread.sleep,FieldOrMethodInfopredicates against JDK, fullelement_valuekind coverage via inline-compiled fixture, parameter annotations indexed by position, bridge/synthetic on erasure bridge.ClassToAPISpecification.scala@throwsannotation emission, warn-on-fallback logging.Deferred (TODOs in source)
These are intentional follow-ups to keep this PR scoped:
Signatureattribute parser (JVMS §4.7.9) —classTypeParametersSafecurrently degrades to an empty type-parameter list when reflection onc.getTypeParametersthrows. A signature parser would also let the method/field fallbacks recover generic return types, generic parameter types, and generic field types. TODO onclassTypeParametersSafe.allSuperTypesSafereturnsSeq.emptyon reflection failure, which drops thecmap.inheritededges (incremental invalidation cascade) and the parent types in the instanceStructure(API hash). Right fix: retry with rawgetSuperclass/getInterfaces(no generics), then if that still throws, usecf.superClassName+cf.interfaceNamesdirectly. TODO onallSuperTypesSafe.c.getAnnotations()silently drops unresolvable annotation types rather than throwing, soclassAnnotationsSafedoesn't get a chance to fall back for that specific case. Pinned as a(limitation)test. Fix: switch class annotation reading off reflection entirely. TODO onclassAnnotationsSafe.