Support for Java 16 Record feature

[PabloNicolasDiaz]

This PR adds support for java 16 record feature (https://openjdk.org/jeps/395). Users now can use JavaClass.isRecord() to detect a record class.

Update : Fixed PR description. The Record feature was introduced as a preview feature in Java 14 (https://openjdk.org/jeps/359) and Java 15 (https://openjdk.org/jeps/384), and its final release was in Java 16 (https://openjdk.org/jeps/395)

[garydgregory]

@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!

[garydgregory]

@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.

[PabloNicolasDiaz]

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).

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 88.88889% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 65.79%. Comparing base (f32c53c) to head (bc9ad6f). Report is 107 commits behind head on master.

Files	Patch %	Lines
...he/bcel/verifier/statics/StringRepresentation.java	0.00%	4 Missing :warning:
...main/java/org/apache/bcel/classfile/JavaClass.java	80.00%	1 Missing and 1 partial :warning:
...rc/main/java/org/apache/bcel/classfile/Record.java	93.75%	0 Missing and 2 partials :warning:
...c/main/java/org/apache/bcel/classfile/Visitor.java	0.00%	2 Missing :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #290      +/-   ##
============================================
+ Coverage     65.10%   65.79%   +0.68%     
- Complexity     3893     3961      +68     
============================================
  Files           364      366       +2     
  Lines         15657    15750      +93     
  Branches       1956     1961       +5     
============================================
+ Hits          10194    10363     +169     
+ Misses         4549     4463      -86     
- Partials        914      924      +10     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[garydgregory]

@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.

[garydgregory]

@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.

[garydgregory]

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.

[garydgregory]

@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.

[PabloNicolasDiaz]

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!

[garydgregory]

@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 :-(

[garydgregory]

Hello @PabloNicolasDiaz Thank you for your update. Some new methods are still 0% tested, for example the new accept methods: and I also now see why the build did not pick up on some of the trailing whitespace problems git reports on the command line: The build did not run Checkstyle on tests, in now does in git master, so please rebase you PR and fix remaining issues :-)

[garydgregory]

@markro49 Would you please review this PR?

[markro49]

completed my review

[garydgregory]

@PabloNicolasDiaz The API RecordComponentInfo#getConstantPool() is neither used nor tested. Also, please rebase on git master to pickup the latest, TY!

[PabloNicolasDiaz]

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