Fix compiler warnings (#565)

To improve code quality, almost all compiler warnings are now treated
as errors. The only exception (for now) is `exports`: Pioneer's
public API mentions a lot of Jupiter classes (e.g. all custom
annotations use Jupiter's annotations), which leads to warnings that
recommend to transitively require the corresponding Jupiter modules.
Doing that would mean that Pioneer users wouldn't have to require
Jupiter's modules, which is backwards - we're the appendix, here.
Since we don't want to pepper `@SuppressWarning("exports")`
everywhere, the warning is disabled.

Other than that, all warnings are either fixed or suppressed and need
to be from here on out. 

PR: #565
Issue: there wasn't one 😱

CONTRIBUTING.md

@@ -205,6 +205,17 @@ It helps with writing parallel tests for them if they do not change global state
 That particularly applies to "store in beforeEach - restore in afterEach"-extensions!
 If they fail after "store", they will still "restore" and thus potentially create a race condition with other tests.
 
+#### Compiler Warnings
+
+The build is configured to treat almost all compiler warnings as errors (see below for exceptions).
+If code that triggers a warning can't be refactored to avoid that, `@SuppressWarning` may be added, but we don't want to do that liberally.
+Developers and reviewers should minimize its use.
+
+Exceptions:
+* `exports` - Pioneer's public API mentions a lot of Jupiter classes (e.g. all custom annotations use Jupiter's annotations), which leads to warnings that recommend to transitively require the corresponding Jupiter modules.
+  Doing that would mean that Pioneer users wouldn't have to require Jupiter's modules, which is backwards - we're the appendix, here.
+  Since we don't want to pepper `@SuppressWarning("exports")` everywhere, the warning is disabled.
+
 ### Tests
 
 The name of test classes _must_ end with `Tests`, otherwise Gradle will ignore them.

build.gradle.kts

@@ -215,10 +215,15 @@ tasks {
 
 	compileJava {
 		options.encoding = "UTF-8"
+		options.compilerArgs.add("-Werror")
+		// do not break the build on "exports" warnings - see CONTRIBUTING.md for details
+		options.compilerArgs.add("-Xlint:all,-exports")
 	}
 
 	compileTestJava {
 		options.encoding = "UTF-8"
+		options.compilerArgs.add("-Werror")
+		options.compilerArgs.add("-Xlint:all")
 	}
 
 	test {
@@ -285,7 +290,7 @@ tasks {
 			// Set javadoc `--release` flag (affects which warnings and errors are reported)
 			// (Note: Gradle adds one leading '-' to the option on its own)
 			// Have to use at least Java 9 to support modular build
-			addStringOption("-release", maxOf(9, targetJavaVersion.majorVersion.toInt()).toString())
+			addStringOption("-release", maxOf(11, targetJavaVersion.majorVersion.toInt()).toString())
 
 			// Enable doclint, but ignore warnings for missing tags, see
 			// https://docs.oracle.com/en/java/javase/17/docs/specs/man/javadoc.html#additional-options-provided-by-the-standard-doclet

src/main/java/org/junitpioneer/internal/PioneerAnnotationUtils.java

@@ -56,33 +56,24 @@ private PioneerAnnotationUtils() {
 	}
 
 	/**
-	 * Determines whether an annotation of any of the specified {@code annotationTypes}
-	 * is either <em>present</em>, <em>indirectly present</em>, <em>meta-present</em>, or
+	 * Determines whether an annotation of the specified {@code annotationType} is either
+	 * <em>present</em>, <em>indirectly present</em>, <em>meta-present</em>, or
 	 * <em>enclosing-present</em> on the test element (method or class) belonging to the
 	 * specified {@code context}.
 	 */
-	public static boolean isAnyAnnotationPresent(ExtensionContext context,
-			Class<? extends Annotation>... annotationTypes) {
-		return Stream
-				.of(annotationTypes)
-				// to check for presence, we don't need all annotations - the closest ones suffice
-				.map(annotationType -> findClosestEnclosingAnnotation(context, annotationType))
-				.anyMatch(Optional::isPresent);
+	public static boolean isAnnotationPresent(ExtensionContext context, Class<? extends Annotation> annotationType) {
+		return findClosestEnclosingAnnotation(context, annotationType).isPresent();
 	}
 
 	/**
-	 * Determines whether an annotation of any of the specified repeatable {@code annotationTypes}
+	 * Determines whether an annotation of the specified repeatable {@code annotationType}
 	 * is either <em>present</em>, <em>indirectly present</em>, <em>meta-present</em>, or
 	 * <em>enclosing-present</em> on the test element (method or class) belonging to the specified
 	 * {@code context}.
 	 */
 	public static boolean isAnyRepeatableAnnotationPresent(ExtensionContext context,
-			Class<? extends Annotation>... annotationTypes) {
-		return Stream
-				.of(annotationTypes)
-				.flatMap(annotationType -> findClosestEnclosingRepeatableAnnotations(context, annotationType))
-				.iterator()
-				.hasNext();
+			Class<? extends Annotation> annotationType) {
+		return findClosestEnclosingRepeatableAnnotations(context, annotationType).iterator().hasNext();
 	}
 
 	/**

src/main/java/org/junitpioneer/jupiter/AbstractEntryBasedExtension.java

@@ -16,6 +16,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -97,17 +98,19 @@ private <A extends Annotation> Stream<A> findAnnotations(AnnotatedElement elemen
 		return AnnotationSupport.findRepeatableAnnotations(element, clazz).stream();
 	}
 
+	@SuppressWarnings("unchecked")
 	private Class<C> getClearAnnotationType() {
-		return getActualTypeArgumentAt(2);
+		return (Class<C>) getActualTypeArgumentAt(2);
 	}
 
+	@SuppressWarnings("unchecked")
 	private Class<S> getSetAnnotationType() {
-		return getActualTypeArgumentAt(3);
+		return (Class<S>) getActualTypeArgumentAt(3);
 	}
 
-	private <T> Class<T> getActualTypeArgumentAt(int index) {
+	private Type getActualTypeArgumentAt(int index) {
 		ParameterizedType abstractEntryBasedExtensionType = (ParameterizedType) getClass().getGenericSuperclass();
-		return (Class<T>) abstractEntryBasedExtensionType.getActualTypeArguments()[index];
+		return abstractEntryBasedExtensionType.getActualTypeArguments()[index];
 	}
 
 	private void preventClearAndSetSameEntries(Collection<K> entriesToClear, Collection<K> entriesToSet) {

src/main/java/org/junitpioneer/jupiter/CartesianEnumSource.java

@@ -26,7 +26,6 @@
 
 import org.junit.jupiter.params.provider.ArgumentsSource;
 import org.junit.platform.commons.PreconditionViolationException;
-import org.junitpioneer.jupiter.CartesianEnumSource.CartesianEnumSources;
 
 /**
  * {@code @CartesianEnumSource} is an argument source for constants of a
@@ -50,7 +49,7 @@
 @Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD })
 @Retention(RetentionPolicy.RUNTIME)
 @Documented
-@Repeatable(CartesianEnumSources.class)
+@Repeatable(CartesianEnumSource.CartesianEnumSources.class)
 @ArgumentsSource(CartesianEnumArgumentsProvider.class)
 @Deprecated
 public @interface CartesianEnumSource {

src/main/java/org/junitpioneer/jupiter/CartesianProductTest.java

@@ -104,6 +104,10 @@ class Sets {
 
 		private final List<List<?>> sets = new ArrayList<>(); //NOSONAR
 
+		// recreate default constructor to prevent compiler warning
+		public Sets() {
+		}
+
 		/**
 		 * Creates a single set of distinct objects (according to
 		 * {@link Object#equals(Object)}) for a CartesianProductTest

src/main/java/org/junitpioneer/jupiter/CartesianProductTestExtension.java

@@ -136,7 +136,7 @@ private ArgumentsProvider initializeArgumentsProvider(Annotation source) {
 	private List<Object> provideArguments(ExtensionContext context, Annotation source, ArgumentsProvider provider)
 			throws Exception {
 		if (provider instanceof CartesianAnnotationConsumer) {
-			((CartesianAnnotationConsumer) provider).accept(source);
+			((CartesianAnnotationConsumer<Annotation>) provider).accept(source);
 			return provider
 					.provideArguments(context)
 					.map(Arguments::get)

src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java

@@ -41,7 +41,7 @@ class RetryingTestExtension implements TestTemplateInvocationContextProvider, Te
 	public boolean supportsTestTemplate(ExtensionContext context) {
 		// the annotation only applies to methods (see its `@Target`),
 		// so it doesn't matter that this method checks meta-annotations
-		return PioneerAnnotationUtils.isAnyAnnotationPresent(context, RetryingTest.class);
+		return PioneerAnnotationUtils.isAnnotationPresent(context, RetryingTest.class);
 	}
 
 	@Override

src/main/java/org/junitpioneer/jupiter/StdOut.java

@@ -25,6 +25,10 @@ public class StdOut extends OutputStream {
 
 	private final StringWriter writer = new StringWriter();
 
+	// recreate default constructor to prevent compiler warning
+	public StdOut() {
+	}
+
 	@Override
 	public void write(int i) {
 		writer.write(i);

src/main/java/org/junitpioneer/jupiter/cartesian/ArgumentSets.java

@@ -83,6 +83,9 @@ public static <T> ArgumentSets argumentsForFirstParameter(Collection<T> argument
 	 * @return a new {@link ArgumentSets} object
 	 */
 	@SafeVarargs
+	// passing varargs on to another varargs method causes a warning
+	// that can't be fixed; only suppressed
+	@SuppressWarnings("varargs")
 	public static <T> ArgumentSets argumentsForFirstParameter(T... arguments) {
 		return new ArgumentSets(Arrays.asList(arguments));
 	}
@@ -125,6 +128,9 @@ public final <T> ArgumentSets argumentsForNextParameter(Collection<T> arguments)
 	 * @return this {@link ArgumentSets} object, for fluent set definitions
 	 */
 	@SafeVarargs
+	// passing varargs on to another varargs method causes a warning
+	// that can't be fixed; only suppressed
+	@SuppressWarnings("varargs")
 	public final <T> ArgumentSets argumentsForNextParameter(T... arguments) {
 		return add(Arrays.asList(arguments));
 	}

src/main/java/org/junitpioneer/jupiter/cartesian/CartesianArgumentsSource.java

@@ -38,6 +38,7 @@
 	/**
 	 * The type of {@link CartesianArgumentsProvider} to be used.
 	 */
+	@SuppressWarnings("rawtypes")
 	Class<? extends CartesianArgumentsProvider> value();
 
 }

src/main/java/org/junitpioneer/jupiter/params/RangeClass.java

@@ -24,6 +24,7 @@
 @Retention(RetentionPolicy.RUNTIME)
 @interface RangeClass {
 
+	@SuppressWarnings("rawtypes")
 	Class<? extends Range> value();
 
 }

src/main/java/org/junitpioneer/jupiter/params/RangeSourceArgumentsProvider.java

@@ -24,7 +24,6 @@
 import org.junit.jupiter.params.provider.ArgumentsProvider;
 import org.junit.jupiter.params.provider.ArgumentsSource;
 import org.junitpioneer.internal.PioneerAnnotationUtils;
-import org.junitpioneer.jupiter.CartesianAnnotationConsumer;
 import org.junitpioneer.jupiter.cartesian.CartesianParameterArgumentsProvider;
 
 /**
@@ -44,8 +43,10 @@
  * @see DoubleRangeSource
  * @see FloatRangeSource
  */
-class RangeSourceArgumentsProvider<N extends Number & Comparable<N>>
-		implements ArgumentsProvider, CartesianAnnotationConsumer<Annotation>, CartesianParameterArgumentsProvider<N> { //NOSONAR deprecated interface use will be removed in later release
+// CartesianAnnotationConsumer is deprecated for removal
+@SuppressWarnings("deprecation")
+class RangeSourceArgumentsProvider<N extends Number & Comparable<N>> implements ArgumentsProvider,
+		org.junitpioneer.jupiter.CartesianAnnotationConsumer<Annotation>, CartesianParameterArgumentsProvider<N> { //NOSONAR deprecated interface use will be removed in later release
 
 	// Once the CartesianAnnotationConsumer is removed we can make this provider stateless.
 	private Annotation argumentsSource;

src/main/java/org/junitpioneer/vintage/ExpectedExceptionExtension.java

@@ -66,6 +66,7 @@ public void afterTestExecution(ExtensionContext context) throws Exception {
 						.ifPresent(error -> {
 							throw error;
 						});
+				break;
 			case WAS_THROWN_AS_EXPECTED:
 				// the exception was thrown as expected so there is nothing to do
 				break;
@@ -80,6 +81,8 @@ public void afterTestExecution(ExtensionContext context) throws Exception {
 		}
 	}
 
+	// vintage @Test is deprecated (not for removal)
+	@SuppressWarnings("deprecation")
 	private static Optional<? extends Class<? extends Throwable>> expectedException(ExtensionContext context) {
 		return findAnnotation(context.getElement(), Test.class)
 				.map(Test::expected)

src/main/java/org/junitpioneer/vintage/TimeoutExtension.java

@@ -53,6 +53,8 @@ private void proceedWithTimeout(Invocation<Void> invocation, ExtensionContext ex
 			format(TEST_RAN_TOO_LONG, extensionContext.getDisplayName(), timeout));
 	}
 
+	// vintage @Test is deprecated (not for removal)
+	@SuppressWarnings("deprecation")
 	private Optional<Long> annotatedTimeout(ExtensionContext context) {
 		return findAnnotation(context.getElement(), Test.class).map(Test::timeout).filter(timeout -> timeout != 0L);
 	}

src/main/module/module-info.java

@@ -1,4 +1,16 @@
+/**
+ * JUnit Pioneer provides extensions for <a href="https://github.com/junit-team/junit5/">JUnit 5</a>
+ * and its Jupiter API.
+ *
+ * <p>Pioneer does not limit itself to proven ideas with wide application but is purposely open to
+ * experiments. It aims to spin off successful and cohesive portions into sibling projects or back
+ * into the JUnit 5 code base.
+ *
+ * <p>The dependencies on Jupiter modules could be marked as <code>transitive</code> but that would
+ * allow users who depend on this module to not `require` org.junit.*, which would be backwards.
+ */
 module org.junitpioneer {
+	// see Javadoc for why these aren't transitive
 	requires org.junit.jupiter.api;
 	requires org.junit.jupiter.params;
 	requires org.junit.platform.commons;

src/test/java/org/junitpioneer/internal/PioneerAnnotationUtilsTests.java

@@ -48,7 +48,7 @@ void notInheritedOnInterface() throws NoSuchMethodException {
 				AnnotationCluster.class.getMethod("notAnnotated"));
 
 			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, PioneerAnnotationUtilsTestCases.NotInheritedAnnotation.class);
+					.isAnnotationPresent(testContext, PioneerAnnotationUtilsTestCases.NotInheritedAnnotation.class);
 
 			assertThat(result).isTrue();
 		}
@@ -60,7 +60,7 @@ void notInheritedOnSuperclass() throws NoSuchMethodException {
 				Extender.class.getMethod("notAnnotated"));
 
 			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, PioneerAnnotationUtilsTestCases.NotInheritedAnnotation.class);
+					.isAnnotationPresent(testContext, PioneerAnnotationUtilsTestCases.NotInheritedAnnotation.class);
 
 			assertThat(result).isFalse();
 		}
@@ -71,8 +71,7 @@ void directlyPresent() throws NoSuchMethodException {
 			TestExtensionContext testContext = new TestExtensionContext(AnnotationCheck.class,
 				AnnotationCheck.class.getMethod("direct"));
 
-			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
+			boolean result = PioneerAnnotationUtils.isAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
 
 			assertThat(result).isTrue();
 		}
@@ -83,8 +82,7 @@ void indirectlyPresent() throws NoSuchMethodException {
 			TestExtensionContext testContext = new TestExtensionContext(Child.class,
 				Child.class.getMethod("notAnnotated"));
 
-			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
+			boolean result = PioneerAnnotationUtils.isAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
 
 			assertThat(result).isTrue();
 		}
@@ -95,8 +93,7 @@ void metaPresent() throws NoSuchMethodException {
 			TestExtensionContext testContext = new TestExtensionContext(AnnotationCheck.class,
 				AnnotationCheck.class.getMethod("meta"));
 
-			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
+			boolean result = PioneerAnnotationUtils.isAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
 
 			assertThat(result).isTrue();
 		}
@@ -107,8 +104,7 @@ void enclosingPresent() throws NoSuchMethodException {
 			TestExtensionContext testContext = new TestExtensionContext(Enclosing.class,
 				Enclosing.class.getMethod("notAnnotated"));
 
-			boolean result = PioneerAnnotationUtils
-					.isAnyAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
+			boolean result = PioneerAnnotationUtils.isAnnotationPresent(testContext, NonRepeatableTestAnnotation.class);
 
 			assertThat(result).isTrue();
 		}