RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

[Maurizio Cimadamore]

On Thu, 12 Jan 2023 19:12:27 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> Uhm. Turns out I probably did not understand the filter correctly, and now I'm more dubious about what it actually does. Say you have this hierarchy:
>> 
>> 
>> interface A { }
>> class B {
>>      B() {
>>           A a = (A)this;
>>           ...
>>       }
>>  }
>>  class C extends B implements A { }
>>  ```
>> 
>> Pathological case, I know. But the filtering will end up dropping the expression Ref on the floor, right? (because B and A are unrelated).
>
>> But the filtering will end up dropping the expression Ref on the floor, right? (because B and A are unrelated).
> 
> Ah, I see what you mean.
> 
> Here's a more complete example:
> 
> public class CastLeak {
> 
>     public CastLeak() {
>         ((CastLeak)(Runnable)this).mightLeak();
>     }
> 
>     public void mightLeak() {
>     }
> }
> 
> That would be a leak for any subclass that implements `Runnable`. Yet no warning is generated.
> 
> So the filtering by expression type is going to potentially create false negatives. But it also eliminates a bunch of false positives. And the false negatives are probably all somewhat pathological like the example above.
> 
> So I still think it's worth doing.

Ok - I thought false negative was the thing to absolutely avoid - and that was the no. 1 concern. I think a possible approach to keep both the filtering and the code more or less similar to what you have, is to save the type of the expression in the ExprRef. Then, I believe that, at the end of scan() you can just get whatever type is there, and check that it's the correct one, if not drop it.

-------------

PR: https://git.openjdk.org/jdk/pull/11874