295 Zeilen
13 KiB
Diff
295 Zeilen
13 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Shane Freeder <theboyetronic@gmail.com>
|
|
Date: Sun, 17 Mar 2019 23:04:30 +0000
|
|
Subject: [PATCH] Test changes
|
|
|
|
- Allow use of TYPE_USE annotations
|
|
- Ignore package-private methods for nullability annotations
|
|
- Add excludes for classes which don't pass
|
|
- Disable stupid BukkitMirrorTest
|
|
|
|
Co-authored-by: Riley Park <rileysebastianpark@gmail.com>
|
|
Co-authored-by: Jake Potrebic <jake.m.potrebic@gmail.com>
|
|
|
|
diff --git a/build.gradle.kts b/build.gradle.kts
|
|
index 0b837b485bec96fa37ed65c18df97e55cecd0e9d..c8a8903d1b0c9822743549ecb8e4fdc7d0fd07c1 100644
|
|
--- a/build.gradle.kts
|
|
+++ b/build.gradle.kts
|
|
@@ -107,6 +107,12 @@ tasks.test {
|
|
useJUnitPlatform()
|
|
}
|
|
|
|
+// Paper start - compile tests with -parameters for better junit parameterized test names
|
|
+tasks.compileTestJava {
|
|
+ options.compilerArgs.add("-parameters")
|
|
+}
|
|
+// Paper end
|
|
+
|
|
// Paper start
|
|
val scanJar = tasks.register("scanJarForBadCalls", io.papermc.paperweight.tasks.ScanJarForBadCalls::class) {
|
|
badAnnotations.add("Lio/papermc/paper/annotation/DoNotUse;")
|
|
diff --git a/src/test/java/io/papermc/paper/testing/EmptyTag.java b/src/test/java/io/papermc/paper/testing/EmptyTag.java
|
|
new file mode 100644
|
|
index 0000000000000000000000000000000000000000..77154095cfb8b259bdb318e8ff40cb6f559ebc18
|
|
--- /dev/null
|
|
+++ b/src/test/java/io/papermc/paper/testing/EmptyTag.java
|
|
@@ -0,0 +1,31 @@
|
|
+package io.papermc.paper.testing;
|
|
+
|
|
+import java.util.Collections;
|
|
+import java.util.Set;
|
|
+import org.bukkit.Keyed;
|
|
+import org.bukkit.NamespacedKey;
|
|
+import org.bukkit.Tag;
|
|
+import org.jetbrains.annotations.NotNull;
|
|
+
|
|
+public record EmptyTag(NamespacedKey key) implements Tag<Keyed> {
|
|
+
|
|
+ @SuppressWarnings("deprecation")
|
|
+ public EmptyTag() {
|
|
+ this(NamespacedKey.randomKey());
|
|
+ }
|
|
+
|
|
+ @Override
|
|
+ public @NotNull NamespacedKey getKey() {
|
|
+ return this.key;
|
|
+ }
|
|
+
|
|
+ @Override
|
|
+ public boolean isTagged(@NotNull final Keyed item) {
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ @Override
|
|
+ public @NotNull Set<Keyed> getValues() {
|
|
+ return Collections.emptySet();
|
|
+ }
|
|
+}
|
|
diff --git a/src/test/java/org/bukkit/AnnotationTest.java b/src/test/java/org/bukkit/AnnotationTest.java
|
|
index 64e7aef6220097edefdff3b98a771b988365930d..f8b8969ee7a0b6f7b3224ff081e35c14a398c9d0 100644
|
|
--- a/src/test/java/org/bukkit/AnnotationTest.java
|
|
+++ b/src/test/java/org/bukkit/AnnotationTest.java
|
|
@@ -29,7 +29,13 @@ public class AnnotationTest {
|
|
"Lorg/jetbrains/annotations/Nullable;",
|
|
"Lorg/jetbrains/annotations/NotNull;",
|
|
"Lorg/jetbrains/annotations/Contract;",
|
|
- "Lorg/bukkit/UndefinedNullability;"
|
|
+ "Lorg/bukkit/UndefinedNullability;",
|
|
+ // Paper start
|
|
+ "Lorg/checkerframework/checker/nullness/qual/MonotonicNonNull;",
|
|
+ "Lorg/checkerframework/checker/nullness/qual/NonNull;",
|
|
+ "Lorg/checkerframework/checker/nullness/qual/Nullable;",
|
|
+ "Lorg/checkerframework/checker/nullness/qual/PolyNull;",
|
|
+ // Paper end
|
|
};
|
|
|
|
private static final String[] EXCLUDED_CLASSES = {
|
|
@@ -40,7 +46,17 @@ public class AnnotationTest {
|
|
"org/bukkit/util/io/Wrapper",
|
|
"org/bukkit/plugin/java/PluginClassLoader",
|
|
// Generic functional interface
|
|
- "org/bukkit/util/Consumer"
|
|
+ "org/bukkit/util/Consumer",
|
|
+ // Paper start
|
|
+ // Timings history is broken in terms of nullability due to guavas Function defining that the param is NonNull
|
|
+ "co/aikar/timings/TimingHistory$2",
|
|
+ "co/aikar/timings/TimingHistory$2$1",
|
|
+ "co/aikar/timings/TimingHistory$2$1$1",
|
|
+ "co/aikar/timings/TimingHistory$2$1$2",
|
|
+ "co/aikar/timings/TimingHistory$3",
|
|
+ "co/aikar/timings/TimingHistory$4",
|
|
+ "co/aikar/timings/TimingHistoryEntry$1"
|
|
+ // Paper end
|
|
};
|
|
|
|
@Test
|
|
@@ -61,20 +77,60 @@ public class AnnotationTest {
|
|
continue;
|
|
}
|
|
|
|
+ // Paper start - skip class if it's @NullMarked
|
|
+ if (isClassNullMarked(clazz, foundClasses)) {
|
|
+ continue;
|
|
+ }
|
|
+ // Paper end - skip class if it's @NullMarked
|
|
+
|
|
for (MethodNode method : clazz.methods) {
|
|
if (!isMethodIncluded(clazz, method, foundClasses)) {
|
|
continue;
|
|
}
|
|
|
|
if (mustBeAnnotated(Type.getReturnType(method.desc)) && !isWellAnnotated(method.invisibleAnnotations)) {
|
|
+ // Paper start - Allow use of TYPE_USE annotations
|
|
+ boolean warn = true;
|
|
+ if (isWellAnnotated(method.visibleTypeAnnotations)) {
|
|
+ warn = false;
|
|
+ } else if (method.invisibleTypeAnnotations != null) {
|
|
+ dance: for (final org.objectweb.asm.tree.TypeAnnotationNode invisibleTypeAnnotation : method.invisibleTypeAnnotations) {
|
|
+ final org.objectweb.asm.TypeReference ref = new org.objectweb.asm.TypeReference(invisibleTypeAnnotation.typeRef);
|
|
+ if (ref.getSort() == org.objectweb.asm.TypeReference.METHOD_RETURN && java.util.Arrays.asList(ACCEPTED_ANNOTATIONS).contains(invisibleTypeAnnotation.desc)) {
|
|
+ warn = false;
|
|
+ break dance; // cha cha real smooth
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+ if (warn)
|
|
+ // Paper end
|
|
warn(errors, clazz, method, "return value");
|
|
}
|
|
|
|
Type[] paramTypes = Type.getArgumentTypes(method.desc);
|
|
List<ParameterNode> parameters = method.parameters;
|
|
|
|
+ dancing: // Paper
|
|
for (int i = 0; i < paramTypes.length; i++) {
|
|
if (mustBeAnnotated(paramTypes[i]) ^ isWellAnnotated(method.invisibleParameterAnnotations == null ? null : method.invisibleParameterAnnotations[i])) {
|
|
+ // Paper start
|
|
+ if (method.invisibleTypeAnnotations != null) {
|
|
+ for (final org.objectweb.asm.tree.TypeAnnotationNode invisibleTypeAnnotation : method.invisibleTypeAnnotations) {
|
|
+ final org.objectweb.asm.TypeReference ref = new org.objectweb.asm.TypeReference(invisibleTypeAnnotation.typeRef);
|
|
+ if (ref.getSort() == org.objectweb.asm.TypeReference.METHOD_FORMAL_PARAMETER && ref.getTypeParameterIndex() == i && java.util.Arrays.asList(ACCEPTED_ANNOTATIONS).contains(invisibleTypeAnnotation.desc)) {
|
|
+ continue dancing;
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+ if (method.visibleTypeAnnotations != null) {
|
|
+ for (final org.objectweb.asm.tree.TypeAnnotationNode invisibleTypeAnnotation : method.visibleTypeAnnotations) {
|
|
+ final org.objectweb.asm.TypeReference ref = new org.objectweb.asm.TypeReference(invisibleTypeAnnotation.typeRef);
|
|
+ if (ref.getSort() == org.objectweb.asm.TypeReference.METHOD_FORMAL_PARAMETER && ref.getTypeParameterIndex() == i && java.util.Arrays.asList(ACCEPTED_ANNOTATIONS).contains(invisibleTypeAnnotation.desc)) {
|
|
+ continue dancing;
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+ // Paper end - Allow use of TYPE_USE annotations
|
|
ParameterNode paramNode = parameters == null ? null : parameters.get(i);
|
|
String paramName = paramNode == null ? null : paramNode.name;
|
|
|
|
@@ -91,13 +147,18 @@ public class AnnotationTest {
|
|
|
|
Collections.sort(errors);
|
|
|
|
- System.out.println(errors.size() + " missing annotation(s):");
|
|
+ StringBuilder builder = new StringBuilder()
|
|
+ .append("There ")
|
|
+ .append(errors.size() != 1 ? "are " : "is ")
|
|
+ .append(errors.size())
|
|
+ .append(" missing annotation")
|
|
+ .append(errors.size() != 1 ? "s:\n" : ":\n");
|
|
+
|
|
for (String message : errors) {
|
|
- System.out.print("\t");
|
|
- System.out.println(message);
|
|
+ builder.append("\t").append(message).append("\n");
|
|
}
|
|
|
|
- fail("There " + errors.size() + " are missing annotation(s)");
|
|
+ fail(builder.toString());
|
|
}
|
|
|
|
private static void collectClasses(@NotNull File from, @NotNull Map<String, ClassNode> to) throws IOException {
|
|
@@ -125,6 +186,23 @@ public class AnnotationTest {
|
|
}
|
|
}
|
|
|
|
+ // Paper start - skip class if it's @NullMarked
|
|
+ private static boolean isClassNullMarked(@NotNull ClassNode clazz, @NotNull Map<String, ClassNode> allClasses) {
|
|
+ if (clazz.nestHostClass != null) {
|
|
+ final ClassNode nestHostNode = allClasses.get(clazz.nestHostClass);
|
|
+ if (nestHostNode != null) {
|
|
+ return isClassNullMarked0(nestHostNode);
|
|
+ }
|
|
+ }
|
|
+
|
|
+ return isClassNullMarked0(clazz);
|
|
+ }
|
|
+
|
|
+ private static boolean isClassNullMarked0(@NotNull ClassNode clazz) {
|
|
+ return clazz.visibleAnnotations != null && clazz.visibleAnnotations.stream().anyMatch(node -> "Lorg/jspecify/annotations/NullMarked;".equals(node.desc));
|
|
+ }
|
|
+ // Paper end - skip class if it's @NullMarked
|
|
+
|
|
private static boolean isClassIncluded(@NotNull ClassNode clazz, @NotNull Map<String, ClassNode> allClasses) {
|
|
// Exclude private, synthetic or deprecated classes and annotations, since their members can't be null
|
|
if ((clazz.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_DEPRECATED | Opcodes.ACC_ANNOTATION)) != 0) {
|
|
@@ -140,6 +218,11 @@ public class AnnotationTest {
|
|
// Exceptions are excluded
|
|
return false;
|
|
}
|
|
+ // Paper start
|
|
+ if (isInternal(clazz.invisibleAnnotations)) {
|
|
+ return false;
|
|
+ }
|
|
+ // Paper end
|
|
|
|
for (String excludedClass : EXCLUDED_CLASSES) {
|
|
if (excludedClass.equals(clazz.name)) {
|
|
@@ -152,7 +235,7 @@ public class AnnotationTest {
|
|
|
|
private static boolean isMethodIncluded(@NotNull ClassNode clazz, @NotNull MethodNode method, @NotNull Map<String, ClassNode> allClasses) {
|
|
// Exclude private, synthetic and deprecated methods
|
|
- if ((method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_DEPRECATED)) != 0) {
|
|
+ if ((method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_DEPRECATED)) != 0 || (method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED | Opcodes.ACC_PUBLIC)) == 0) { // Paper - ignore package-private
|
|
return false;
|
|
}
|
|
|
|
@@ -170,11 +253,30 @@ public class AnnotationTest {
|
|
if ("<init>".equals(method.name) && isAnonymous(clazz)) {
|
|
return false;
|
|
}
|
|
+ // Paper start
|
|
+ if (isInternal(method.invisibleAnnotations)) {
|
|
+ return false;
|
|
+ }
|
|
+ // Paper end
|
|
|
|
return true;
|
|
}
|
|
+ // Paper start
|
|
+ private static boolean isInternal(List<? extends AnnotationNode> annotations) {
|
|
+ if (annotations == null) {
|
|
+ return false;
|
|
+ }
|
|
+ for (AnnotationNode node : annotations) {
|
|
+ if (node.desc.equals("Lorg/jetbrains/annotations/ApiStatus$Internal;")) {
|
|
+ return true;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ return false;
|
|
+ }
|
|
+ // Paper end
|
|
|
|
- private static boolean isWellAnnotated(@Nullable List<AnnotationNode> annotations) {
|
|
+ private static boolean isWellAnnotated(@Nullable List<? extends AnnotationNode> annotations) { // Paper
|
|
if (annotations == null) {
|
|
return false;
|
|
}
|
|
diff --git a/src/test/java/org/bukkit/BukkitMirrorTest.java b/src/test/java/org/bukkit/BukkitMirrorTest.java
|
|
index 89ca06ebecdaadd5dfc7bc74473ca15ad36f6eff..5974ceea58940e1799f3589eac0e39b925a42c3b 100644
|
|
--- a/src/test/java/org/bukkit/BukkitMirrorTest.java
|
|
+++ b/src/test/java/org/bukkit/BukkitMirrorTest.java
|
|
@@ -9,6 +9,7 @@ import org.junit.jupiter.params.ParameterizedTest;
|
|
import org.junit.jupiter.params.provider.Arguments;
|
|
import org.junit.jupiter.params.provider.MethodSource;
|
|
|
|
+@org.junit.jupiter.api.Disabled // Paper
|
|
public class BukkitMirrorTest {
|
|
|
|
public static Stream<Arguments> data() {
|
|
diff --git a/src/test/java/org/bukkit/support/TestServer.java b/src/test/java/org/bukkit/support/TestServer.java
|
|
index c67e0784c043ed194f6acde32411e156412a9b24..eb1fd4b911c4af76cdd3eac85d5365e7941a4a2e 100644
|
|
--- a/src/test/java/org/bukkit/support/TestServer.java
|
|
+++ b/src/test/java/org/bukkit/support/TestServer.java
|
|
@@ -83,6 +83,11 @@ public final class TestServer {
|
|
UnsafeValues unsafeValues = mock(withSettings().stubOnly());
|
|
when(instance.getUnsafe()).thenReturn(unsafeValues);
|
|
|
|
+ // Paper start - testing changes
|
|
+ when(instance.getTag(anyString(), any(NamespacedKey.class), any())).thenAnswer(ignored -> new io.papermc.paper.testing.EmptyTag());
|
|
+ when(instance.getScoreboardCriteria(anyString())).thenReturn(null);
|
|
+ // Paper end - testing changes
|
|
+
|
|
Bukkit.setServer(instance);
|
|
}
|
|
|