INRIA / INRIA/spoon

Need to expand AstCheckerTest to support more patterns for delegation/surcharging

Open
#4,100 3 comments 0 reactions 0 assignees View on GitHub
test
Dominant language
Java
Stars
2k
Forks
392
Avg merge
11h 24m
Merged PRs (30d)
36

Description

As evidenced by #4038, the AstCheckerTest is too conservative. I'll use #4038 as grounds for the following patterns we probably want to add support for.

First of all, the PushStackInIntercessionChecker needs a refactoring, it's pretty hard to understand what's expected e.g. for a method to count as delegated.

[`CtInterfaceImpl.removeNestedType`](https://github.com/INRIA/spoon/pull/4038/files#diff-aebac4f9ac007ac937bc14a49470399a1ce6edf5ed772bfbd0c187cc58d2f175R85-R105) should be detected as a delegated method, but it isn't as the method invocation isn't in any location expected by `PushStackInIntercessionChecker.isDelegate`, so that should be expanded. I'm thinking we might want to cover this pattern:

```java
// store return value of delegated-to method in a variable
@Override
public SomeType method() {
SomeType val = super.method();
[...]
if (something) {
// all returns must use val
return val;
}

return val;
}
```

Now, the pattern used in this PR is more like this:

```java
// use delegated method in early return if condition
@Override
public boolean method() {
if (!super.method()) {
return false;
}
[...]

return true;
}
```

But supporting that pattern is a little bit more difficult as it's harder to verify that the return value of the method is actually used. It is used implicitly here, and it's possible to verify, it's just some amount of work to do so, whereas verifying that all return values reference a particular local variable is trivial.

[`CtTypeImpl.removeNestedType`](https://github.com/INRIA/spoon/pull/4038/files#diff-c27f4bbf924c0556ebbba08b2f902851adc380122ba9fe15c1fcd8be50ce08c0R265-R281) has essentially the same problem, but for the surcharge instead of the delegate check. I think supporting the pattern I suggested above should cover this as well.

Obviously the pattern doesn't make for ideal code, but it's easy to verify and applicable to all kinds of return values.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.