Handling for new warning?

[Maurizio Cimadamore]

My (very humble) 0.02$ on this; I can see where Mike is coming from 
_and_ where Jon and Mark are coming from.

The problem is that, when we are inside an _implementation_ class (think 
of a non-exported package in a module), we probably don't want to be too 
formal about the set of unchecked annotations being thrown.

Conversely, when we are in a public-facing API, it is very likely that 
unchecked exceptions carry value (IllegalArgumentExecption, 
IllegalStateException to name a few) and should therefore be documented. 
To be more concrete, on my recent work on the memory access API, while 
developing the API and writing the docs I wish I had an automatic way to 
detect all unchecked exception I missed in the 'throws' clause, as they 
were likely (a) some documentation missing somewhere or (b) a missing 
test for that particular failure mode.

The problem is how to reconcile these two aspects - you want a warnings 
that's useful, but not one that's excessively punitive so that 
apparently good code gets flagged (I am reminded of when I introduced 
the warning for raw types and later on relaxed that not to be triggered 
on stuff like 'instanceof List', which is pretty widespread - and for 
which adding an extra '<?>' doesn't really add a lot of value).

And, in this case, it can be tricky to come up with a way to distinguish 
between API and internals. Perhaps we could use the module system to our 
advantage, and only enable the Lint check for public methods in public 
classes/interfaces in exported packages. I'm pretty sure something like 
this should reduce the 'noise' of such a check quite significantly (and 
the remaining places where the warning is still issued are likely to be 
informative for us).

Thoughts?

Maurizio

On 25/09/2019 01:59, Jonathan Gibbons wrote:
>
> On 9/24/19 5:46 PM, Mike Duigou wrote:
>>
>>
>>
>>
>> On 2019-09-24 16:25, mark.reinhold at oracle.com wrote:
>>> 2019/9/24 15:56:37 -0700, openjdk at duigou.org:
>>>> The proposed change is to add a lint level warning, ie.
>>>> -XLint:runtime-undeclared , suggestion to add the unrequired but 
>>>> missing
>>>> throws declarations for RuntimeException et al. The advantage to
>>>> providing the complete throws including RuntimeExceptions is to make
>>>> them more visible and encourage handling of them. We have one good
>>>> example of a library which does declare the RuntimeException that are
>>>> thrown, the JDK.
>>>
>>> I’m confused.  I thought you said earlier that there are thousands of
>>> missing cases in the JDK.
>>
>> JDK seems to be in pretty good shape though -Werror stops compilation 
>> well before completion. The observed problems with large numbers of 
>> missing declarations have been in javac where there are hundreds 
>> (perhaps many more) missing throws declarations primarily, so far, 
>> for IllegalArgumentException and CompletionFailure.
>
> I don't think it would be acceptable to change javac so that 
> CompletionFailure is specified all over the place.   Story: a long 
> time ago, back when I joined the javac team, I tried doing something 
> similar to what you're proposing now, except that I tried to do it by 
> changing CompletionFailure to a checked exception. As you might guess, 
> the size of the change became unreasonable and I abandoned the 
> experiment.
>
> I'd also be concerned about IllegalArgumentException and friends like 
> NullPointerException becoming too pervasive across the codebase.
>
> -- Jon
>
>
>>
>> There's no easy way to know the total number of potential throws 
>> "fixes" needed because adding missing declarations has a tendency to 
>> produce new warnings in the methods which call the fixed method. I 
>> added a couple hundred across a dozen or so files but didn't feel I 
>> was any near complete based on the number of files I hadn't yet 
>> covered in the javac source.
>>
>>>
>>>> ...
>>>>
>>>> The check will apply to all instances of RuntimeException and all
>>>> subclasses.
>>>
>>> I agree with Jon: This proposal is likely to have broad impact and thus
>>> needs wider discussion.  May I suggest the jdk-dev list?
>>
>>
>> For JDK this would seem to be a separate problem--the missing 
>> "throws" could in some cases mean that the method signature and 
>> javadoc are an incomplete specification of the API and a 
>> potential/probable JCK issue.
>>
>> For the moment I am going to working on completing the minimal patch 
>> which provides the new lint warning and allows compilation by 
>> disabling the warning.
>>
>>>
>>> - Mark
>>
>>
>> Cheers,
>>
>> Mike