Fix the declared Exceptions of Expression#evaluate() to match those of DoubleValues#doubleValue() (apache#12878)

Author: uschindler

lucene/CHANGES.txt

@@ -121,6 +121,9 @@ Bug Fixes
 
 * GITHUB#12220: Hunspell: disallow hidden title-case entries from compound middle/end
 
+* GITHUB#12878: Fix the declared Exceptions of Expression#evaluate() to match those
+  of DoubleValues#doubleValue(). (Uwe Schindler)
+
 Other
 ---------------------
 

lucene/MIGRATE.md

@@ -122,6 +122,13 @@ The new implementation of the Javascript expressions compiler no longer supports
 Due to the use of `MethodHandle`, classloader isolation is no longer needed, because JS code can only call
 MHs that were resolved by the application before using the expressions module.
 
+### `Expression#evaluate()` declares to throw IOException (GITHUB#12878)
+
+The expressions module has changed the `Expression#evaluate()` method signature:
+It now declares that it may throw `IOException`. This was an oversight because
+compiled expressions call `DoubleValues#doubleValue` behind the scenes, which
+may throw `IOException` on index problems, bubbling up unexpectedly to the caller.
+
 ## Migration from Lucene 9.0 to Lucene 9.1
 
 ### Test framework package migration and module (LUCENE-10301)

lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java

@@ -17,7 +17,6 @@
 package org.apache.lucene.benchmark.jmh;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
@@ -83,12 +82,8 @@ private static double ident(double v) {
   private static final Expression NATIVE_IDENTITY_EXPRESSION =
       new Expression(NATIVE_IDENTITY_NAME, new String[] {"x"}) {
         @Override
-        public double evaluate(DoubleValues[] functionValues) {
-          try {
-            return functionValues[0].doubleValue();
-          } catch (IOException e) {
-            throw new UncheckedIOException(e);
-          }
+        public double evaluate(DoubleValues[] functionValues) throws IOException {
+          return functionValues[0].doubleValue();
         }
       };
 

lucene/expressions/src/java/org/apache/lucene/expressions/Expression.java

@@ -16,6 +16,7 @@
  */
 package org.apache.lucene.expressions;
 
+import java.io.IOException;
 import org.apache.lucene.expressions.js.JavascriptCompiler;
 import org.apache.lucene.search.DoubleValues;
 import org.apache.lucene.search.DoubleValuesSource;
@@ -89,7 +90,7 @@ protected Expression(String sourceText, String[] variables) {
    * @param functionValues {@link DoubleValues} for each element of {@link #variables}.
    * @return The computed value of the expression for the given document.
    */
-  public abstract double evaluate(DoubleValues[] functionValues);
+  public abstract double evaluate(DoubleValues[] functionValues) throws IOException;
 
   /**
    * Get a DoubleValuesSource which can compute the value of this expression in the context of the

lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java

@@ -120,6 +120,7 @@ public final class JavascriptCompiler {
       DOUBLE_VAL_METHOD = getAsmMethod(double.class, "doubleValue"),
       PATCH_STACK_METHOD =
           getAsmMethod(Throwable.class, "patchStackTrace", Throwable.class, Expression.class);
+  private static final Type[] EVALUATE_EXCEPTIONS = new Type[] {Type.getType(IOException.class)};
   private static final Handle DYNAMIC_CONSTANT_BOOTSTRAP_HANDLE =
       new Handle(
           Opcodes.H_INVOKESTATIC,
@@ -211,16 +212,6 @@ static Expression compile(String sourceText, Map<String, MethodHandle> functions
     return new JavascriptCompiler(sourceText, functions, picky).compileExpression();
   }
 
-  /**
-   * This method is unused, it is just here to make sure that the function signatures don't change.
-   * If this method fails to compile, you also have to change the byte code generator to correctly
-   * use the FunctionValues class.
-   */
-  @SuppressWarnings({"unused"})
-  private static void unusedTestCompile(DoubleValues f) throws IOException {
-    f.doubleValue();
-  }
-
   /**
    * Constructs a compiler for expressions with specific set of functions
    *
@@ -355,7 +346,8 @@ private void generateClass(
     constructor.endMethod();
 
     final GeneratorAdapter gen =
-        new GeneratorAdapter(Opcodes.ACC_PUBLIC, EVALUATE_METHOD, null, null, classWriter);
+        new GeneratorAdapter(
+            Opcodes.ACC_PUBLIC, EVALUATE_METHOD, null, EVALUATE_EXCEPTIONS, classWriter);
 
     // add a try/catch block to rewrite stack trace of any Throwable
     final Label beginTry = gen.newLabel(), endTry = gen.newLabel(), catchHandler = gen.newLabel();

lucene/expressions/src/test/org/apache/lucene/expressions/js/TestAPISanity.java

@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.expressions.js;
+
+import java.io.IOException;
+import org.apache.lucene.expressions.Expression;
+import org.apache.lucene.search.DoubleValues;
+
+/** Some sanity checks with API as those are not detected by {@link JavascriptCompiler} */
+public class TestAPISanity extends CompilerTestCase {
+
+  // if this class does not compile, the Exception types of evaluate and DoubleValues do not match.
+  @SuppressWarnings("unused")
+  private static class SanityExpression extends Expression {
+
+    public SanityExpression(String sourceText, String[] variables) {
+      super(sourceText, variables);
+    }
+
+    @Override
+    public double evaluate(DoubleValues[] functionValues) throws IOException {
+      return functionValues[0].doubleValue();
+    }
+  }
+
+  public void testBytecodeExceptions() throws Exception {
+    Expression expr = compile("1");
+    assertArrayEquals(
+        Expression.class.getDeclaredMethod("evaluate", DoubleValues[].class).getExceptionTypes(),
+        expr.getClass().getDeclaredMethod("evaluate", DoubleValues[].class).getExceptionTypes());
+  }
+}