Add Matcher#matches to ForbiddenApis to avoid its accidental use

Matcher#matches is only needed when implementing new Matcher implementations
which is very rare.

The common test pitfall is
assertThat(contains(actual).matches(expected), is(true))

It should better be written as
assertThat(actual, contains(expected))

build.gradle.kts

@@ -574,6 +574,7 @@ allprojects {
 
         configure<CheckForbiddenApisExtension> {
             failOnUnsupportedJava = false
+            ignoreSignaturesOfMissingClasses = true
             bundledSignatures.addAll(
                 listOf(
                     "jdk-unsafe",
@@ -675,7 +676,8 @@ allprojects {
                     "**/org/apache/calcite/runtime/Resources${'$'}Inst.class",
                     "**/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.class",
                     "**/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript${'$'}ShellCommand.class",
-                    "**/org/apache/calcite/util/Unsafe.class"
+                    "**/org/apache/calcite/util/Unsafe.class",
+                    "**/org/apache/calcite/test/Unsafe.class"
                 )
             }
 

core/src/test/java/org/apache/calcite/materialize/NormalizationTrimFieldTest.java

@@ -46,7 +46,6 @@
 
 import static org.apache.calcite.test.Matchers.isLinux;
 
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /** Tests trimming unused fields before materialized view matching. */
@@ -104,6 +103,6 @@ public static Frameworks.ConfigBuilder config() {
         + "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n"
         + "  LogicalTableScan(table=[[mv0]])\n";
     final String relOptimizedStr = RelOptUtil.toString(relOptimized.get(0).getKey());
-    assertThat(isLinux(optimized).matches(relOptimizedStr), is(true));
+    assertThat(relOptimizedStr, isLinux(optimized));
   }
 }

core/src/test/java/org/apache/calcite/test/AbstractMaterializedViewTest.java

@@ -51,6 +51,7 @@
 import org.apache.calcite.util.ImmutableBeans;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.TestUtil;
+import org.apache.calcite.util.Util;
 
 import com.google.common.collect.ImmutableList;
 
@@ -76,8 +77,9 @@ public abstract class AbstractMaterializedViewTest {
   protected Function<String, Boolean> resultContains(
       final String... expected) {
     return s -> {
+      String sLinux = Util.toLinux(s);
       for (String st : expected) {
-        if (!Matchers.containsStringLinux(st).matches(s)) {
+        if (!sLinux.contains(Util.toLinux(st))) {
           return false;
         }
       }

core/src/test/java/org/apache/calcite/test/Matchers.java

@@ -362,7 +362,7 @@ private static class ComposingMatcher<F, T> extends TypeSafeMatcher<F> {
     }
 
     protected boolean matchesSafely(F item) {
-      return matcher.matches(f.apply(item));
+      return Unsafe.matches(matcher, f.apply(item));
     }
 
     public void describeTo(Description description) {

core/src/test/java/org/apache/calcite/test/Unsafe.java

@@ -0,0 +1,43 @@
+/*
+ * 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.calcite.test;
+
+import org.hamcrest.Matcher;
+
+/**
+ * Contains methods that call JDK methods that the
+ * <a href="https://github.com/policeman-tools/forbidden-apis">forbidden
+ * APIs checker</a> does not approve of.
+ *
+ * <p>This class is excluded from the check, so methods called via this class
+ * will not fail the build.
+ */
+public class Unsafe {
+  private Unsafe() {}
+
+  /**
+   * {@link Matcher#matches(Object)} is forbidden in regular test code in favour of
+   * {@link org.hamcrest.MatcherAssert#assertThat}.
+   * Note: {@code Matcher#matches} is still useful when testing matcher implementations.
+   * @param matcher matcher
+   * @param actual actual value
+   * @return the result of matcher.matches(actual)
+   */
+  public static <T> boolean matches(Matcher<T> matcher, Object actual) {
+    return matcher.matches(actual);
+  }
+}

core/src/test/java/org/apache/calcite/util/UtilTest.java

@@ -36,6 +36,7 @@
 import org.apache.calcite.sql.util.SqlString;
 import org.apache.calcite.test.DiffTestCase;
 import org.apache.calcite.test.Matchers;
+import org.apache.calcite.test.Unsafe;
 import org.apache.calcite.testlib.annotations.LocaleEnUs;
 
 import com.google.common.collect.ImmutableList;
@@ -2720,13 +2721,10 @@ private void checkNameMultimap(String s, NameMultimap<Integer> map) {
 
   /** Unit test for {@link Matchers#compose}. */
   @Test void testComposeMatcher() {
-    assertThat("x", is("x"));
-    assertThat(is("x").matches("x"), is(true));
-    assertThat(is("X").matches("x"), is(false));
     final Function<String, String> toUpper = s -> s.toUpperCase(Locale.ROOT);
-    assertThat(Matchers.compose(is("A"), toUpper).matches("a"), is(true));
-    assertThat(Matchers.compose(is("A"), toUpper).matches("A"), is(true));
-    assertThat(Matchers.compose(is("a"), toUpper).matches("A"), is(false));
+    assertThat(Unsafe.matches(Matchers.compose(is("A"), toUpper), "a"), is(true));
+    assertThat(Unsafe.matches(Matchers.compose(is("A"), toUpper), "A"), is(true));
+    assertThat(Unsafe.matches(Matchers.compose(is("a"), toUpper), "A"), is(false));
     assertThat(describe(Matchers.compose(is("a"), toUpper)), is("is \"a\""));
     assertThat(mismatchDescription(Matchers.compose(is("a"), toUpper), "A"),
         is("was \"A\""));
@@ -2737,18 +2735,18 @@ private void checkNameMultimap(String s, NameMultimap<Integer> map) {
     assertThat("xy", isLinux("xy"));
     assertThat("x\ny", isLinux("x\ny"));
     assertThat("x\r\ny", isLinux("x\ny"));
-    assertThat(isLinux("x").matches("x"), is(true));
-    assertThat(isLinux("X").matches("x"), is(false));
+    assertThat(Unsafe.matches(isLinux("x"), "x"), is(true));
+    assertThat(Unsafe.matches(isLinux("X"), "x"), is(false));
     assertThat(mismatchDescription(isLinux("X"), "x"), is("was \"x\""));
     assertThat(describe(isLinux("X")), is("is \"X\""));
-    assertThat(isLinux("x\ny").matches("x\ny"), is(true));
-    assertThat(isLinux("x\ny").matches("x\r\ny"), is(true));
+    assertThat(Unsafe.matches(isLinux("x\ny"), "x\ny"), is(true));
+    assertThat(Unsafe.matches(isLinux("x\ny"), "x\r\ny"), is(true));
     //\n\r is not a valid windows line ending
-    assertThat(isLinux("x\ny").matches("x\n\ry"), is(false));
-    assertThat(isLinux("x\ny").matches("x\n\ryz"), is(false));
+    assertThat(Unsafe.matches(isLinux("x\ny"), "x\n\ry"), is(false));
+    assertThat(Unsafe.matches(isLinux("x\ny"), "x\n\ryz"), is(false));
     // left-hand side must be linux or will never match
-    assertThat(isLinux("x\r\ny").matches("x\r\ny"), is(false));
-    assertThat(isLinux("x\r\ny").matches("x\ny"), is(false));
+    assertThat(Unsafe.matches(isLinux("x\r\ny"), "x\r\ny"), is(false));
+    assertThat(Unsafe.matches(isLinux("x\r\ny"), "x\ny"), is(false));
   }
 
   /** Tests {@link Util#andThen(UnaryOperator, UnaryOperator)}. */

example/function/build.gradle.kts

@@ -19,10 +19,5 @@ dependencies {
     api(project(":linq4j"))
     api("org.checkerframework:checker-qual")
 
-    implementation("com.google.guava:guava") {
-        because("""ForbiddenApis' signatures.txt contains com.google.common.base.Precondition
-          and it needs the file on a classpath to parse the configuration"""
-        )
-    }
     testImplementation("sqlline:sqlline")
 }

src/main/config/forbidden-apis/signatures.txt

@@ -101,3 +101,6 @@ com.google.common.collect.Maps#newTreeMap()
 
 @defaultMessage Use "new HashSet<>()"
 com.google.common.collect.Sets#newHashSet()
+
+@defaultMessage Use "assertThat(expected, matcher)", do not call Matcher#matches directly
+org.hamcrest.Matcher#matches(java.lang.Object)