Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7800

Remove code that potentially rethrows checked exceptions from methods that don't declare them ("sneaky throw" hack)

    Details

    • Type: Task
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.7, 7.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      For a long time I considered the "sneaky" throw hack to be a nice way of coding around some of Java's limitations (especially with invoking methods via reflection or method handles), but with time I started to see how it can be potentially dangerous and is nearly always confusing. If you have a Java method and its signature doesn't indicate the possibility of a checked exception you, as a programmer, simply don't expect it to happen. Never. So, for example, you could write:

      try {
       luceneApi();
      } catch (RuntimeException | Error e) {
        // Handle unchecked exceptions here.
      }
      

      and consider the code above to be absolutely bullet-proof in terms of handling exceptions. Unfortunately with sneaky throws anywhere in the "luceneApi" this is no longer the case – you can receive a checked exception that will simply fall through and hit some code frame above.

      So I suggest we remove sneaky throw from the core entirely. It only exists in two places – private methods inside Snowball programs invoked via method handles (these don't even declare checked exceptions so I assume they can't occur) and AttributeFactory – here there is a real possibility somebody could declare an attribute class's constructor that does throw an unchecked exception. In that case I think it is more reasonable to wrap it in a RuntimeException than rethrow it as original.

      Alternatively, we can modify the signature of createAttributeInstance and getStaticImplementation to declare some kind of checked exception (either a wrapper or even a Throwable), but I see little reason for it and it'd change the API.

        Attachments

        1. LUCENE-7800.patch
          11 kB
          Dawid Weiss
        2. LUCENE-7800.patch
          8 kB
          Dawid Weiss
        3. LUCENE-7800.patch
          3 kB
          Dawid Weiss

          Issue Links

            Activity

              People

              • Assignee:
                dweiss Dawid Weiss
                Reporter:
                dweiss Dawid Weiss
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: