Need to expand AstCheckerTest to support more patterns for delegation/surcharging
- 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
Assessment
This issue has not been assessed yet.