Enable ErrorProne check: CheckReturnValue

This catches some instances of mis-used `assertThat()` and some places
where apparently `withMessage` on exception assertion was intended.

There's also one big method wich calls several of data parsing methods
and is completely disinterested in the results, apart from validating,
that they are valid values. That one is suppressed in its entirety.

core/trino-main/src/test/java/io/trino/operator/scalar/FunctionAssertions.java

@@ -394,7 +394,8 @@ private void assertCachedInstanceHasBoundedRetainedSizeInTx(String projection, S
                     newSimpleAggregatedMemoryContext().newLocalMemoryContext(PageProcessor.class.getSimpleName()),
                     SOURCE_PAGE);
             // consume the iterator
-            Iterators.getOnlyElement(output);
+            @SuppressWarnings("unused")
+            Optional<Page> ignored = Iterators.getOnlyElement(output);
 
             long retainedSize = processor.getProjections().stream()
                     .mapToLong(this::getRetainedSizeOfCachedInstance)

core/trino-main/src/test/java/io/trino/operator/window/matcher/MatchAssert.java

@@ -66,7 +66,7 @@ public MatchAssert hasLabels(char[] expectedLabels)
         for (int i = 0; i < expectedLabels.length; i++) {
             mappedExpected[i] = labelMapping.get(new IrLabel(String.valueOf(expectedLabels[i])));
         }
-        return satisfies(actual -> assertThat(actual.isMatched()))
+        return satisfies(actual -> assertThat(actual.isMatched()).isTrue())
                 .satisfies(actual -> assertThat(actual.getLabels().toArray())
                         .as("Matched labels")
                         .isEqualTo(mappedExpected));
@@ -75,7 +75,7 @@ public MatchAssert hasLabels(char[] expectedLabels)
     @CanIgnoreReturnValue
     public MatchAssert hasCaptures(int[] expectedCaptures)
     {
-        return satisfies(actual -> assertThat(actual.isMatched()))
+        return satisfies(actual -> assertThat(actual.isMatched()).isTrue())
                 .satisfies(actual -> assertThat(actual.getExclusions().toArray())
                         .as("Captured exclusions")
                         .isEqualTo(expectedCaptures));
@@ -84,7 +84,7 @@ public MatchAssert hasCaptures(int[] expectedCaptures)
     @CanIgnoreReturnValue
     public MatchAssert isNoMatch()
     {
-        return satisfies(actual -> assertThat(!actual.isMatched()));
+        return satisfies(actual -> assertThat(actual.isMatched()).isFalse());
     }
 
     private static LabelEvaluator identityEvaluator(int[] input)

plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestCassandraConnector.java

@@ -288,6 +288,7 @@ public void testGetTupleType()
         assertEquals(rowNumber, 2);
     }
 
+    @SuppressWarnings({"ResultOfMethodCallIgnored", "CheckReturnValue"}) // we only check if the values are valid, we don't need them otherwise
     private static void assertReadFields(RecordCursor cursor, List<ColumnMetadata> schema)
     {
         for (int columnIndex = 0; columnIndex < schema.size(); columnIndex++) {

plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestBrokerQueries.java

@@ -169,6 +169,6 @@ public void testBrokerResponseHasTooManyRows()
                 LIMIT_FOR_BROKER_QUERIES);
         assertThatExceptionOfType(PinotException.class)
                 .isThrownBy(() -> pageSource.getNextPage())
-                .withFailMessage("Broker query returned '3' rows, maximum allowed is '2' rows. with query \"SELECT col_1, col_2, col_3 FROM test_table\"");
+                .withMessage("Broker query returned '3' rows, maximum allowed is '2' rows. with query \"SELECT col_1, col_2, col_3 FROM test_table\"");
     }
 }

plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestDynamicTable.java

@@ -132,7 +132,7 @@ public void testDoubleWithScientificNotation()
         String tableName = "primitive_types_table";
         String query = "SELECT string_col FROM " + tableName + " WHERE double_col = 3.5E5";
         assertThatNullPointerException().isThrownBy(() -> buildFromPql(pinotMetadata, new SchemaTableName("default", query)))
-                .withFailMessage("");
+                .withMessage(Runtime.version().feature() < 15 ? null : "Cannot invoke \"java.lang.Integer.intValue()\" because \"this.scale\" is null");
     }
 
     @Test

pom.xml

@@ -1721,6 +1721,7 @@
                                     -Xep:BadInstanceof:ERROR
                                     -Xep:BadShiftAmount:ERROR
                                     -Xep:BoxedPrimitiveConstructor:ERROR
+                                    -Xep:CheckReturnValue:ERROR
                                     -Xep:ClassCanBeStatic:ERROR
                                     -Xep:CompareToZero:ERROR
                                     -Xep:ComparingThisWithNull:ERROR

testing/trino-product-tests/src/main/java/io/trino/tests/product/TestAlterTable.java

@@ -73,7 +73,7 @@ public void renameColumn()
         assertQueryFailure(() -> query(format("ALTER TABLE %s RENAME COLUMN nationkey TO n_regionkeY", TABLE_NAME)))
                 .hasMessageContaining("Column 'n_regionkey' already exists");
 
-        assertThat(query(format("ALTER TABLE %s RENAME COLUMN nationkey TO n_nationkey", TABLE_NAME)));
+        query(format("ALTER TABLE %s RENAME COLUMN nationkey TO n_nationkey", TABLE_NAME));
     }
 
     @Test(groups = {ALTER_TABLE, SMOKE})

testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveIgnoreAbsentPartitions.java

@@ -78,7 +78,7 @@ public void testShouldThrowErrorOnUnpartitionedTableMissingData()
     {
         String tableName = "unpartitioned_absent_table_data";
 
-        assertThat(query("DROP TABLE IF EXISTS " + tableName));
+        query("DROP TABLE IF EXISTS " + tableName);
 
         assertThat(query(format("CREATE TABLE %s AS SELECT * FROM (VALUES 1,2,3) t(dummy_col)", tableName))).containsOnly(row(3));
         assertThat(query("SELECT count(*) FROM " + tableName)).containsOnly(row(3));
@@ -93,6 +93,6 @@ public void testShouldThrowErrorOnUnpartitionedTableMissingData()
         query("SET SESSION hive.ignore_absent_partitions = true");
         assertQueryFailure(() -> query("SELECT count(*) FROM " + tableName)).hasMessageContaining("Partition location does not exist");
 
-        assertThat(query("DROP TABLE " + tableName));
+        query("DROP TABLE " + tableName);
     }
 }