JDK-8336027 : Hide Transform implementation for Class-File API
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.lang.classfile
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 24
  • Submitted: 2024-07-09
  • Updated: 2024-07-13
  • Resolved: 2024-07-13
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Remove `ClassFileTransform.ResolvedTransform`, `ClassFileTransform::resolve`, and `ClassFileBuilder::canWriteDirect`. Update `ClassFileBuilder::transform` to return itself for chaining.

Problem
-------

The `ResolvedTransform` exposed is error-prone as users may miss to call the start or end handler. Users also have not enough resources to implement `resolve`, as the `ChainedBuilder` are only available to ClassFile API implementations. These APIs together are confusing to users who wish to transform class files. Also, `ClassFileBuilder` does not need to write constant pool indices, so it doesn't need `canWriteDirect` checks; only attribute mappers do. And there's no reason why `ClassFileBuilder::transform` does not support chaining calls like other builder methods.

Solution
--------

Remove these confusing APIs. Change `ClassFileBuilder::transform` to return the builder itself. Users wishing to use transform are advised to use `ClassFileBuilder::transform`, `ClassBuilder::transformMethod`, etc., and chaining transform will happen through `ClassFileTransform::andThen`.

An API specification update is planned later, after ClassFile API has been cleaned up; otherwise, specification changes are prone to outdated information and merge conflicts.

Specification
-------------

    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java b/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
    index 9381c0510ef..46dcb1cf375 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
    @@ -71,25 +72,18 @@ default void accept(E e) {
          */
         ConstantPoolBuilder constantPool();
     
    -    /**
    -     * {@return whether the provided constant pool is compatible with this builder}
    -     * @param source the constant pool to test compatibility with
    -     */
    -    default boolean canWriteDirect(ConstantPool source) {
    -        return constantPool().canWriteDirect(source);
    -    }
    -
         /**
          * Apply a transform to a model, directing results to this builder.
          * @param model the model to transform
          * @param transform the transform to apply
    +     * @return this builder
          */
    -    default void transform(CompoundElement<E> model, ClassFileTransform<?, E, B> transform) {
    +    default B transform(CompoundElement<E> model, ClassFileTransform<?, E, B> transform) {
             @SuppressWarnings("unchecked")
             B builder = (B) this;
    -        var resolved = transform.resolve(builder);
    +        var resolved = TransformImpl.resolve(transform, builder);
             resolved.startHandler().run();
             model.forEachElement(resolved.consumer());
             resolved.endHandler().run();
    +        return builder;
         }
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java b/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
    index f67da06a36d..851f4deb03e 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
    @@ -124,46 +123,4 @@ default void atStart(B builder) {
          * @return the chained transform
          */
         C andThen(C next);
    -
    -    /**
    -     * The result of binding a transform to a builder.  Used primarily within
    -     * the implementation to perform transformation.
    -     *
    -     * @param <E> the element type
    -     *
    -     * @since 22
    -     */
    -    @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
    -    interface ResolvedTransform<E extends ClassFileElement> {
    -        /**
    -         * {@return a {@link Consumer} to receive elements}
    -         */
    -        Consumer<E> consumer();
    -
    -        /**
    -         * {@return an action to call at the end of transformation}
    -         */
    -        Runnable endHandler();
    -
    -        /**
    -         * {@return an action to call at the start of transformation}
    -         */
    -        Runnable startHandler();
    -    }
    -
    -    /**
    -     * Bind a transform to a builder.  If the transform is chained, intermediate
    -     * builders are created for each chain link.  If the transform is stateful
    -     * (see, e.g., {@link ClassTransform#ofStateful(Supplier)}), the supplier is
    -     * invoked to get a fresh transform object.
    -     *
    -     * <p>This method is a low-level method that should rarely be used by
    -     * user code; most of the time, user code should prefer
    -     * {@link ClassFileBuilder#transform(CompoundElement, ClassFileTransform)},
    -     * which resolves the transform and executes it on the current builder.
    -     *
    -     * @param builder the builder to bind to
    -     * @return the bound result
    -     */
    -    ResolvedTransform<E> resolve(B builder);
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassTransform.java b/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
    index a2391b320f9..743a3985114 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
    @@ -171,15 +171,4 @@ static ClassTransform transformingFields(FieldTransform xform) {
         default ClassTransform andThen(ClassTransform t) {
             return new TransformImpl.ChainedClassTransform(this, t);
         }
    -
    -    /**
    -     * @implSpec The default implementation returns a resolved transform bound
    -     *           to the given class builder.
    -     */
    -    @Override
    -    default ResolvedTransform<ClassElement> resolve(ClassBuilder builder) {
    -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
    -                                                         () -> atEnd(builder),
    -                                                         () -> atStart(builder));
    -    }
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/CodeTransform.java b/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
    index a0996a9cb34..cdc7a3b1434 100644
    --- a/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
    +++ b/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
    @@ -96,15 +96,4 @@ public void atEnd(CodeBuilder builder) {
         default CodeTransform andThen(CodeTransform t) {
             return new TransformImpl.ChainedCodeTransform(this, t);
         }
    -
    -    /**
    -     * @implSpec The default implementation returns a resolved transform bound
    -     *           to the given code builder.
    -     */
    -    @Override
    -    default ResolvedTransform<CodeElement> resolve(CodeBuilder builder) {
    -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
    -                                                         () -> atEnd(builder),
    -                                                         () -> atStart(builder));
    -    }
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/FieldTransform.java b/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
    index 1a27a1d6ee6..4e39f1e9c7f 100644
    --- a/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
    +++ b/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
    @@ -111,15 +111,4 @@ static FieldTransform dropping(Predicate<FieldElement> filter) {
         default FieldTransform andThen(FieldTransform t) {
             return new TransformImpl.ChainedFieldTransform(this, t);
         }
    -
    -    /**
    -     * @implSpec The default implementation returns a resolved transform bound
    -     *           to the given field builder.
    -     */
    -    @Override
    -    default ResolvedTransform<FieldElement> resolve(FieldBuilder builder) {
    -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
    -                                                         () -> atEnd(builder),
    -                                                         () -> atStart(builder));
    -    }
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/MethodTransform.java b/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
    index 829ca041c5a..e7e024ebc34 100644
    --- a/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
    +++ b/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
    @@ -111,17 +111,6 @@ static MethodTransform transformingCode(CodeTransform xform) {
             return new TransformImpl.MethodCodeTransform(xform);
         }
     
    -    /**
    -     * @implSpec The default implementation returns a resolved transform bound
    -     *           to the given method builder.
    -     */
    -    @Override
    -    default ResolvedTransform<MethodElement> resolve(MethodBuilder builder) {
    -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
    -                                                         () -> atEnd(builder),
    -                                                         () -> atStart(builder));
    -    }
    -
         /**
          * @implSpec
          * The default implementation returns this method transform chained with another
    

Comments
Moving to Approved.
13-07-2024