Reducing "exception is never thrown in body of corresponding try statement" Error to a Warning

[Mike Duigou]

While the proposed change doesn't directly alter the inability to refine 
(narrow) the exception specification in method/constructor signatures 
because use of narrowing/elimination would break overrides you are 
correct in pointing it out as a a potential issue. I also agree that a 
sub-class throwing a wider set of checked exceptions than the parent 
will always need to be a compilation error.

I do believe that there are cases such as private methods/constructors, 
final classes, and internal APIs where the ability to narrow/remove 
exception specifications will be useful. Addressing the original intent 
of the error, I believe that the "All subtypes of exception that the try 
block can throw have already been caught." warning introduced in Java SE 
7 has proved to be sufficient warning for the similar problem. This 
proposed adjustment would bring the severity of "never-thrown" and 
"already-handled" cases into alignment which seems appropriate.

For -Werror users the effect of reducing the "never-thrown" error to a 
warning would be to allow suppression of the warning which might be 
useful in the case of a need to link against a library which adjusted 
method exception specifications between versions. As an error the 
"never-thrown" can't be surpressed and I can report that when a certain 
Apache library removed exceptions from several methods it was huge 
annoyance to support two versions of the library simultaneously (Maven 
hacks to "generate" (select) appropriate source based on a profile of 
which library version was being used. Yuck!)

Cheers,

Mike

On 2019-10-20 15:55, Brian Goetz wrote:
> I will admit that this strictness is sometimes annoying.
> 
> Your goal here, which is a good one, is to allow narrowing the set of
> checked exceptions to be a source-compatible change.  I seriously
> doubt much consideration was given in 1995 for the
> source-compatibility of _removing_ exceptions from a descriptor, even
> though this should be a compatible change (as it narrows the range of
> accepted effects.)  You correctly argue that this would be a
> compatible change for clients that hadn’t gotten the memo about the
> narrowed exception set.
> 
> Unfortunately, there’s another group we have to worry about —
> subclasses.  Because, if you have a hierarchy:
> 
>     class Foo {
>         void m() throws Exception { };
>     }
> 
>     class Bar extends Foo {
>         @Override
>         void m() throws Exception { };
>     }
> 
> where Foo and Bar are in separate maintenance domains, then reducing
> the set of exceptions in Foo:
> 
>     class Foo {
>         void m() throws IOException { };
>     }
> 
> now renders Bar uncompilable, because it throws a wider set of
> exceptions than the method it overrides (violating LSP) and therefore
> reducing the set of exceptions for overridable methods is not a
> source-compatible change for this group.
> 
> (General observation: It’s really easy to forget the “subclasses” part
> of compatibility — it’s easy to focus only on the clients.  This has
> bitten me more than once.)
> 
> Given that this change still would not allows us to say reducing the
> set of thrown exceptions is a source-compatible change, I’d think that
> the benefit is reduced somewhat?
> 
> 
>> On Oct 20, 2019, at 6:36 PM, Mike Duigou <openjdk at duigou.org> wrote:
>> 
>> Hello all;
>> 
>> As part of my Oracle CodeOne talk "Exceptions 2020" I made two 
>> proposals for improving Java exceptions. One of the proposals was to 
>> remove the compile time error produced when and exception is caught 
>> that is not thrown by the enclosed statements. Specifically I am 
>> proposing the removal of the following section from JLS 11.2.3
>> 
>>    "It is a compile-time error if a catch clause can catch checked 
>> exception class E1
>>     and it is not the case that the try block corresponding to the 
>> catch clause can
>>     throw a checked exception class that is a subclass or superclass 
>> of E1, unless E1
>>     is Exception or a superclass of Exception."
>> 
>> The advantage of removal of this rule is that removal allows API 
>> authors to better document the actual exceptions thrown by 
>> constructors and methods. Vestigial or inaccurate throws 
>> specifications can be eliminated without "breaking" calling code. This 
>> makes it easier to evolve the API by narrowing (including to empty), 
>> the exception specification of method without impacting existing code 
>> which uses the API.
>> 
>> To allow application authors to clean up dead code an equivalent 
>> warning would still be provided and probably should be encouraged by 
>> the JLS specification. The warning would alert them that the catch 
>> clause will not be executed.
>> 
>> As an API author I have been frustrated many times by being unable to 
>> reduce or remove a method's exception specification without causing 
>> compatibility issues for my users and I have also been frustrated when 
>> libraries did choose to alter their method exception specifications 
>> and "broke" my code which caught the no longer thrown exceptions. Does 
>> this seem like a feature/change worth pursuing?
>> 
>> Cheers,
>> 
>> Mike
>> 
>> The changes to javac needed to accomplish this change are quite 
>> simple:
>> 
>> diff --git 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
>> index 0c1fead9a4..f7793d0605 100644
>> --- 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
>> +++ 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
>> @@ -1271,7 +1277,7 @@ public class Flow {
>>             } else if (!chk.isUnchecked(pos, exc) &&
>>                     !isExceptionOrThrowable(exc) &&
>>                     !chk.intersects(exc, thrownInTry)) {
>> -                log.error(pos, Errors.ExceptNeverThrownInTry(exc));
>> +                log.warning(pos, 
>> Warnings.ExceptNeverThrownInTry(exc));
>>             } else {
>>                 List<Type> catchableThrownTypes = 
>> chk.intersect(List.of(exc), thrownInTry);
>>                 // 'catchableThrownTypes' cannnot possibly be empty - 
>> if 'exc' was an
>> diff --git 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
>> index 1f29df0704..e29b5583e4 100644
>> --- 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
>> +++ 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
>> @@ -533,10 +533,6 @@ compiler.err.error.reading.file=\
>> compiler.err.except.already.caught=\
>>     exception {0} has already been caught
>> 
>> -# 0: type
>> -compiler.err.except.never.thrown.in.try=\
>> -    exception {0} is never thrown in body of corresponding try 
>> statement
>> -
>> # 0: symbol
>> compiler.err.final.parameter.may.not.be.assigned=\
>>     final parameter {0} may not be assigned
>> @@ -2019,6 +2015,14 @@ 
>> compiler.warn.unchecked.generic.array.creation=\
>> compiler.warn.unchecked.varargs.non.reifiable.type=\
>>     Possible heap pollution from parameterized vararg type {0}
>> 
>> +# 0: type
>> +compiler.warn.except.never.thrown.in.try=\
>> +    exception {0} is never thrown in body of corresponding try 
>> statement
>> +
>> # 0: symbol
>> compiler.warn.varargs.unsafe.use.varargs.param=\
>>     Varargs method could cause heap pollution from non-reifiable 
>> varargs parameter {0}