[Bug] Unable to parse TryWithResourcesStatement when the resource list contains a VariableAccess

[henryhchchc]

Describe the bug
In Java language specification, the resource list of a try with resource expression can be either a variable declaration or a variable access (JLS15).

TryWithResourcesStatement:
    try ResourceSpecification Block [Catches] [Finally]
ResourceSpecification:
    ( ResourceList [;] )
ResourceList:
    Resource {; Resource}
Resource:
    {VariableModifier} LocalVariableType Identifier = Expression 
    VariableAccess
VariableAccess:
    ExpressionName 
    FieldAccess

However, the VariableAccess is missing if parsed with spoon.

To Reproduce

Input:

/*Code being processed by spoon and leading to the bug.*/

public class Test {
    public void test() {
       AutoClosable resource = null;
       try (resource) {
       
       }
    }
}

Processing with Spoon:

/*Code calling spoon on the input and triggering the bug.*/
// N/A

Output:

/*Output code or stacktrace if unexpected Exception is raised*/
// N/A

The parsing result for the code snippet above will be

public class Test {
    public void test() {
        AutoClosable resource = null;
        try  {
        }
    }
}

Operating system, JDK and Spoon version used

• OS: macOS 12.2
• JDK: OpenJDK 17.0.1
• Spoon version: 10.0.0

[SirYwell]

Seems like this was missed when adding Java 9 support. I think the best solution here would be to introduce a CtResource interface that can be used in CtTryWithResource instead of CtLocalVariable. Then, both CtLocalVariable and CtVariableRead can extend CtResource. That way, we're more aligned with the JLS.

It would be possible but a bit ugly to keep CtTryWithResource API-compatible, another idea would be to add a CtTryWithResource*s* interface (as that's what it is called in the JLS) and deprecate the old one... this might not break API, but it would change behavior of existing software that relies on CtTryWithResource.

[henryhchchc]

Seems like this was missed when adding Java 9 support. I think the best solution here would be to introduce a CtResource interface that can be used in CtTryWithResource instead of CtLocalVariable. Then, both CtLocalVariable and CtVariableRead can extend CtResource. That way, we're more aligned with the JLS.

I totally agree with this solution. When I tried to add a class like CtResource, some tests in CtGenerationTest failed because of the lack for the meta models of CtResource. I could not fix the failure since I am not familiar with the concepts like CtRole.

As a workaround, I replaced the type of the resources field in CtTryWithResource with List<CtCodeElement>, which is the closest common ancestor of CtLocalVariable and CtVariableRead. The commit is available at here. I can open a pull request if maintainers are fine with that.

[slarse]

I think that something being broken is a valid reason to break the API. The model is incorrect and it needs to be amended.

I personally would favor breaking the API over doing some strange backwards compatible workaround, because the latter is likely to just stick around forever. And then the reason for it will be forgotten, and it will just be a weird quirk of Spoon.

[SirYwell]

When I tried to add a class like CtResource, some tests in CtGenerationTest failed because of the lack for the meta models of CtResource. I could not fix the failure since I am not familiar with the concepts like CtRole.

Yes, there are a few pitfalls when modifying the model. If a test in CtGenerationTest fails, it means that one of the automatically generated files is outdated. You can simply replace the file, the test code contains comments you can run to copy it over (e.g.

spoon/src/test/java/spoon/processing/CtGenerationTest.java

Line 87 in 918568c

// cp ./target/generated/spoon/support/visitor/replace/ReplacementVisitor.java ./src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java

)

You'll likely also need to add an entry (I think something like "spoon.support.reflect.code.CtResourceImpl") to this list

spoon/src/test/java/spoon/generating/CloneVisitorGenerator.java

Line 206 in e123a8e

private final List<String> excludesAST = Arrays.asList(//

and also add the interface to the Metamodel here

spoon/src/main/java/spoon/metamodel/Metamodel.java

Line 56 in c65b2ea

public static Set<CtType<?>> getAllMetamodelInterfaces() {

and you'll also need to modify some methods in CtInheritanceScanner, but the failing tests will tell you then which ones need to be modified how.

[henryhchchc]

@SirYwell Thanks for the advice. I open a pull request (#4371) with my fix.