gnodet commented on code in PR #1144: URL: https://github.com/apache/maven-compiler-plugin/pull/1144#discussion_r4179220642
########## src/test/java/org/apache/maven/plugin/compiler/incremental/BytecodeAnalyzerTest.java: ########## @@ -0,0 +1,303 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.compiler.incremental; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class BytecodeAnalyzerTest { + + @TempDir + Path sourceDir; + + @TempDir + Path outputDir; + + @Test + void descriptorToReadableVoid() { + assertEquals("void", BytecodeAnalyzer.descriptorToReadable("V")); + } + + @Test + void descriptorToReadableBoolean() { + assertEquals("boolean", BytecodeAnalyzer.descriptorToReadable("Z")); + } + + @Test + void descriptorToReadableByte() { + assertEquals("byte", BytecodeAnalyzer.descriptorToReadable("B")); + } + + @Test + void descriptorToReadableChar() { + assertEquals("char", BytecodeAnalyzer.descriptorToReadable("C")); + } + + @Test + void descriptorToReadableShort() { + assertEquals("short", BytecodeAnalyzer.descriptorToReadable("S")); + } + + @Test + void descriptorToReadableInt() { + assertEquals("int", BytecodeAnalyzer.descriptorToReadable("I")); + } + + @Test + void descriptorToReadableLong() { + assertEquals("long", BytecodeAnalyzer.descriptorToReadable("J")); + } + + @Test + void descriptorToReadableFloat() { + assertEquals("float", BytecodeAnalyzer.descriptorToReadable("F")); + } + + @Test + void descriptorToReadableDouble() { + assertEquals("double", BytecodeAnalyzer.descriptorToReadable("D")); + } + + @Test + void descriptorToReadableObject() { + assertEquals("java.lang.String", BytecodeAnalyzer.descriptorToReadable("Ljava/lang/String;")); + } + + @Test + void descriptorToReadableArray() { + assertEquals("int[]", BytecodeAnalyzer.descriptorToReadable("[I")); + assertEquals("java.lang.String[]", BytecodeAnalyzer.descriptorToReadable("[Ljava/lang/String;")); + } + + @Test + void descriptorToReadableEmpty() { + assertEquals("", BytecodeAnalyzer.descriptorToReadable("")); + } + + @Test + void parseParamsMultiple() { + assertEquals("java.lang.String, int", BytecodeAnalyzer.parseParams("(Ljava/lang/String;I)V")); + } + + @Test + void parseParamsEmpty() { + assertEquals("", BytecodeAnalyzer.parseParams("()V")); + } + + @Test + void parseReturnType() { + assertEquals("java.lang.String", BytecodeAnalyzer.parseReturn("(I)Ljava/lang/String;")); + assertEquals("void", BytecodeAnalyzer.parseReturn("()V")); + assertEquals("int", BytecodeAnalyzer.parseReturn("()I")); + } + + @Test + void analyzePathProducesCorrectClassName() throws Exception { + CompilerTestHelper.writeSource( + sourceDir, + "test", + "Hello", + "package test; public class Hello { public String greet() { return \"hi\"; } }"); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + + Path classFile = outputDir.resolve("test/Hello.class"); + assertTrue(Files.exists(classFile)); + + var analysis = BytecodeAnalyzer.analyze(classFile); + assertEquals("test.Hello", analysis.className()); + assertNotNull(analysis.abiFingerprint()); + assertNotNull(analysis.abiCanonical()); + } + + @Test + void analyzeBytesEquivalentToAnalyzePath() throws Exception { + CompilerTestHelper.writeSource( + sourceDir, "test", "Hello", "package test; public class Hello { public int value() { return 42; } }"); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + + Path classFile = outputDir.resolve("test/Hello.class"); + var fromPath = BytecodeAnalyzer.analyze(classFile); + var fromBytes = BytecodeAnalyzer.analyze(Files.readAllBytes(classFile)); + + assertEquals(fromPath.className(), fromBytes.className()); + assertEquals(fromPath.abiFingerprint(), fromBytes.abiFingerprint()); + assertEquals(fromPath.abiCanonical(), fromBytes.abiCanonical()); + assertEquals(fromPath.signatureTypes(), fromBytes.signatureTypes()); + assertEquals(fromPath.implementationTypes(), fromBytes.implementationTypes()); + } + + @Test + void genericTypeChangeAffectsFingerprint() throws Exception { + // Compile with List<String> + CompilerTestHelper.writeSource(sourceDir, "test", "Generics", """ + package test; + public class Generics { + public java.util.List<String> getNames() { return null; } + public java.util.Map<String, Integer> getMap() { return null; } + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + var analysis1 = BytecodeAnalyzer.analyze(outputDir.resolve("test/Generics.class")); + + // Recompile with List<Integer> — erased descriptor is identical, + // but generic signature differs + Path srcFile = sourceDir.resolve("test/Generics.java"); + Files.writeString(srcFile, """ + package test; + public class Generics { + public java.util.List<Integer> getNames() { return null; } + public java.util.Map<String, Integer> getMap() { return null; } + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + var analysis2 = BytecodeAnalyzer.analyze(outputDir.resolve("test/Generics.class")); + + assertNotEquals( + analysis1.abiFingerprint(), + analysis2.abiFingerprint(), + "Changing List<String> to List<Integer> should change bytecode ABI fingerprint"); + assertTrue(analysis2.abiCanonical().contains("<sig:"), "Canonical form should include generic signatures"); + } + + @Test + void classLevelGenericSignatureAffectsFingerprint() throws Exception { + CompilerTestHelper.writeSource(sourceDir, "test", "Box", """ + package test; + public class Box<T> { + public T get() { return null; } + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + var analysis1 = BytecodeAnalyzer.analyze(outputDir.resolve("test/Box.class")); + + Files.writeString(sourceDir.resolve("test/Box.java"), """ + package test; + public class Box<T extends Comparable<T>> { + public T get() { return null; } + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + var analysis2 = BytecodeAnalyzer.analyze(outputDir.resolve("test/Box.class")); + + assertNotEquals( + analysis1.abiFingerprint(), + analysis2.abiFingerprint(), + "Changing type parameter bounds should change bytecode ABI fingerprint"); + } + + @Test + @org.junit.jupiter.api.condition.EnabledForJreRange(min = org.junit.jupiter.api.condition.JRE.JAVA_24) + void genericTypeArgumentsTrackedAsDependencies() throws Exception { + // Foo is a user-defined type used only as a generic type argument + CompilerTestHelper.writeSource(sourceDir, "test", "Foo", "package test; public class Foo {}"); + CompilerTestHelper.writeSource(sourceDir, "test", "Bar", "package test; public class Bar {}"); + CompilerTestHelper.writeSource(sourceDir, "test", "Subject", """ + package test; + import java.util.List; + import java.util.Map; + import java.util.function.Function; + public class Subject { + // Foo appears only as a type argument — not in the erased descriptor + public List<Foo> getItems() { return null; } + // Bar appears as a Map value type argument + public Map<String, Bar> getMap() { return null; } + // Wildcards: Foo as lower bound + public List<? extends Foo> getBounded() { return null; } + // Foo in field generic signature + public java.util.Optional<Foo> optFoo = java.util.Optional.empty(); + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + + var analysis = BytecodeAnalyzer.analyze(outputDir.resolve("test/Subject.class")); + + assertTrue( + analysis.signatureTypes().contains("test.Foo"), + "test.Foo used as generic type arg should be in signatureTypes; got: " + analysis.signatureTypes()); + assertTrue( + analysis.signatureTypes().contains("test.Bar"), + "test.Bar used as generic type arg should be in signatureTypes; got: " + analysis.signatureTypes()); + } + + @Test + @org.junit.jupiter.api.condition.EnabledForJreRange(min = org.junit.jupiter.api.condition.JRE.JAVA_24) + void genericSuperclassTypeArgsTracked() throws Exception { + CompilerTestHelper.writeSource(sourceDir, "test", "Item", "package test; public class Item {}"); + CompilerTestHelper.writeSource(sourceDir, "test", "ItemList", """ + package test; + import java.util.AbstractList; + // Item appears only in the class generic signature (extends AbstractList<Item>) + public class ItemList extends AbstractList<Item> { + @Override public Item get(int i) { return null; } + @Override public int size() { return 0; } + } + """); + CompilerTestHelper.compileAndAnalyze(sourceDir, outputDir); + + var analysis = BytecodeAnalyzer.analyze(outputDir.resolve("test/ItemList.class")); + + assertTrue( + analysis.signatureTypes().contains("test.Item"), + "test.Item as superclass type arg should be in signatureTypes; got: " + analysis.signatureTypes()); + } + Review Comment: Fixed in e474b58. Added `formalTypeParameterBoundsTracked()` and `multipleTypeParameterBoundsAllTracked()` tests to `BytecodeAnalyzerTest`, both confirming bounds like `<T extends MyBound>` appear in `signatureTypes`. ########## src/main/java24/org/apache/maven/plugin/compiler/incremental/ClassfileClassAnalyzer.java: ########## @@ -0,0 +1,614 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.compiler.incremental; + +import java.lang.classfile.Attributes; +import java.lang.classfile.ClassFile; +import java.lang.classfile.ClassModel; +import java.lang.classfile.FieldModel; +import java.lang.classfile.MethodModel; +import java.lang.classfile.attribute.ModuleAttribute; +import java.lang.classfile.attribute.ModuleProvideInfo; +import java.lang.classfile.attribute.ModuleRequireInfo; +import java.lang.classfile.instruction.FieldInstruction; +import java.lang.classfile.instruction.InvokeInstruction; +import java.lang.classfile.instruction.NewMultiArrayInstruction; +import java.lang.classfile.instruction.NewObjectInstruction; +import java.lang.classfile.instruction.TypeCheckInstruction; +import java.lang.reflect.AccessFlag; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +/** + * {@link ClassAnalyzer} implementation using the standard {@code java.lang.classfile} + * API, available since JDK 24. + * + * <p>This implementation is loaded reflectively by {@link BytecodeAnalyzer} when + * the running JVM version is 24 or later. It is compiled separately with + * {@code --release 24} to avoid a compile-time dependency on the classfile API + * in the main sources. + * + * <p>Type references are classified into two sets: + * <ul> + * <li><b>signatureTypes</b> — types from the public API surface: supertype, interfaces, + * field/method descriptors of non-private members, exception types, annotations. + * These are what downstream consumers structurally depend on.</li> + * <li><b>implementationTypes</b> — types referenced only in method body bytecode + * instructions (INVOKE*, field access, NEW, CHECKCAST, etc.), and descriptor + * types of private members. Changes to these do not cascade to the class's + * signature consumers.</li> + * </ul> + * + * <p>{@code module-info.class} is handled specially: its ABI fingerprint is derived + * from the {@code Module} attribute directives (requires, exports, opens, uses, + * provides), and no implementation types are collected. + * + * @see BytecodeAnalyzer + * @see ClassAnalyzer + */ +class ClassfileClassAnalyzer extends ClassAnalyzer { + + @Override + public BytecodeAnalyzer.ClassAnalysis analyze(byte[] classBytes) { + ClassModel cm = ClassFile.of().parse(classBytes); + + // module-info.class has the MODULE access flag + if (cm.flags().has(AccessFlag.MODULE)) { + return analyzeModuleInfo(cm); + } + + String className = BytecodeAnalyzer.toJavaName(cm.thisClass().asInternalName()); + Set<String> signatureTypes = new TreeSet<>(); + Set<String> implementationTypes = new TreeSet<>(); + Set<String> annotationTypes = new TreeSet<>(); + + collectTypes(cm, signatureTypes, implementationTypes, annotationTypes); + + // Remove self-references and JDK types + signatureTypes.remove(className); + implementationTypes.remove(className); + implementationTypes.removeAll(signatureTypes); // sig takes precedence + signatureTypes.removeIf(ClassfileClassAnalyzer::isJdkType); + implementationTypes.removeIf(ClassfileClassAnalyzer::isJdkType); + annotationTypes.removeIf(ClassfileClassAnalyzer::isJdkType); + + String abiCanonical = buildCanonical(cm); + String abiFingerprint = Sha256.hash(abiCanonical); + + // Read the SourceFile attribute for accurate source-file attribution + String sourceFileName = cm.findAttribute(Attributes.sourceFile()) + .map(sf -> sf.sourceFile().stringValue()) + .orElse(""); + + return new BytecodeAnalyzer.ClassAnalysis( + className, + abiFingerprint, + abiCanonical, + signatureTypes, + implementationTypes, + annotationTypes, + /* moduleName= */ "", + /* isModuleInfo= */ false, + sourceFileName); + } + + // --- module-info handling --- + + private static BytecodeAnalyzer.ClassAnalysis analyzeModuleInfo(ClassModel cm) { + var moduleAttrOpt = cm.findAttribute(Attributes.module()); + if (moduleAttrOpt.isEmpty()) { + return new BytecodeAnalyzer.ClassAnalysis( + "module-info", "", "", Set.of(), Set.of(), Set.of(), "", true, "module-info.java"); + } + + ModuleAttribute mod = moduleAttrOpt.get(); + String moduleName = mod.moduleName().name().stringValue(); + boolean isOpen = cm.flags().has(AccessFlag.OPEN); + + var requires = new TreeSet<String>(); + var exports = new TreeSet<String>(); + var opens = new TreeSet<String>(); + var uses = new TreeSet<String>(); + var provides = new TreeSet<String>(); + + for (ModuleRequireInfo req : mod.requires()) { + var sb = new StringBuilder("requires "); + if (req.requiresFlags().contains(AccessFlag.TRANSITIVE)) sb.append("transitive "); + if (req.requiresFlags().contains(AccessFlag.STATIC_PHASE)) sb.append("static "); + sb.append(req.requires().name().stringValue()); + requires.add(sb.toString()); + } + for (var exp : mod.exports()) { + var sb = new StringBuilder("exports ") + .append(exp.exportedPackage().name().stringValue().replace('/', '.')); + var tos = exp.exportsTo(); + if (!tos.isEmpty()) { + sb.append(" to ") + .append(tos.stream() + .map(e -> e.name().stringValue()) + .sorted() + .reduce((a, b) -> a + ", " + b) + .orElse("")); + } + exports.add(sb.toString()); + } + for (var op : mod.opens()) { + var sb = new StringBuilder("opens ") + .append(op.openedPackage().name().stringValue().replace('/', '.')); + var tos = op.opensTo(); + if (!tos.isEmpty()) { + sb.append(" to ") + .append(tos.stream() + .map(e -> e.name().stringValue()) + .sorted() + .reduce((a, b) -> a + ", " + b) + .orElse("")); + } + opens.add(sb.toString()); + } + for (var u : mod.uses()) { + uses.add("uses " + BytecodeAnalyzer.toJavaName(u.asInternalName())); + } + for (ModuleProvideInfo p : mod.provides()) { + var sb = new StringBuilder("provides ") + .append(BytecodeAnalyzer.toJavaName(p.provides().asInternalName())); + var impls = p.providesWith(); + if (!impls.isEmpty()) { + sb.append(" with ") + .append(impls.stream() + .map(i -> BytecodeAnalyzer.toJavaName(i.asInternalName())) + .sorted() + .reduce((a, b) -> a + ", " + b) + .orElse("")); + } + provides.add(sb.toString()); + } + + var canonical = new StringBuilder(); + if (isOpen) canonical.append("open "); + canonical.append("module ").append(moduleName).append('\n'); + for (String r : requires) canonical.append(" ").append(r).append('\n'); + for (String e : exports) canonical.append(" ").append(e).append('\n'); + for (String o : opens) canonical.append(" ").append(o).append('\n'); + for (String u : uses) canonical.append(" ").append(u).append('\n'); + for (String p : provides) canonical.append(" ").append(p).append('\n'); + + String abiCanonical = canonical.toString(); + String abiFingerprint = Sha256.hash(abiCanonical); + + // Signature deps for module-info: service types from uses/provides + var sigTypes = new TreeSet<String>(); + for (var u : mod.uses()) { + String name = BytecodeAnalyzer.toJavaName(u.asInternalName()); + if (!isJdkType(name)) sigTypes.add(name); + } + for (ModuleProvideInfo p : mod.provides()) { + String svc = BytecodeAnalyzer.toJavaName(p.provides().asInternalName()); + if (!isJdkType(svc)) sigTypes.add(svc); + for (var impl : p.providesWith()) { + String implName = BytecodeAnalyzer.toJavaName(impl.asInternalName()); + if (!isJdkType(implName)) sigTypes.add(implName); + } + } + + // module-info qualified name uses the MODULE_PREFIX convention + String qualifiedName = AbiIncrementalBuild.MODULE_PREFIX + moduleName; + + return new BytecodeAnalyzer.ClassAnalysis( + qualifiedName, + abiFingerprint, + abiCanonical, + sigTypes, + Set.of(), + Set.of(), + moduleName, + /* isModuleInfo= */ true, + /* sourceFileName= */ "module-info.java"); + } + + // --- type reference collection --- + + private static void collectTypes( + ClassModel cm, Set<String> signatureTypes, Set<String> implementationTypes, Set<String> annotationTypes) { + + // Superclass and interfaces are always signature-level + cm.superclass().ifPresent(sup -> addRef(sup.asInternalName(), signatureTypes)); + for (var iface : cm.interfaces()) { + addRef(iface.asInternalName(), signatureTypes); + } + + // Class generic signature (e.g. "class Foo<T extends Bar>") — extract type args + cm.findAttribute(Attributes.signature()) + .ifPresent(sig -> addGenericSignatureRefs(sig.signature().stringValue(), signatureTypes)); + + // Class-level annotations → signature + annotation tracking + collectAnnotations( + cm.findAttribute(Attributes.runtimeVisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + signatureTypes, + annotationTypes); + collectAnnotations( + cm.findAttribute(Attributes.runtimeInvisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + signatureTypes, + annotationTypes); + + for (FieldModel field : cm.fields()) { + boolean isPrivate = field.flags().has(AccessFlag.PRIVATE); + Set<String> descTarget = isPrivate ? implementationTypes : signatureTypes; + + addDescriptor(field.fieldType().stringValue(), descTarget); + + // Field generic signature — extract concrete type arguments (e.g. Foo in List<Foo>) + field.findAttribute(Attributes.signature()) + .ifPresent(sig -> addGenericSignatureRefs(sig.signature().stringValue(), descTarget)); + + // Field annotations go to the same target as the field descriptor + collectAnnotations( + field.findAttribute(Attributes.runtimeVisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + descTarget, + annotationTypes); + collectAnnotations( + field.findAttribute(Attributes.runtimeInvisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + descTarget, + annotationTypes); + } + + for (MethodModel method : cm.methods()) { + boolean isPrivate = method.flags().has(AccessFlag.PRIVATE); + Set<String> sigTarget = isPrivate ? implementationTypes : signatureTypes; + + // Method descriptor types (params + return) and exceptions → sig if non-private + addMethodDescriptor(method.methodType().stringValue(), sigTarget); + method.findAttribute(Attributes.exceptions()) + .ifPresent(ex -> ex.exceptions().forEach(e -> addRef(e.asInternalName(), sigTarget))); + + // Method generic signature — extract concrete type arguments (e.g. Foo in List<Foo>) + method.findAttribute(Attributes.signature()) + .ifPresent(sig -> addGenericSignatureRefs(sig.signature().stringValue(), sigTarget)); + + // Method annotations → same target as descriptor + collectAnnotations( + method.findAttribute(Attributes.runtimeVisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + sigTarget, + annotationTypes); + collectAnnotations( + method.findAttribute(Attributes.runtimeInvisibleAnnotations()) + .map(a -> a.annotations()) + .orElse(List.of()), + sigTarget, + annotationTypes); + + // Method body instructions → always implementation-level + method.code().ifPresent(code -> { + for (var element : code) { + switch (element) { + case InvokeInstruction ii -> addRef(ii.owner().asInternalName(), implementationTypes); + case FieldInstruction fi -> addRef(fi.owner().asInternalName(), implementationTypes); + case TypeCheckInstruction tci -> addRef(tci.type().asInternalName(), implementationTypes); + case NewObjectInstruction noi -> addRef(noi.className().asInternalName(), implementationTypes); + case NewMultiArrayInstruction nma -> + addRef(nma.arrayType().asInternalName(), implementationTypes); + default -> {} + } + } + }); + } + + // Scan the constant pool for class references that are not captured by instructions. + // This covers compile-time constants (static final primitives) whose values are inlined + // by javac — the referencing class has no GETSTATIC instruction, but the resolved class + // is still present in the constant pool. + for (var entry : cm.constantPool()) { + if (entry instanceof java.lang.classfile.constantpool.ClassEntry ce) { + String internalName = ce.asInternalName(); + // Skip array type descriptors and the class itself + if (!internalName.startsWith("[") + && !internalName.equals(cm.thisClass().asInternalName())) { + addRef(internalName, implementationTypes); + } + } + } + } + + private static void collectAnnotations( + java.util.List<? extends java.lang.classfile.Annotation> annotations, + Set<String> typeTarget, + Set<String> annotationTarget) { + for (var ann : annotations) { + String descriptor = ann.className().stringValue(); + String name = descriptorToJavaName(descriptor); + if (name != null) { + typeTarget.add(name); + annotationTarget.add(name); + } + } + } + + // --- ABI canonical form --- + + private static String buildCanonical(ClassModel cm) { + var fields = new ArrayList<FieldInfo>(); + var methods = new ArrayList<MethodInfo>(); + + for (FieldModel field : cm.fields()) { + int access = accessMask(field.flags().flags()); + if (!isPrivateOrSynthetic(access)) { + Object constantValue = field.findAttribute(Attributes.constantValue()) + .map(cv -> cv.constant().constantValue()) + .orElse(null); + String fieldSig = field.findAttribute(Attributes.signature()) + .map(s -> s.signature().stringValue()) + .orElse(null); + fields.add(new FieldInfo( + access, + field.fieldName().stringValue(), + field.fieldType().stringValue(), + constantValue, + fieldSig)); + } + } + + for (MethodModel method : cm.methods()) { + int access = accessMask(method.flags().flags()); + String name = method.methodName().stringValue(); + if (!isPrivateOrSynthetic(access) && !"<clinit>".equals(name)) { + String methodSig = method.findAttribute(Attributes.signature()) + .map(s -> s.signature().stringValue()) + .orElse(null); + methods.add(new MethodInfo(access, name, method.methodType().stringValue(), methodSig)); + } + } + + int classAccess = accessMask(cm.flags().flags()); + String className = BytecodeAnalyzer.toJavaName(cm.thisClass().asInternalName()); + String classSignature = cm.findAttribute(Attributes.signature()) + .map(s -> s.signature().stringValue()) + .orElse(null); + String superName = cm.superclass() + .map(sup -> BytecodeAnalyzer.toJavaName(sup.asInternalName())) + .orElse(null); + List<String> ifaceNames = cm.interfaces().stream() + .map(iface -> BytecodeAnalyzer.toJavaName(iface.asInternalName())) + .toList(); + + return buildCanonicalForm(classAccess, className, classSignature, superName, ifaceNames, fields, methods); + } + + // --- generic signature type reference extraction --- + + /** + * Extracts all concrete class type references from a JVM generic signature string + * (JVMS §4.7.9.1) and adds them to {@code types}. + * + * <p>Examples (signature → extracted types): + * <ul> + * <li>{@code Ljava/util/List<Lcom/example/Foo;>;} → {@code com.example.Foo} + * <li>{@code Ljava/util/Map<Lcom/example/Key;Lcom/example/Val;>;} → {@code com.example.Key}, {@code com.example.Val} + * <li>{@code (Lcom/example/Req;)Lcom/example/Resp;} → {@code com.example.Req}, {@code com.example.Resp} + * <li>{@code TT;} (type variable) → nothing + * <li>{@code +Lcom/Foo;} (wildcard) → {@code com.Foo} + * </ul> + * + * <p>Type variables ({@code TName;}) and primitive types are intentionally skipped — + * they are not concrete dependencies. + */ + private static void addGenericSignatureRefs(String sig, Set<String> types) { + if (sig == null || sig.isEmpty()) { + return; + } + int[] pos = {0}; + // A ClassSignature or MethodSignature may start with FormalTypeParameters: <T:Lbound;>... + // Detect this case and parse the FormalTypeParameters block with the dedicated mode. + if (sig.charAt(0) == '<') { + pos[0]++; // consume '<' + parseSig(sig, pos, types, true); // FormalTypeParameter mode + if (pos[0] < sig.length() && sig.charAt(pos[0]) == '>') { + pos[0]++; // consume '>' + } + } + // Parse the remainder (SuperclassSignature, SuperinterfaceSignatures, or method sig body) + parseSig(sig, pos, types, false); + } + + /** + * Recursive descent parser for JVM generic signatures (JVMS §4.7.9.1). + * {@code inFormalTypeParams} must be {@code true} when called from inside a + * {@code FormalTypeParameters} block ({@code <...>} at the top of a class or method + * signature), where the grammar is {@code Identifier ClassBound {InterfaceBound}} rather + * than {@code TypeArgument*}. All other call sites pass {@code false}. + */ + private static void parseSig(String sig, int[] pos, Set<String> types, boolean inFormalTypeParams) { + while (pos[0] < sig.length()) { + char c = sig.charAt(pos[0]); + if (inFormalTypeParams) { + // Inside FormalTypeParameters: Identifier ClassBound {InterfaceBound} + // Identifier is an arbitrary Java identifier (NOT prefixed by 'T') + // followed by ':' (ClassBound) or ':' (InterfaceBound) + if (c == '>') { + // End of FormalTypeParameters block — stop, let the caller consume '>' + return; + } + // Skip the Identifier (type parameter name, e.g. "T", "E", "Type") + while (pos[0] < sig.length() && sig.charAt(pos[0]) != ':' && sig.charAt(pos[0]) != '>') { + pos[0]++; + } + // Parse ClassBound and InterfaceBound(s): each is ':' followed by a FieldTypeSignature + while (pos[0] < sig.length() && sig.charAt(pos[0]) == ':') { + pos[0]++; // consume ':' + // ClassBound may be empty (just ':' with no FieldTypeSignature before next ':' or '>') + if (pos[0] < sig.length()) { + char next = sig.charAt(pos[0]); + if (next == 'L' || next == '[' || next == 'T') { + // There is a FieldTypeSignature — parse it as a TypeArgument (NOT inFormalTypeParams) + parseSig(sig, pos, types, false); Review Comment: Fixed in e474b58. The FTP bound parser no longer calls `parseSig(sig, pos, types, false)` for each bound (which looped to end-of-string). It now dispatches directly to `parseClassTypeSignature` for `L`, handles `[` and `T` inline — consuming exactly one `FieldTypeSignature` per bound. ########## pom.xml: ########## @@ -188,10 +188,88 @@ under the License. <excludes> <exclude>unit/**</exclude> </excludes> + <!-- + Tests run against the exploded classes directory, not the packaged JAR. + The JVM's multi-release JAR dispatch does not apply to directories, so + the JDK 24+ BytecodeAnalyzer override must be placed before the root stub + on the classpath. We prepend META-INF/versions/24/ via the system property + so it is resolved before target/classes/ by the bootstrap class loader. + --> + <classesDirectory>${project.build.outputDirectory}/META-INF/versions/24</classesDirectory> + <additionalClasspathElements> Review Comment: Fixed in 1ba1359. Comment now reads: "We prepend META-INF/versions/24/ via the classesDirectory parameter". ########## src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java: ########## @@ -915,6 +919,241 @@ private static boolean removeFirsts(Deque<Path> paths, Integer count) { } } + /** + * Compiles using the ABI-fingerprint incremental strategy. This method handles the full + * lifecycle: determining what to compile, running javac with the analysis TaskListener, + * cascading on ABI changes, and persisting state. + * + * @param compiler the compiler + * @param configuration the options to give to the Java compiler + * @param mojo the MOJO for configuration access + * @throws IOException if an error occurred while reading or writing a file + * @throws MojoException if the compilation failed + */ + void compileWithAbiIncremental(JavaCompiler compiler, final Options configuration, final AbstractCompilerMojo mojo) + throws IOException { + var abiBuild = new AbiIncrementalBuild(outputDirectory); + + // Collect classpath entries for cross-module tracking + var classpathPaths = new ArrayList<Path>(); + var reactorPaths = new LinkedHashSet<Path>(); + for (var entry : dependencies.entrySet()) { + if (entry.getKey() instanceof JavaPathType type) { + var location = type.location(); + if (location.isPresent() + && (location.get() == StandardLocation.CLASS_PATH + || location.get() == StandardLocation.MODULE_PATH)) { + classpathPaths.addAll(entry.getValue()); + if (location.get() == StandardLocation.MODULE_PATH) { + for (Path p : entry.getValue()) { + if (Files.isDirectory(p)) { + reactorPaths.add(p); + } + } + } + } + } + } + abiBuild.setClasspathEntries(classpathPaths); + if (!reactorPaths.isEmpty()) { + abiBuild.setReactorModulePaths(reactorPaths); + } + + // Collect annotation processor path for processor classification + var processorPaths = new ArrayList<Path>(); + for (var entry : dependencies.entrySet()) { + if (entry.getKey() instanceof JavaPathType type) { + var location = type.location(); + if (location.isPresent() + && (location.get() == StandardLocation.ANNOTATION_PROCESSOR_PATH + || location.get() == StandardLocation.ANNOTATION_PROCESSOR_MODULE_PATH)) { + processorPaths.addAll(entry.getValue()); + } + } + } + if (!processorPaths.isEmpty()) { + abiBuild.setProcessorPath(processorPaths); + } + + // Hash module-info-patch.maven files for config change detection + abiBuild.setConfigHash(computeConfigHash(configuration)); + + // Collect all source file paths + var allSourcePaths = new ArrayList<Path>(); + for (SourceFile sf : sourceFiles) { + allSourcePaths.add(sf.file); + } + + Set<Path> toCompile = abiBuild.initialize(allSourcePaths); + if (toCompile.isEmpty()) { + logger.info("Nothing to compile - all classes are up to date (ABI strategy)."); + abiBuild.finish(); + return; + } + + logger.info( + abiBuild.isFullBuild() + ? "Compiling " + toCompile.size() + " source file(s) (ABI: full build)." + : "Compiling " + toCompile.size() + " source file(s) (ABI: incremental)."); + if (mojo.showCompilationChanges && abiBuild.getRebuildCause() != null) { + logger.info("Rebuild cause: " + abiBuild.getRebuildCause()); + for (Path f : toCompile) { + logger.info(" " + f); + } + } + + var originalSourceFiles = new ArrayList<>(sourceFiles); + boolean success = true; + // Safety bound: the compile set is monotonically growing (bounded by total source count). + // If a bug causes processCompiledClasses to return files already compiled, this prevents + // an infinite loop. In practice this limit should never be reached. + int maxRounds = originalSourceFiles.size() + 1; + int rounds = 0; + + try { + while (!toCompile.isEmpty()) { + if (++rounds > maxRounds) { + throw new IllegalStateException("ABI cascade loop did not converge after " + maxRounds + + " rounds — " + "possible dependency cycle or bug in processCompiledClasses()"); + } + Set<Path> compileSet = toCompile; + sourceFiles = originalSourceFiles.stream() + .filter(sf -> compileSet.contains(sf.file)) + .collect(Collectors.toList()); + + if (sourceFiles.isEmpty()) { + break; + } + + var compilerOutput = new StringWriter(); + success = compileWithAbiAnalyzer(compiler, configuration, compilerOutput, abiBuild); + String output = compilerOutput.toString(); + if (!output.isBlank()) { + logger.warn(output); + } + if (!success) { + break; + } + + toCompile = abiBuild.processCompiledClasses(compileSet); + if (!toCompile.isEmpty()) { + logger.info("ABI cascade: recompiling " + toCompile.size() + " additional file(s)."); + if (mojo.showCompilationChanges) { + for (Path f : toCompile) { + logger.info(" " + f); + } + } + } + } + } finally { + sourceFiles = originalSourceFiles; + } + + if (success) { + abiBuild.finish(); + logger.info("Compiled " + abiBuild.compiledCount() + " file(s), " + abiBuild.unchangedCount() + + " unchanged (ABI strategy)."); + } else { + abiBuild.invalidate(); + throw new CompilationFailureException("Compilation failed (ABI incremental strategy)."); + } + } + + private String computeConfigHash(Options configuration) { + var digest = new StringBuilder(); + + // Include compiler options in the config hash so changes to -source/-target/-release + // etc. trigger a full rebuild under the ABI strategy. + String optionsRepr = String.join("|", configuration.options); + digest.append("opts:").append(optionsRepr).append(';'); + + for (SourceDirectory source : sourceDirectories) { + Path patchFile = source.root.resolve(ModuleInfoPatch.FILENAME); + if (Files.isRegularFile(patchFile)) { + try { + byte[] content = Files.readAllBytes(patchFile); + digest.append(patchFile) + .append(':') + .append(Sha256.hash(content)) + .append(';'); + } catch (IOException e) { + digest.append(patchFile).append(":unreadable;"); + } + } + } + if (digest.isEmpty()) { + return ""; + } + return Sha256.hash(digest.toString()); + } + + /** + * Compiles sources with the ABI analyzer attached as a TaskListener. Review Comment: Fixed in 1ba1359. Javadoc updated to: "Compiles sources and performs ABI analysis on the compiled classes." ########## pom.xml: ########## @@ -188,10 +188,88 @@ under the License. <excludes> <exclude>unit/**</exclude> </excludes> + <!-- + Tests run against the exploded classes directory, not the packaged JAR. + The JVM's multi-release JAR dispatch does not apply to directories, so + the JDK 24+ BytecodeAnalyzer override must be placed before the root stub + on the classpath. We prepend META-INF/versions/24/ via the system property + so it is resolved before target/classes/ by the bootstrap class loader. Review Comment: Fixed in 1ba1359. Comment now reads "via the classesDirectory parameter". -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
