From 320c8fed01b5bb62ea1219c3ee3c3e1cd8d332b4 Mon Sep 17 00:00:00 2001 From: forax Date: Sat, 12 Oct 2019 15:00:25 +0200 Subject: [PATCH] fix issues raised during code review --- .../asm/tree/RecordComponentNodeTest.java | 59 +++++++++++++++++++ .../java/org/objectweb/asm/util/ASMifier.java | 8 +-- .../org/objectweb/asm/util/Textifier.java | 3 + .../asm/util/TraceRecordComponentVisitor.java | 7 ++- .../jdk14.AllStructures$RecordSubType.txt | 4 +- .../java/org/objectweb/asm/ClassReader.java | 2 +- .../java/org/objectweb/asm/ClassVisitor.java | 2 +- .../java/org/objectweb/asm/ClassWriter.java | 16 ++--- .../objectweb/asm/RecordComponentVisitor.java | 20 +++---- .../objectweb/asm/RecordComponentWriter.java | 12 +++- 10 files changed, 101 insertions(+), 32 deletions(-) create mode 100644 asm-tree/src/test/java/org/objectweb/asm/tree/RecordComponentNodeTest.java diff --git a/asm-tree/src/test/java/org/objectweb/asm/tree/RecordComponentNodeTest.java b/asm-tree/src/test/java/org/objectweb/asm/tree/RecordComponentNodeTest.java new file mode 100644 index 00000000..4333a5aa --- /dev/null +++ b/asm-tree/src/test/java/org/objectweb/asm/tree/RecordComponentNodeTest.java @@ -0,0 +1,59 @@ +// ASM: a very small and fast Java bytecode manipulation framework +// Copyright (c) 2000-2011 INRIA, France Telecom +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions +// are met: +// 1. Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// 2. Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in the +// documentation and/or other materials provided with the distribution. +// 3. Neither the name of the copyright holders nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF +// THE POSSIBILITY OF SUCH DAMAGE. +package org.objectweb.asm.tree; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.objectweb.asm.test.AsmTest; + +/** + * Unit tests for {@link RecordComponentNode}. + * + * @author Eric Bruneton + */ +public class RecordComponentNodeTest extends AsmTest { + + @Test + public void testConstructor() { + RecordComponentNode fieldNode = new RecordComponentNode(123, "component", "I", null); + + assertEquals(123, fieldNode.accessExperimental); + assertEquals("component", fieldNode.nameExperimental); + assertEquals("I", fieldNode.descriptorExperimental); + } + + @Test + public void testConstructor_illegalState() { + Executable constructor = () -> new RecordComponentNode(123, "component", "I", null) {}; + + assertThrows(IllegalStateException.class, constructor); + } +} diff --git a/asm-util/src/main/java/org/objectweb/asm/util/ASMifier.java b/asm-util/src/main/java/org/objectweb/asm/util/ASMifier.java index 9949ebc4..b5dde311 100644 --- a/asm-util/src/main/java/org/objectweb/asm/util/ASMifier.java +++ b/asm-util/src/main/java/org/objectweb/asm/util/ASMifier.java @@ -612,7 +612,7 @@ public class ASMifier extends Printer { @Override public ASMifier visitRecordComponentAnnotationExperimental( final String descriptor, final boolean visible) { - // Use this call when not experimental anymore + // TODO Use this call when not experimental anymore // return visitAnnotation(descriptor, visible); stringBuilder.setLength(0); stringBuilder @@ -632,7 +632,7 @@ public class ASMifier extends Printer { @Override public ASMifier visitRecordComponentTypeAnnotationExperimental( final int typeRef, final TypePath typePath, final String descriptor, final boolean visible) { - // Use this call when not experimental anymore + // TODO Use this call when not experimental anymore // return visitTypeAnnotation(typeRef, typePath, descriptor, visible); return visitTypeAnnotation( "visitTypeAnnotationExperimental", typeRef, typePath, descriptor, visible); @@ -640,7 +640,7 @@ public class ASMifier extends Printer { @Override public void visitRecordComponentAttributeExperimental(final Attribute attribute) { - // Use this call when not experimental anymore + // TODO Use this call when not experimental anymore // visitAttribute(attribute); stringBuilder.setLength(0); stringBuilder.append("// ATTRIBUTE ").append(attribute.type).append('\n'); @@ -659,7 +659,7 @@ public class ASMifier extends Printer { @Override public void visitRecordComponentEndExperimental() { stringBuilder.setLength(0); - // Use this call when not experimental anymore + // TODO Use this call when not experimental anymore // stringBuilder.append(name).append(VISIT_END); stringBuilder.append(name).append(".visitEndExperimental();\n"); text.add(stringBuilder.toString()); diff --git a/asm-util/src/main/java/org/objectweb/asm/util/Textifier.java b/asm-util/src/main/java/org/objectweb/asm/util/Textifier.java index 201a2c3c..cc17d13a 100644 --- a/asm-util/src/main/java/org/objectweb/asm/util/Textifier.java +++ b/asm-util/src/main/java/org/objectweb/asm/util/Textifier.java @@ -345,6 +345,9 @@ public class Textifier extends Printer { appendJavaDeclaration(name, signature); } + stringBuilder.append(tab); + appendAccess(access); + appendDescriptor(FIELD_DESCRIPTOR, descriptor); stringBuilder.append(' ').append(name); diff --git a/asm-util/src/main/java/org/objectweb/asm/util/TraceRecordComponentVisitor.java b/asm-util/src/main/java/org/objectweb/asm/util/TraceRecordComponentVisitor.java index 0d51cd44..682bc1a4 100644 --- a/asm-util/src/main/java/org/objectweb/asm/util/TraceRecordComponentVisitor.java +++ b/asm-util/src/main/java/org/objectweb/asm/util/TraceRecordComponentVisitor.java @@ -34,7 +34,8 @@ import org.objectweb.asm.RecordComponentVisitor; import org.objectweb.asm.TypePath; /** - * A {@link RecordComponentVisitor} that prints the fields it visits with a {@link Printer}. + * A {@link RecordComponentVisitor} that prints the record components it visits with a {@link + * Printer}. * * @author Remi Forax * @deprecated This is an experimental API. @@ -52,7 +53,7 @@ public final class TraceRecordComponentVisitor extends RecordComponentVisitor { /** * Constructs a new {@link TraceRecordComponentVisitor}. * - * @param printer the printer to convert the visited field into text. + * @param printer the printer to convert the visited record component into text. * @deprecated This is an experimental API. */ @Deprecated @@ -65,7 +66,7 @@ public final class TraceRecordComponentVisitor extends RecordComponentVisitor { * * @param recordComponentVisitor the record component visitor to which to delegate calls. May be * {@literal null}. - * @param printer the printer to convert the visited field into text. + * @param printer the printer to convert the visited record component into text. * @deprecated This is an experimental API. */ @Deprecated diff --git a/asm-util/src/test/resources/jdk14.AllStructures$RecordSubType.txt b/asm-util/src/test/resources/jdk14.AllStructures$RecordSubType.txt index e8d1a276..381f21cf 100644 --- a/asm-util/src/test/resources/jdk14.AllStructures$RecordSubType.txt +++ b/asm-util/src/test/resources/jdk14.AllStructures$RecordSubType.txt @@ -10,11 +10,11 @@ public final class jdk14/AllStructures$RecordSubType extends java/lang/Record im public final static INNERCLASS java/lang/invoke/MethodHandles$Lookup java/lang/invoke/MethodHandles Lookup // access flags 0x0 -I component1 + I component1 @Ljdk14/Ann;() // invisible // access flags 0x0 -Ljava/lang/String; component2 + Ljava/lang/String; component2 // access flags 0x8012 private final mandated I component1 diff --git a/asm/src/main/java/org/objectweb/asm/ClassReader.java b/asm/src/main/java/org/objectweb/asm/ClassReader.java index d3b6f6e8..3d0d250b 100644 --- a/asm/src/main/java/org/objectweb/asm/ClassReader.java +++ b/asm/src/main/java/org/objectweb/asm/ClassReader.java @@ -857,7 +857,7 @@ public class ClassReader { * @param classVisitor the current class visitor * @param context information about the class being parsed. * @param recordComponentOffset the offset of the current record component. - * @return he offset of the first byte following the record component. + * @return the offset of the first byte following the record component. */ private int readRecordComponent( final ClassVisitor classVisitor, final Context context, final int recordComponentOffset) { diff --git a/asm/src/main/java/org/objectweb/asm/ClassVisitor.java b/asm/src/main/java/org/objectweb/asm/ClassVisitor.java index 7be6d1fb..cb911316 100644 --- a/asm/src/main/java/org/objectweb/asm/ClassVisitor.java +++ b/asm/src/main/java/org/objectweb/asm/ClassVisitor.java @@ -289,7 +289,7 @@ public abstract class ClassVisitor { * * @param access the record component access flags, the only possible value is {@link * Opcodes#ACC_DEPRECATED}. - * @param name the field's name. + * @param name the record component name. * @param descriptor the record component descriptor (see {@link Type}). * @param signature the record component signature. May be {@literal null} if the record component * type does not use generic types. diff --git a/asm/src/main/java/org/objectweb/asm/ClassWriter.java b/asm/src/main/java/org/objectweb/asm/ClassWriter.java index c7e9bb18..cb8877c5 100644 --- a/asm/src/main/java/org/objectweb/asm/ClassWriter.java +++ b/asm/src/main/java/org/objectweb/asm/ClassWriter.java @@ -185,15 +185,15 @@ public class ClassWriter extends ClassVisitor { /** * The record components of this class, stored in a linked list of {@link RecordComponentWriter} - * linked via their {@link RecordComponentWriter#rcv} field. This field stores the first element - * of this list. + * linked via their {@link RecordComponentWriter#delegate} field. This field stores the first + * element of this list. */ private RecordComponentWriter firstRecordComponent; /** * The record components of this class, stored in a linked list of {@link RecordComponentWriter} - * linked via their {@link RecordComponentWriter#rcv} field. This field stores the last element of - * this list. + * linked via their {@link RecordComponentWriter#delegate} field. This field stores the last + * element of this list. */ private RecordComponentWriter lastRecordComponent; @@ -414,7 +414,7 @@ public class ClassWriter extends ClassVisitor { if (firstRecordComponent == null) { firstRecordComponent = recordComponentWriter; } else { - lastRecordComponent.rcv = recordComponentWriter; + lastRecordComponent.delegate = recordComponentWriter; } return lastRecordComponent = recordComponentWriter; } @@ -581,7 +581,7 @@ public class ClassWriter extends ClassVisitor { while (recordComponentWriter != null) { ++recordComponentCount; recordSize += recordComponentWriter.computeRecordComponentInfoSize(); - recordComponentWriter = (RecordComponentWriter) recordComponentWriter.rcv; + recordComponentWriter = (RecordComponentWriter) recordComponentWriter.delegate; } ++attributesCount; size += 8 + recordSize; @@ -706,7 +706,7 @@ public class ClassWriter extends ClassVisitor { RecordComponentWriter recordComponentWriter = firstRecordComponent; while (recordComponentWriter != null) { recordComponentWriter.putRecordComponentInfo(result); - recordComponentWriter = (RecordComponentWriter) recordComponentWriter.rcv; + recordComponentWriter = (RecordComponentWriter) recordComponentWriter.delegate; } } if (firstAttribute != null) { @@ -780,7 +780,7 @@ public class ClassWriter extends ClassVisitor { RecordComponentWriter recordComponentWriter = firstRecordComponent; while (recordComponentWriter != null) { recordComponentWriter.collectAttributePrototypes(attributePrototypes); - recordComponentWriter = (RecordComponentWriter) recordComponentWriter.rcv; + recordComponentWriter = (RecordComponentWriter) recordComponentWriter.delegate; } return attributePrototypes.toArray(); } diff --git a/asm/src/main/java/org/objectweb/asm/RecordComponentVisitor.java b/asm/src/main/java/org/objectweb/asm/RecordComponentVisitor.java index 142eec89..3b1619d6 100644 --- a/asm/src/main/java/org/objectweb/asm/RecordComponentVisitor.java +++ b/asm/src/main/java/org/objectweb/asm/RecordComponentVisitor.java @@ -47,7 +47,7 @@ public abstract class RecordComponentVisitor { /** * The record visitor to which this visitor must delegate method calls. May be {@literal null}. */ - protected RecordComponentVisitor rcv; + protected RecordComponentVisitor delegate; /** * Constructs a new {@link RecordComponentVisitor}. @@ -84,7 +84,7 @@ public abstract class RecordComponentVisitor { Constants.checkAsm8Experimental(this); } this.api = api; - this.rcv = recordComponentVisitor; + this.delegate = recordComponentVisitor; } /** @@ -99,8 +99,8 @@ public abstract class RecordComponentVisitor { @Deprecated public AnnotationVisitor visitAnnotationExperimental( final String descriptor, final boolean visible) { - if (rcv != null) { - return rcv.visitAnnotationExperimental(descriptor, visible); + if (delegate != null) { + return delegate.visitAnnotationExperimental(descriptor, visible); } return null; } @@ -124,8 +124,8 @@ public abstract class RecordComponentVisitor { @Deprecated public AnnotationVisitor visitTypeAnnotationExperimental( final int typeRef, final TypePath typePath, final String descriptor, final boolean visible) { - if (rcv != null) { - return rcv.visitTypeAnnotationExperimental(typeRef, typePath, descriptor, visible); + if (delegate != null) { + return delegate.visitTypeAnnotationExperimental(typeRef, typePath, descriptor, visible); } return null; } @@ -138,8 +138,8 @@ public abstract class RecordComponentVisitor { */ @Deprecated public void visitAttributeExperimental(final Attribute attribute) { - if (rcv != null) { - rcv.visitAttributeExperimental(attribute); + if (delegate != null) { + delegate.visitAttributeExperimental(attribute); } } @@ -151,8 +151,8 @@ public abstract class RecordComponentVisitor { */ @Deprecated public void visitEndExperimental() { - if (rcv != null) { - rcv.visitEndExperimental(); + if (delegate != null) { + delegate.visitEndExperimental(); } } } diff --git a/asm/src/main/java/org/objectweb/asm/RecordComponentWriter.java b/asm/src/main/java/org/objectweb/asm/RecordComponentWriter.java index 5cf27823..c8e9a39d 100644 --- a/asm/src/main/java/org/objectweb/asm/RecordComponentWriter.java +++ b/asm/src/main/java/org/objectweb/asm/RecordComponentWriter.java @@ -31,6 +31,12 @@ final class RecordComponentWriter extends RecordComponentVisitor { /** Where the constants used in this RecordComponentWriter must be stored. */ private final SymbolTable symbolTable; + // Note: fields are ordered as in the component_info structure, and those related to attributes + // are ordered as in Section TODO of the JVMS. + // the field accessFlag doesn't exist in the component_info structure but is used to carry + // ACC_DEPRECATED which is represented by an attribute in the structure and as an access flag by + // ASM + /** The access_flags field can only be {@link Opcodes#ACC_DEPRECATED}. */ private final int accessFlags; @@ -158,7 +164,7 @@ final class RecordComponentWriter extends RecordComponentVisitor { * RecordComponentWriter. Also adds the names of the attributes of this record component in the * constant pool. * - * @return the size in bytes of the field_info JVMS structure. + * @return the size in bytes of the record_component_info of the Record attribute. */ int computeRecordComponentInfoSize() { // name_index, descriptor_index and attributes_count fields use 6 bytes. @@ -189,7 +195,7 @@ final class RecordComponentWriter extends RecordComponentVisitor { * Puts the content of the record component generated by this RecordComponentWriter into the given * ByteVector. * - * @param output where the field_info structure must be put. + * @param output where the record_component_info structure must be put. */ void putRecordComponentInfo(final ByteVector output) { output.putShort(nameIndex).putShort(descriptorIndex); @@ -232,7 +238,7 @@ final class RecordComponentWriter extends RecordComponentVisitor { } /** - * Collects the attributes of this field into the given set of attribute prototypes. + * Collects the attributes of this record component into the given set of attribute prototypes. * * @param attributePrototypes a set of attribute prototypes. */ -- GitLab