openclover / clover

This repository contains source code of OpenClover Core as well as its integrations: Clover-for-Ant, Clover-for-Eclipse and Clover-for-IDEA plugins. Sources are licensed under Apache 2.0 license.
Other
57 stars 14 forks source link

Instrumentation of the instanceof pattern matching with the 'final' keyword fails #237

Closed mebigfatguy closed 5 months ago

mebigfatguy commented 5 months ago

Using the new 4.5.2 version of open-clover, using jdk17, i get errors hitting

    private Object loadPickerValues(Object value, FilterOperator equality, GridEOType type, FilterAvailabilityModel availabilityModel, Session session) {
        if (value == null) {
            return null;
        }

        if (equality != null && FilterOperator.IN_TYPES.contains(equality) && value instanceof final List ids) {
                .....
                .....

i get the error

unexpected token: equality

but i'm assuming it's the instanceof pattern matching at the end of the line

marek-parfianowicz commented 5 months ago

Hi! Thank you for reporting this problem so quickly. I looked at OpenClover's Java grammar file and I think it fails on the 'final' keyword before the type definition. Could you please rewrite the expression to:

...&& value instanceof List ids

and tell me if it worked?

marek-parfianowicz commented 5 months ago

PR: #238

marek-parfianowicz commented 5 months ago

The problem is fixed now. It will wait until 4.6.0 release, unfortunately. ETA end of March. As a workaround, you can remove the 'final' keyword.

mebigfatguy commented 5 months ago

yes it was the final attribute. good call. i can live with that. :)

mebigfatguy commented 5 months ago

found another one. Assuming this is caused by the wildcard?

if (binding instanceof FilterableBinding<?> filterBinding) {

marek-parfianowicz commented 5 months ago

From java.g:

        |
            (INSTANCEOF FINAL type=typeSpec IDENT) =>
            INSTANCEOF FINAL type=typeSpec IDENT
        |
            (INSTANCEOF type=typeSpec IDENT) =>
            INSTANCEOF type=typeSpec IDENT
        |
            INSTANCEOF type=typeSpec
        )
    ;

where typeSpec is:

(
            spec = classTypeSpec
        |
            spec = builtInTypeSpec
        )

where classTypeSpec has:

        typeSpec = classOrInterfaceType
        arrayOpt = arraySpecOpt

and finally classOrInterfaceType has:

        (ann=annotation)*
        IDENT
        (typeArguments)?
marek-parfianowicz commented 5 months ago

Deciphering this :), the pattern matching should allow generic type arguments. Do you have instrumentation error, like previously (e.g. "unexpected token" or similar) or rather a compilation error?

I suspect the latter one. Possibly related with the InstanceOfState detector I just enhanced to handle the 'final' keyword.

I believe the problem is that OpenClover added branch instrumentation to this expression, which caused that the 'filterBinding' from your example lost it's scope (not visible inside { } block).

And yes, removing the <?> is a workaround.

marek-parfianowicz commented 5 months ago

PR #240

marek-parfianowicz commented 5 months ago

Fixed again.