You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org> on 2024/03/19 20:36:08 UTC
[PR] Support for java 14 Record feature [commons-bcel]
PabloNicolasDiaz opened a new pull request, #290:
URL: https://github.com/apache/commons-bcel/pull/290
This PR adds support for java 14 record feature (https://openjdk.org/jeps/359). Users now can use `JavaClass.isRecord()` to detect a record class.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1542274040
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+/**
+ * Record component info from a record. Instances from this class maps
+ * every component from a given record.
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30">
+ * The Java Virtual Machine Specification, Java SE 14 Edition, Records (preview)</a>
+ */
+public class RecordComponentInfo implements Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ public RecordComponentInfo(final DataInput input, ConstantPool constantPool) throws IOException {
+ this.index = input.readUnsignedShort();
+ this.descriptorIndex = input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ this.attributes = new Attribute[attributesCount];
+ for (int j = 0; j < attributesCount; j++) {
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ }
+ this.constantPool = constantPool;
+ }
+
+ public void dump(DataOutputStream file) throws IOException {
+ file.writeShort(index);
+ file.writeShort(descriptorIndex);
+ file.writeShort(attributes.length);
+ for (final Attribute attribute : attributes) {
+ attribute.dump(file);
+ }
+ }
+
+ @Override
+ public void accept(Visitor v) {
+ v.visitRecordComponent(this);
+ }
+
+ public int getIndex() {
Review Comment:
I made the getters `getIndex()`, `getDescriptorIndex()` and `getAttributes()` to give users access to this data. I'm just pushing a commit right now with the corresponding javadoc comment and covered them in `RecordTestCase`
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "markro49 (via GitHub)" <gi...@apache.org>.
markro49 commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545511096
##########
src/test/java/org/apache/bcel/classfile/RecordTestCase.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.bcel.AbstractTestCase;
+import org.apache.bcel.util.SyntheticRepository;
+import org.apache.bcel.visitors.CountingVisitor;
+import org.junit.jupiter.api.Test;
+
+public class RecordTestCase extends AbstractTestCase {
+
+ /**
+ * A record type, once compiled, should result in a class file that is
+ * marked such that we can determine from the access flags
+ * (through BCEL) that it is in fact a record.
+ *
+ * @throws IOException
+ * @throws ClassFormatException
+ */
+ @Test
+ public void testRecordClassSaysItIs() throws ClassNotFoundException, ClassFormatException, IOException {
+ final JavaClass clazz = new ClassParser("src/test/resources/record/SimpleRecord.class").parse();
Review Comment:
Should SimpleRecord.java be added to the resources/record directory for backup/documentation purposes?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545650255
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -237,4 +249,15 @@ default void visitStackMapType(final StackMapType obj) {
void visitSynthetic(Synthetic obj);
void visitUnknown(Unknown obj);
+
+ /**
+ * Visits a {@link RecordComponentInfo} object.
+ *
+ * @param obj record component to visit
Review Comment:
This causes:
```
[ERROR] /Users/garydgregory/git/commons-bcel/src/main/java/org/apache/bcel/classfile/Visitor.java:256: error: @param name not found
[ERROR] * @param obj record component to visit
[ERROR] ^
[ERROR] 1 error
[ERROR] 100 warnings
```
when I run `mvn clean clean install site -P jacoco`
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 14 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1531140604
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+import org.apache.commons.lang3.ArrayUtils;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and interfaces that are authorized to claim
+ * membership in the nest hosted by the current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool) throws IOException {
+ this(nameIndex, length, (RecordComponentInfo[]) null, constantPool);
+ final int classCount = input.readUnsignedShort();
+ components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ final int index = input.readUnsignedShort();
+ final int descriptorIndex =input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ final Attribute[] attributes = new Attribute[attributesCount];
+ for (int j = 0 ; j< attributesCount; j++)
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ components[i] = new RecordComponentInfo(
+ index,
+ descriptorIndex,
+ attributes,input,constantPool);
+ }
+ }
+
+ /**
+ * @param nameIndex Index in constant pool
Review Comment:
Javadoc's first sentence is missing.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+public class RecordComponentInfo implements Cloneable, Node {
Review Comment:
Must the class be public. Let's not make it `Cloneable` if we can. In general, we want to move away from Cloneable and toward serialization proxies instead.
##########
src/test/java/org/apache/bcel/CounterVisitorTestCase.java:
##########
@@ -217,4 +217,8 @@ public void testSyntheticCount() {
public void testUnknownCount() {
assertEquals(0, getVisitor().unknownCount, "unknownCount");
}
+ @Test
Review Comment:
Please add a blank line to separate methods (watch out for trailing whitespace).
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -207,6 +207,14 @@ default void visitNestMembers(final NestMembers obj) {
// empty
}
+ /**
+ * @since 6.5.0
Review Comment:
The next version will be 6.9.0.
##########
src/main/java/org/apache/bcel/classfile/JavaClass.java:
##########
@@ -906,4 +910,22 @@ public String toString() {
}
return buf.toString();
}
+
+ public boolean isRecord() {
Review Comment:
Javadoc missing, the next version will be 6.9.0 since this is a PR for a new feature.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+public class RecordComponentInfo implements Cloneable, Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ public RecordComponentInfo(int index, int descriptorIndex, Attribute[] attributes, DataInput input,
+ ConstantPool constantPool) {
+ this.index=index;
+ this.descriptorIndex=descriptorIndex;
+ this.attributes=attributes;
+ this.constantPool=constantPool;
+ }
+
+ public void write(DataOutputStream file) throws IOException {
Review Comment:
Javadoc missing.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+public class RecordComponentInfo implements Cloneable, Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ public RecordComponentInfo(int index, int descriptorIndex, Attribute[] attributes, DataInput input,
Review Comment:
Javadoc missing.
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+import org.apache.commons.lang3.ArrayUtils;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and interfaces that are authorized to claim
+ * membership in the nest hosted by the current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool) throws IOException {
+ this(nameIndex, length, (RecordComponentInfo[]) null, constantPool);
+ final int classCount = input.readUnsignedShort();
+ components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ final int index = input.readUnsignedShort();
+ final int descriptorIndex =input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ final Attribute[] attributes = new Attribute[attributesCount];
+ for (int j = 0 ; j< attributesCount; j++)
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ components[i] = new RecordComponentInfo(
+ index,
+ descriptorIndex,
+ attributes,input,constantPool);
+ }
+ }
+
+ /**
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param classes Table of Record component info
+ * @param constantPool Array of constants
+ */
+ public Record(final int nameIndex, final int length, final RecordComponentInfo[] classes, final ConstantPool constantPool) {
+ super(Const.ATTR_RECORD, nameIndex, length, constantPool);
+ this.components = classes != null ? classes : EMPTY_RCI_ARRAY;
+ Args.requireU2(this.components.length, "attributes.length");
+ }
+
+ /**
+ * Initialize from another object. Note that both objects use the same references (shallow copy). Use copy() for a
+ * physical copy.
+ *
+ * @param c Source to copy.
+ */
+ public Record(final Record c) {
+ this(c.getNameIndex(), c.getLength(), c.getComponents(), c.getConstantPool());
+ }
+
+ /**
+ * Called by objects that are traversing the nodes of the tree implicitly defined by the contents of a Java class.
+ * I.e., the hierarchy of methods, fields, attributes, etc. spawns a tree of objects.
+ *
+ * @param v Visitor object
+ */
+ @Override
+ public void accept(final Visitor v) {
+ v.visitRecord(this);
+ }
+
+ /**
+ * @return deep copy of this attribute
Review Comment:
Javadoc's first sentence is missing.
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+import org.apache.commons.lang3.ArrayUtils;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and interfaces that are authorized to claim
+ * membership in the nest hosted by the current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool) throws IOException {
+ this(nameIndex, length, (RecordComponentInfo[]) null, constantPool);
+ final int classCount = input.readUnsignedShort();
+ components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ final int index = input.readUnsignedShort();
+ final int descriptorIndex =input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ final Attribute[] attributes = new Attribute[attributesCount];
+ for (int j = 0 ; j< attributesCount; j++)
+ attributes[j] = Attribute.readAttribute(input, constantPool);
Review Comment:
Always use `{` and `}`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2015961970
@PabloNicolasDiaz
For nth time, you MUST run `mvn` (no extra arguments needed) on the command line before you push and fix issues, otherwise you'll just keep on breaking PR builds :-(
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2015682627
Hello @garydgregory, I've rebased and added a new test to check dump functionality and removed unnecessary methods. I've run the coverage report and its much better now
Thanks for your time!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2028731158
@PabloNicolasDiaz
The API `RecordComponentInfo#getConstantPool()` is neither used nor tested.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2014928831
@PabloNicolasDiaz
Again, rebase on git master if you haven't, run `mvn` (or the same as `maven.yml`: `mvn --show-version --batch-mode --no-transfer-progress -DtrimStackTrace=false -Ddoclint=none`), and fix issues, starting with:
```
git apply ~/Downloads/290.patch
/Users/.../Downloads/290.patch:73: trailing whitespace.
/Users/.../Downloads/290.patch:80: trailing whitespace.
/Users/.../Downloads/290.patch:93: trailing whitespace.
/Users/garydgregory/Downloads/290.patch:124: trailing whitespace.
/Users/garydgregory/Downloads/290.patch:135: trailing whitespace.
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545655543
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -237,4 +249,15 @@ default void visitStackMapType(final StackMapType obj) {
void visitSynthetic(Synthetic obj);
void visitUnknown(Unknown obj);
+
+ /**
+ * Visits a {@link RecordComponentInfo} object.
+ *
+ * @param obj record component to visit
Review Comment:
Please rebase on git master to see this error in the CI build. For some reason, we had doclint disabled.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 14 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2008175461
@PabloNicolasDiaz
The build is broken with:
```
Error: Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.18.5:cmp (default-cli) on project bcel: There is at least one incompatibility: org.apache.bcel.classfile.Visitor.visitRecordComponent(org.apache.bcel.classfile.RecordComponentInfo):METHOD_ADDED_TO_INTERFACE -> [Help 1]
```
Use a default method.
Run `mvn` by itself before you push to catch all build checks.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2016485841
Hello @PabloNicolasDiaz
Thank you for your update. Some new methods are still 0% tested, for example the new `accept` methods:
<img width="1102" alt="image" src="https://github.com/apache/commons-bcel/assets/1187639/1ce716dd-4551-4406-98ab-7e2f58720601">
and
<img width="1058" alt="image" src="https://github.com/apache/commons-bcel/assets/1187639/502a403f-8d4e-4501-8ad4-3e095709e5f1">
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2008356001
## [Codecov](https://app.codecov.io/gh/apache/commons-bcel/pull/290?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `50.56180%` with `44 lines` in your changes are missing coverage. Please review.
> Project coverage is 64.49%. Comparing base [(`f32c53c`)](https://app.codecov.io/gh/apache/commons-bcel/commit/f32c53cd1690d5cc4edf5e1939ec0e438e94f140?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`ce967c6`)](https://app.codecov.io/gh/apache/commons-bcel/pull/290?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 35 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/commons-bcel/pull/290?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rc/main/java/org/apache/bcel/classfile/Record.java](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1JlY29yZC5qYXZh) | 53.84% | [16 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...org/apache/bcel/classfile/RecordComponentInfo.java](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1JlY29yZENvbXBvbmVudEluZm8uamF2YQ==) | 55.55% | [11 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...a/org/apache/bcel/classfile/DescendingVisitor.java](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0Rlc2NlbmRpbmdWaXNpdG9yLmphdmE=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...main/java/org/apache/bcel/classfile/JavaClass.java](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0phdmFDbGFzcy5qYXZh) | 66.66% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...c/main/java/org/apache/bcel/classfile/Visitor.java](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1Zpc2l0b3IuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/commons-bcel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #290 +/- ##
============================================
- Coverage 65.10% 64.49% -0.62%
+ Complexity 3893 3868 -25
============================================
Files 364 366 +2
Lines 15657 15745 +88
Branches 1956 1962 +6
============================================
- Hits 10194 10154 -40
- Misses 4549 4674 +125
- Partials 914 917 +3
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-bcel/pull/290?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 14 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2008164954
@PabloNicolasDiaz
Thank you for your PR.
I fixed a whole set of whitespace issues.
1. Please rebase on Git master to pick up a new version of our Checkstyle configuration, then
2. Run `mvn` by itself to run all build checks or `mvn checkstyle:check' to detect the remaining whitespace issues in this PR
3. Fix whatever the above detects then push your changes.
Question: Does the new class `RecordComponentInfo` need to be public? The general idea is to make as little as possible public.
Missing: All new public and protected items need Javadoc, and for some new items this PR is missing the first sentence in the Javadoc.
TY!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 14 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2008184067
Hello @garydgregory, let me figure out if i can solve all the errors and if RecordComponentInfo needs to be acceder to users. RecordComponentInfo should be mapped to every `components` element in a record attribute (it's documented in https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30).
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2010811757
Hello @PabloNicolasDiaz
Thank you for your update. The current code coverage is poor as reported by JaCoCo, for example, the new Record class is only at 66% and 58% for branches.
<img width="1096" alt="image" src="https://github.com/apache/commons-bcel/assets/1187639/6faf192c-8925-4f61-abb1-2038cd0b5fbe">
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 14 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1531181712
##########
src/test/java/org/apache/bcel/CounterVisitorTestCase.java:
##########
@@ -217,4 +217,8 @@ public void testSyntheticCount() {
public void testUnknownCount() {
assertEquals(0, getVisitor().unknownCount, "unknownCount");
}
+ @Test
Review Comment:
fixed in https://github.com/apache/commons-bcel/pull/290/commits/ed41dba669d77ce3a7fca96471691eef4af9a79d
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1544870277
##########
src/test/java/org/apache/bcel/classfile/RecordTestCase.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.bcel.AbstractTestCase;
+import org.apache.bcel.util.SyntheticRepository;
+import org.apache.bcel.visitors.CountingVisitor;
+import org.junit.jupiter.api.Test;
+
+public class RecordTestCase extends AbstractTestCase {
+
+ /**
+ * A record type, once compiled, should result in a class file that is
+ * marked such that we can determine from the access flags
+ * (through BCEL) that it is in fact a record.
+ *
+ * @throws IOException
+ * @throws ClassFormatException
+ */
+ @Test
+ public void testRecordClassSaysItIs() throws ClassNotFoundException, ClassFormatException, IOException {
+ final JavaClass clazz = new ClassParser("src/test/resources/record/SimpleRecord.class").parse();
+ assertTrue(clazz.isRecord(), "Expected SimpleEnum class to say it was a record - but it didn't !");
Review Comment:
SimpleEnum -> SimpleRecord in test failure message
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2009660812
@PabloNicolasDiaz
It does not look like you rebased on git master and run `mvn` locally because this fails for me:
```
git apply ~/Downloads/290.diff.txt
.../Downloads/290.diff.txt:402: trailing whitespace.
.../Downloads/290.diff.txt:445: trailing whitespace.
* @throws IOException
.../Downloads/290.diff.txt:446: trailing whitespace.
* @throws ClassFormatException
.../Downloads/290.diff.txt:455: trailing whitespace.
../Downloads/290.diff.txt:459: trailing whitespace.
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2025100267
@markro49
Would you please review this PR?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1548606847
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -237,4 +249,15 @@ default void visitStackMapType(final StackMapType obj) {
void visitSynthetic(Synthetic obj);
void visitUnknown(Unknown obj);
+
+ /**
+ * Visits a {@link RecordComponentInfo} object.
+ *
+ * @param obj record component to visit
Review Comment:
@PabloNicolasDiaz
You still have not fixed the above, the build fails.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2033457159
Hi and sorry @garydgregory :(, I have run `mvn` but didn't see the error in this class. I've fixed it in my last commit
Thanks for your time and sorry again
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1539156648
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+/**
+ * Record component info from a record. Instances from this class maps
+ * every component from a given record.
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30">
+ * The Java Virtual Machine Specification, Java SE 14 Edition, Records (preview)</a>
+ */
+public class RecordComponentInfo implements Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ public RecordComponentInfo(final DataInput input, ConstantPool constantPool) throws IOException {
+ this.index = input.readUnsignedShort();
+ this.descriptorIndex = input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ this.attributes = new Attribute[attributesCount];
+ for (int j = 0; j < attributesCount; j++) {
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ }
+ this.constantPool = constantPool;
+ }
+
+ public void dump(DataOutputStream file) throws IOException {
+ file.writeShort(index);
+ file.writeShort(descriptorIndex);
+ file.writeShort(attributes.length);
+ for (final Attribute attribute : attributes) {
+ attribute.dump(file);
+ }
+ }
+
+ @Override
+ public void accept(Visitor v) {
+ v.visitRecordComponent(this);
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getDescriptorIndex() {
+ return descriptorIndex;
+ }
+
+ public Attribute[] getAttributes() {
Review Comment:
Hi @PabloNicolasDiaz
This method is neither used or tested, how about removing until it is needed?
Or, is it used by you in an other app?
It is also missing a Javadoc.
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -207,6 +207,14 @@ default void visitNestMembers(final NestMembers obj) {
// empty
}
+ /**
+ * @since 6.9.0
Review Comment:
Hi @PabloNicolasDiaz
The Javadoc is missing the first sentence.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+/**
+ * Record component info from a record. Instances from this class maps
+ * every component from a given record.
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30">
+ * The Java Virtual Machine Specification, Java SE 14 Edition, Records (preview)</a>
+ */
+public class RecordComponentInfo implements Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ public RecordComponentInfo(final DataInput input, ConstantPool constantPool) throws IOException {
+ this.index = input.readUnsignedShort();
+ this.descriptorIndex = input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ this.attributes = new Attribute[attributesCount];
+ for (int j = 0; j < attributesCount; j++) {
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ }
+ this.constantPool = constantPool;
+ }
+
+ public void dump(DataOutputStream file) throws IOException {
+ file.writeShort(index);
+ file.writeShort(descriptorIndex);
+ file.writeShort(attributes.length);
+ for (final Attribute attribute : attributes) {
+ attribute.dump(file);
+ }
+ }
+
+ @Override
+ public void accept(Visitor v) {
+ v.visitRecordComponent(this);
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getDescriptorIndex() {
Review Comment:
Hi @PabloNicolasDiaz
This method is neither used or tested, how about removing until it is needed?
Or, is it used by you in an other app?
It is also missing a Javadoc.
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -237,4 +245,14 @@ default void visitStackMapType(final StackMapType obj) {
void visitSynthetic(Synthetic obj);
void visitUnknown(Unknown obj);
+
+ /**
+ * Visits a {@link RecordComponentInfo} object.
+ * @param obj record component to visit
+ * @since 6.9.0
+ */
+ default void visitRecordComponent(RecordComponentInfo obj) {
+
Review Comment:
Remove blank line or add a comment that says `// noop`.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+/**
+ * Record component info from a record. Instances from this class maps
+ * every component from a given record.
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30">
+ * The Java Virtual Machine Specification, Java SE 14 Edition, Records (preview)</a>
+ */
+public class RecordComponentInfo implements Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ public RecordComponentInfo(final DataInput input, ConstantPool constantPool) throws IOException {
+ this.index = input.readUnsignedShort();
+ this.descriptorIndex = input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ this.attributes = new Attribute[attributesCount];
+ for (int j = 0; j < attributesCount; j++) {
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ }
+ this.constantPool = constantPool;
+ }
+
+ public void dump(DataOutputStream file) throws IOException {
+ file.writeShort(index);
+ file.writeShort(descriptorIndex);
+ file.writeShort(attributes.length);
+ for (final Attribute attribute : attributes) {
+ attribute.dump(file);
+ }
+ }
+
+ @Override
+ public void accept(Visitor v) {
+ v.visitRecordComponent(this);
+ }
+
+ public int getIndex() {
Review Comment:
Hi @PabloNicolasDiaz
This method is neither used or tested, how about removing until it is needed?
Or, is it used by you in an other app?
It is also missing a Javadoc.
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and
+ * interfaces that are authorized to claim membership in the nest hosted by the
+ * current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ this(nameIndex, length, readComponents(input, constantPool), constantPool);
+ }
+
+ private static RecordComponentInfo[] readComponents(final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ final int classCount = input.readUnsignedShort();
+ final RecordComponentInfo[] components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ components[i] = new RecordComponentInfo(input, constantPool);
+ }
+ return components;
+ }
+
+ /**
+ * Construct elements using its components
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param classes Array of Record Component Info elements
+ * @param constantPool Array of constants
+ */
+ public Record(final int nameIndex, final int length, final RecordComponentInfo[] classes,
+ final ConstantPool constantPool) {
+ super(Const.ATTR_RECORD, nameIndex, length, constantPool);
+ this.components = classes != null ? classes : EMPTY_RCI_ARRAY;
+ Args.requireU2(this.components.length, "attributes.length");
+ }
+
+ /**
+ * Called by objects that are traversing the nodes of the tree implicitly
+ * defined by the contents of a Java class. I.e., the hierarchy of methods,
+ * fields, attributes, etc. spawns a tree of objects.
+ *
+ * @param v Visitor object
+ */
+ @Override
+ public void accept(final Visitor v) {
+ v.visitRecord(this);
+ }
+
+ /**
+ * @return deep copy of this record attribute
Review Comment:
Javadoc is missing its first sentence.
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and
+ * interfaces that are authorized to claim membership in the nest hosted by the
+ * current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ this(nameIndex, length, readComponents(input, constantPool), constantPool);
+ }
+
+ private static RecordComponentInfo[] readComponents(final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ final int classCount = input.readUnsignedShort();
+ final RecordComponentInfo[] components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ components[i] = new RecordComponentInfo(input, constantPool);
+ }
+ return components;
+ }
+
+ /**
+ * Construct elements using its components
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param classes Array of Record Component Info elements
+ * @param constantPool Array of constants
+ */
+ public Record(final int nameIndex, final int length, final RecordComponentInfo[] classes,
+ final ConstantPool constantPool) {
+ super(Const.ATTR_RECORD, nameIndex, length, constantPool);
+ this.components = classes != null ? classes : EMPTY_RCI_ARRAY;
+ Args.requireU2(this.components.length, "attributes.length");
+ }
+
+ /**
+ * Called by objects that are traversing the nodes of the tree implicitly
+ * defined by the contents of a Java class. I.e., the hierarchy of methods,
+ * fields, attributes, etc. spawns a tree of objects.
+ *
+ * @param v Visitor object
+ */
+ @Override
+ public void accept(final Visitor v) {
+ v.visitRecord(this);
+ }
+
+ /**
+ * @return deep copy of this record attribute
+ */
+ @Override
+ public Attribute copy(final ConstantPool constantPool) {
+ final Record c = (Record) clone();
+ if (components.length > 0) {
+ c.components = components.clone();
+ }
+ c.setConstantPool(constantPool);
+ return c;
+ }
+
+ /**
+ * Dump RecordComponentInfo attribute to file stream in binary format.
+ *
+ * @param file Output file stream
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public void dump(final DataOutputStream file) throws IOException {
+ super.dump(file);
+ file.writeShort(components.length);
+ for (final RecordComponentInfo component : components) {
+ component.dump(file);
+ }
+ }
+
+ /**
+ * @return array of Record Component Info elements.
Review Comment:
Javadoc is missing its first sentence.
##########
src/main/java/org/apache/bcel/classfile/RecordComponentInfo.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+
+/**
+ * Record component info from a record. Instances from this class maps
+ * every component from a given record.
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se14/preview/specs/records-jvms.html#jvms-4.7.30">
+ * The Java Virtual Machine Specification, Java SE 14 Edition, Records (preview)</a>
+ */
+public class RecordComponentInfo implements Node {
+
+ private final int index;
+ private final int descriptorIndex;
+ private final Attribute[] attributes;
+ private final ConstantPool constantPool;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ public RecordComponentInfo(final DataInput input, ConstantPool constantPool) throws IOException {
+ this.index = input.readUnsignedShort();
+ this.descriptorIndex = input.readUnsignedShort();
+ final int attributesCount = input.readUnsignedShort();
+ this.attributes = new Attribute[attributesCount];
+ for (int j = 0; j < attributesCount; j++) {
+ attributes[j] = Attribute.readAttribute(input, constantPool);
+ }
+ this.constantPool = constantPool;
+ }
+
+ public void dump(DataOutputStream file) throws IOException {
Review Comment:
Missing Javadoc
##########
src/test/java/org/apache/bcel/classfile/RecordTestCase.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.bcel.AbstractTestCase;
+import org.apache.bcel.util.SyntheticRepository;
+import org.apache.bcel.visitors.CountingVisitor;
+import org.junit.jupiter.api.Test;
+
+public class RecordTestCase extends AbstractTestCase {
+
+ /**
+ * An record type, once compiled, should result in a class file that is marked such that we can determine from the
Review Comment:
`An record` -> `A record`
##########
src/main/java/org/apache/bcel/classfile/Record.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.bcel.classfile;
+
+import java.io.DataInput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.util.Args;
+
+/**
+ * This class is derived from <em>Attribute</em> and records the classes and
+ * interfaces that are authorized to claim membership in the nest hosted by the
+ * current class or interface. There may be at most one Record attribute in a
+ * ClassFile structure.
+ *
+ * @see Attribute
+ * @since 6.9.0
+ */
+public final class Record extends Attribute {
+
+ private static final RecordComponentInfo[] EMPTY_RCI_ARRAY = new RecordComponentInfo[] {};
+
+ private RecordComponentInfo[] components;
+
+ /**
+ * Constructs object from input stream.
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param input Input stream
+ * @param constantPool Array of constants
+ * @throws IOException if an I/O error occurs.
+ */
+ Record(final int nameIndex, final int length, final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ this(nameIndex, length, readComponents(input, constantPool), constantPool);
+ }
+
+ private static RecordComponentInfo[] readComponents(final DataInput input, final ConstantPool constantPool)
+ throws IOException {
+ final int classCount = input.readUnsignedShort();
+ final RecordComponentInfo[] components = new RecordComponentInfo[classCount];
+ for (int i = 0; i < classCount; i++) {
+ components[i] = new RecordComponentInfo(input, constantPool);
+ }
+ return components;
+ }
+
+ /**
+ * Construct elements using its components
+ *
+ * @param nameIndex Index in constant pool
+ * @param length Content length in bytes
+ * @param classes Array of Record Component Info elements
+ * @param constantPool Array of constants
+ */
+ public Record(final int nameIndex, final int length, final RecordComponentInfo[] classes,
+ final ConstantPool constantPool) {
+ super(Const.ATTR_RECORD, nameIndex, length, constantPool);
+ this.components = classes != null ? classes : EMPTY_RCI_ARRAY;
+ Args.requireU2(this.components.length, "attributes.length");
+ }
+
+ /**
+ * Called by objects that are traversing the nodes of the tree implicitly
+ * defined by the contents of a Java class. I.e., the hierarchy of methods,
+ * fields, attributes, etc. spawns a tree of objects.
+ *
+ * @param v Visitor object
+ */
+ @Override
+ public void accept(final Visitor v) {
+ v.visitRecord(this);
+ }
+
+ /**
+ * @return deep copy of this record attribute
+ */
+ @Override
+ public Attribute copy(final ConstantPool constantPool) {
+ final Record c = (Record) clone();
+ if (components.length > 0) {
+ c.components = components.clone();
+ }
+ c.setConstantPool(constantPool);
+ return c;
+ }
+
+ /**
+ * Dump RecordComponentInfo attribute to file stream in binary format.
+ *
+ * @param file Output file stream
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public void dump(final DataOutputStream file) throws IOException {
+ super.dump(file);
+ file.writeShort(components.length);
+ for (final RecordComponentInfo component : components) {
+ component.dump(file);
+ }
+ }
+
+ /**
+ * @return array of Record Component Info elements.
+ */
+ public RecordComponentInfo[] getComponents() {
+ return components;
+ }
+
+ /**
+ * @return String representation, i.e., a list of classes.
Review Comment:
Javadoc is missing its first sentence.
##########
src/main/java/org/apache/bcel/classfile/JavaClass.java:
##########
@@ -906,4 +911,27 @@ public String toString() {
}
return buf.toString();
}
+
+ /**
+ * Query method to check if this class was declared as a record
+ *
+ * @return true if a record attribute is present, false otherwise
Review Comment:
Missing `@since` tag.
##########
src/test/java/org/apache/bcel/classfile/RecordTestCase.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.bcel.AbstractTestCase;
+import org.apache.bcel.util.SyntheticRepository;
+import org.apache.bcel.visitors.CountingVisitor;
+import org.junit.jupiter.api.Test;
+
+public class RecordTestCase extends AbstractTestCase {
+
+ /**
+ * An record type, once compiled, should result in a class file that is marked such that we can determine from the
+ * access flags (through BCEL) that it is in fact a record
+ * @throws IOException
+ * @throws ClassFormatException
+ */
+ @Test
+ public void testRecordClassSaysItIs() throws ClassNotFoundException, ClassFormatException, IOException {
+ final JavaClass clazz = new ClassParser("src/test/resources/record/SimpleRecord.class").parse();
+ assertTrue(clazz.isRecord(), "Expected SimpleEnum class to say it was a record - but it didn't !");
+ final JavaClass simpleClazz = getTestJavaClass(PACKAGE_BASE_NAME + ".data.SimpleClass");
+ assertFalse(simpleClazz.isRecord(), "Expected SimpleClass class to say it was not a record - but it didn't !");
+ }
+
+ /**
+ * An simple record with two simple fields, an integer and a String field, should
Review Comment:
`An simple` -> `A simple`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2008589150
@PabloNicolasDiaz
The PR still causes the build to fail.
Also, note that the code coverage is insufficient as reported by Codecov:
```
Attention: Patch coverage is 50.56180% with 44 lines in your changes are missing coverage. Please review.
Project coverage is 65.09%. Comparing base (f32c53c) to head (ce967c6).
Report is 35 commits behind head on master.
Files Patch % Lines
...rc/main/java/org/apache/bcel/classfile/Record.java 53.84% 16 Missing and 2 partials ⚠️
...org/apache/bcel/classfile/RecordComponentInfo.java 55.55% 11 Missing and 1 partial ⚠️
...a/org/apache/bcel/classfile/DescendingVisitor.java 0.00% 8 Missing ⚠️
...main/java/org/apache/bcel/classfile/JavaClass.java 66.66% 2 Missing and 2 partials ⚠️
...c/main/java/org/apache/bcel/classfile/Visitor.java 0.00% 2 Missing ⚠️Additional details and impacted files
@@ Coverage Diff @@
## master #290 +/- ##
============================================
- Coverage 65.10% 65.09% -0.02%
- Complexity 3893 3909 +16
============================================
Files 364 366 +2
Lines 15657 15745 +88
Branches 1956 1967 +11
============================================
+ Hits 10194 10249 +55
- Misses 4549 4577 +28
- Partials 914 919 +5
```
You can also use JaCoCo locally with `mvn clean site -P jacoco` and open the site in `target/site` where you'll find a nice and detailed HTML report; look for JaCoCo in the left-hand side menu.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "markro49 (via GitHub)" <gi...@apache.org>.
markro49 commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545496027
##########
src/main/java/org/apache/bcel/classfile/JavaClass.java:
##########
@@ -906,4 +911,28 @@ public String toString() {
}
return buf.toString();
}
+
+ /**
+ * Tests whether this class was declared as a record
+ *
+ * @return true if a record attribute is present, false otherwise.
+ * @since 6.9.0
+ */
+ public boolean isRecord() {
+ computeIsRecord();
+ return this.isRecord;
+ }
+
+ private void computeIsRecord() {
+ if (computedRecord) {
+ return;
+ }
+ for (final Attribute attribute : this.attributes) {
+ if (attribute instanceof Record) {
+ isRecord = true;
+ break;
+ }
+ }
+ this.computedRecord = false;
Review Comment:
shouldn't this be set to true?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545655543
##########
src/main/java/org/apache/bcel/classfile/Visitor.java:
##########
@@ -237,4 +249,15 @@ default void visitStackMapType(final StackMapType obj) {
void visitSynthetic(Synthetic obj);
void visitUnknown(Unknown obj);
+
+ /**
+ * Visits a {@link RecordComponentInfo} object.
+ *
+ * @param obj record component to visit
Review Comment:
Hello @PabloNicolasDiaz
Please rebase on git master to see this error in the CI build. For some reason, we had doclint disabled.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "markro49 (via GitHub)" <gi...@apache.org>.
markro49 commented on PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#issuecomment-2028495585
completed my review
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "PabloNicolasDiaz (via GitHub)" <gi...@apache.org>.
PabloNicolasDiaz commented on code in PR #290:
URL: https://github.com/apache/commons-bcel/pull/290#discussion_r1545529676
##########
src/main/java/org/apache/bcel/classfile/JavaClass.java:
##########
@@ -906,4 +911,28 @@ public String toString() {
}
return buf.toString();
}
+
+ /**
+ * Tests whether this class was declared as a record
+ *
+ * @return true if a record attribute is present, false otherwise.
+ * @since 6.9.0
+ */
+ public boolean isRecord() {
+ computeIsRecord();
+ return this.isRecord;
+ }
+
+ private void computeIsRecord() {
+ if (computedRecord) {
+ return;
+ }
+ for (final Attribute attribute : this.attributes) {
+ if (attribute instanceof Record) {
+ isRecord = true;
+ break;
+ }
+ }
+ this.computedRecord = false;
Review Comment:
fixed in https://github.com/apache/commons-bcel/pull/290/commits/e3c2f2eff322f998fd97416becb60c68d6a3ba48
##########
src/test/java/org/apache/bcel/classfile/RecordTestCase.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.bcel.AbstractTestCase;
+import org.apache.bcel.util.SyntheticRepository;
+import org.apache.bcel.visitors.CountingVisitor;
+import org.junit.jupiter.api.Test;
+
+public class RecordTestCase extends AbstractTestCase {
+
+ /**
+ * A record type, once compiled, should result in a class file that is
+ * marked such that we can determine from the access flags
+ * (through BCEL) that it is in fact a record.
+ *
+ * @throws IOException
+ * @throws ClassFormatException
+ */
+ @Test
+ public void testRecordClassSaysItIs() throws ClassNotFoundException, ClassFormatException, IOException {
+ final JavaClass clazz = new ClassParser("src/test/resources/record/SimpleRecord.class").parse();
Review Comment:
added in https://github.com/apache/commons-bcel/pull/290/commits/e3c2f2eff322f998fd97416becb60c68d6a3ba48
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for Java 16 Record feature [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #290:
URL: https://github.com/apache/commons-bcel/pull/290
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org