OpenClover 4.5.0 fails to instrument code with "unexpected token: Optional"

[matic277]

We are trying to migrate from 4.4.1 to 4.5.0, which is supposed to be able to process some new java features, like records for example.

We are using gradle in our project, with clovers plugin:
dependencies { classpath "org.openclover:clover:4.4.1" // 4.5.0 }
Which works, but bumping to 4.5.0, clover stops working.
We have the standard gradle tasks, as is suggested at https://openclover.org/doc/manual/4.2.0/gradle--gradle-clover-plugin.html.

Running the clover instrumentation, we get an error:

ERROR: full/path/to/class/SomeClass.java:52:5:unexpected token: Optional
ERROR: Instrumentation error
full/path/to/class/SomeClass.java:52:5:unexpected token: Optional

Instrumentation error

The source code does indeed use an Optional in that line.

More info:
When running instrumentation, in clover 4.4.1, the source level was set to 1.9:
build 10-Oct-2023 09:11:12 Processing files at 1.9 source level.

However, using clover 4.5.0, we now get:

build	10-Oct-2023 09:15:39	> Task :cloverInstr
build	10-Oct-2023 09:15:39	OpenClover Version 4.5.0, built on 2023-10-04
build	10-Oct-2023 09:15:39	Creating new database at 'path\to\db\clover.db'.
build	10-Oct-2023 09:15:39	Processing files at JAVA_7 source level.

This seems strange to me, so I tried tricking clover to set the value back to 1.9, or at least JAVA_11 (max supported in 4.5.0).

task cloverInst(...) {
  ...
  // also tried adding to argsList: [..."--source", "9"]

  // right before calling CloverInstr.mainImpl !
  com.atlassian.clover.cfg.instr.java.SourceLevel.JAVA_9.versionStrings.add("17")

  // or: com.atlassian.clover.cfg.instr.java.SourceLevel.JAVA_11.versionStrings.add("17")
  
  com.atlassian.clover.CloverInstr.mainImpl(args)
}

which then results in log when running clover:
build 10-Oct-2023 09:18:41 Processing files at JAVA_11 source level.
or
build 10-Oct-2023 09:18:41 Processing files at JAVA_9 source level.

Which is fine, however, the same error pops up; unexpected token: Optional.

We are using Java 17 Temurin 17.0.4.
And in clovers sourcecode, the class JavaEnvUtils does in fact correctly set to
javaVersion = JAVA_17
in it's static block.

[marek-parfianowicz]

Thank you for reporting this bug. I'll try to fix it as quickly as possible.

It would be very helpful if you could share the source file, which OpenClover failed to instrument. Ideally truncated to a minimum size (minimum reproducible test case).

Also, is there any chance that you use records in this file (bug #206) or case expressions (feature request not yet implemented)?

[matic277]

We have been on Java 17 for a while, but stayed on features of 1.8 (except for the 'var' keyword) due to clover 4.4.1 not being compatible with newer features.

So, no, when migrating from 4.4.1 to 4.5.0, no source code was modified, and no records are being used anywhere in the project.

I will try to provide the source file tomorrow, or try to create an example that reproduces the problem, and I'll post it here.

[matic277]

This:

public Data getData(LocalDate date, boolean exceptionOnNone)
{
  var x = 1;
  if (x == 1)
  {
    System.out.println("var used");
  }
  Optional<Data> data = getOnDate(date);
  return exceptionOnNone?
          data.orElseThrow(() -> new TaskException("...") :
          data.orElse(null);
}

and this:

public Data getData(LocalDate date, boolean exceptionOnNone)
{
  Optional<Data> data = getOnDate(date);
  return exceptionOnNone?
          data.orElseThrow(() -> new TaskException("...") :
          data.orElse(null);
}

Both produce:

unexpected token: Optional

But this

public Data getData(LocalDate date, boolean exceptionOnNone)
{
  var data = getOnDate(date);
  return exceptionOnNone?
          data.orElseThrow(() -> new TaskException("...") :
          data.orElse(null);
}

produces

unexpected token: var

marking the var (in the last code example) with final, then produces

unexpected token: final

To make matters worse, we are using final/var/Optional all over the code base,
and in fact, some classes using those get instrumented correctly before reaching
this specific class.

Here is a snippet of code that in can find in build/sources-instr/.../SomeClass:

...
private void initForManagerTag(Element managerTag)
  {__CLR4_5_0g3g3lnmree7d.R.inc(607);
    __CLR4_5_0g3g3lnmree7d.R.inc(608);final var mandatory = EnumSet.complementOf(ConfigProperty.DB_TAG_MANDATORY_PROPERTIES);
    __CLR4_5_0g3g3lnmree7d.R.inc(609);mandatory.remove(ConfigProperty.IS_PRIMARY);
    __CLR4_5_0g3g3lnmree7d.R.inc(610);init(managerTag, mandatory);
    ...
}
...

Here, final and var get instrumented just fine.

Next, I tried to comment that code out (the method getDate(date, boolean), that produces unexpected token:var/Optional)
and then the same error happens in some other class: unexpected token: Optional.

What is worse, is that I can also find examples of correctly instrumented optionals:

    ... // inside some method
    __CLR4_5_06gf6gflnmrkpmc.R.inc(8392);Optional<PolicySpecialtyData> commissioningSpecialty = policy.getPolicySpecialtyDataList()
        .getSpecialty(PolicySpecialtyOffering.COMMISSIONING_SHARE, m_processDate);
    __CLR4_5_06gf6gflnmrkpmc.R.inc(8393);if ((((!commissioningSpecialty.isPresent())&&(__CLR4_5_06gf6gflnmrkpmc.R.iget(8394)!=0|true))||(__CLR4_5_06gf6gflnmrkpmc.R.iget(8395)==0&false)))
    {{
    ...

I tried to reproduce in a new project with gradle+clover integrated, but everything instrumented without an error :(.
In that project, I used Java 17, and used finalvar/optional/record. Strangely (or not), I also get:
Processing files at JAVA_7 source level.

The original source file that is producing the original error, is unfortunately too bussiness heavy and has too many imports/dependendies for me to hand it over.

Strange that some files get instrumented and some do not.

Next.... I tried to comment out problematic lines that produce errors. I had to comment out 2 Optionals in 2 separate classes, which then lead me to this:

public String someMethod(int i1, String s1, boolean b1)
{
/* line: 50 */)  final var billingAddress = m_corporateCustomer.getBillingAddressOrThrow(CommonSettings.getCurrentDate());
/* line: 51 */)  final var mailingAddress = m_corporateCustomer.getMailingAddressOrThrow(CommonSettings.getCurrentDate());
/* line: 52 */)  final var record = new Record(c_delimiter)
/* line: 53 */)      .add("01", 1)
/* line: .. */)      /*about 15 chained .add() calls*/
/* line: 66 */)      .add(...);
/* line: 67 */)  ...

produced
.../SomeClassXYZ.java:52:5:unexpected token: final

produced on line 52, but not on 50 and 51?

Please let me know if you need anything more.

[matic277]

I have managed to construct an example that produces that last error i described in my last post:
this project (without gradle wrapper, gradle version 7.4.1), when running gradle task "build", produces

> Task :cloverInstr
OpenClover Version 4.5.0, built on 2023-10-04
Creating new database at 'C:\Users\MaticA\IdeaProjects\CloverOptionalBug\build\clover\clover.db'.
Processing files at JAVA_7 source level.
ERROR: C:\Users\MaticA\IdeaProjects\CloverOptionalBug\src\main\java\org\example\Printout.java:20:5:unexpected token: final
ERROR: Instrumentation error
C:\Users\MaticA\IdeaProjects\CloverOptionalBug\src\main\java\org\example\Printout.java:20:5:unexpected token: final

Instrumentation error

Note that Customer.java and Main.java do in fact get instrumented correctly!

The repository is available at https://github.com/matic277/CloverOptionalBug

I just realised i named it CloverOptionalBug, when in reality this example throws "final" keyword error, eh.

[marek-parfianowicz]

Thank you! I will investigate this.

[marek-parfianowicz]

Update: I'm working on upgrading the test infrastructure, to build entire code and tests with Java 8/11/17 and Groovy 2/3/4. This way I will be able to ensure that code instrumentation works as expected. See issue #209 or PR#210 for details. It's a rather big task but I'm half-way through.

Once done, I'll switch to your bug report. I have a strong suspicion that it's closely correlated with issue #206, i.e. that the new ANTLR grammar written for handling records is too strict, leading to parse errors in "random places", while in fact the root cause is use of the 'record' keyword as a name of a variable or a method.

[matic277]

Interesting.

In our project, the keyword "record" appears too many times for it to be removed to test this hypothesis. But it does look like you are right.

Good luck :)

[marek-parfianowicz]

Fixed by PR #208