diff --git a/CMakeLists.txt b/CMakeLists.txt index 794885abe8..e7592ac89c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -289,6 +289,7 @@ endif() set(FILENAME_DEP_REPLACE "get_filename_component(SELF_DIR \"$\{CMAKE_CURRENT_LIST_FILE\}\" PATH)") set(SELF_DIR "$\{SELF_DIR\}") set(WPIUNITS_DEP_REPLACE_IMPL "find_dependency(wpiunits)") +set(WPIANNOTATIONS_DEP_REPLACE_IMPL "find_dependency(wpiannotations)") set(WPIUTIL_DEP_REPLACE "find_dependency(wpiutil)") add_subdirectory(wpiutil) @@ -308,6 +309,10 @@ if(WITH_WPIMATH) add_subdirectory(wpimath) endif() +if(WITH_JAVA) + add_subdirectory(wpiannotations) +endif() + if(WITH_WPIUNITS AND NOT WITH_WPIMATH) # In case of building wpiunits standalone set(WPIUNITS_DEP_REPLACE ${WPIUNITS_DEP_REPLACE_IMPL}) diff --git a/docs/build.gradle b/docs/build.gradle index a500a38961..423dc96cb2 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -9,6 +9,7 @@ evaluationDependsOn(':cscore') evaluationDependsOn(':epilogue-runtime') evaluationDependsOn(':hal') evaluationDependsOn(':ntcore') +evaluationDependsOn(':wpiannotations') evaluationDependsOn(':wpilibNewCommands') evaluationDependsOn(':wpilibc') evaluationDependsOn(':wpilibj') @@ -174,6 +175,7 @@ task generateJavaDocs(type: Javadoc) { source project(':epilogue-runtime').sourceSets.main.java source project(':hal').sourceSets.main.java source project(':ntcore').sourceSets.main.java + source project(':wpiannotations').sourceSets.main.java source project(':wpilibNewCommands').sourceSets.main.java source project(':wpilibj').sourceSets.main.java source project(':wpimath').sourceSets.main.java diff --git a/javacPlugin/BUILD.bazel b/javacPlugin/BUILD.bazel new file mode 100644 index 0000000000..2c3b786338 --- /dev/null +++ b/javacPlugin/BUILD.bazel @@ -0,0 +1,11 @@ +load("@rules_java//java:defs.bzl", "java_plugin") + +java_plugin( + name = "plugin", + srcs = glob(["src/main/java/**/*.java"]), + resources = glob(["src/main/resources/**"]), + visibility = ["//visibility:public"], + deps = [ + "//wpiannotations", + ], +) diff --git a/javacPlugin/build.gradle b/javacPlugin/build.gradle new file mode 100644 index 0000000000..89adb15320 --- /dev/null +++ b/javacPlugin/build.gradle @@ -0,0 +1,17 @@ +ext { + useJava = true + useCpp = false + baseId = 'wpilibj-javac-plugin' + groupId = 'org.wpilib' + + nativeName = '' + devMain = '' +} + +apply from: "${rootDir}/shared/java/javacommon.gradle" + +dependencies { + implementation project(':wpiannotations') + testImplementation 'com.google.testing.compile:compile-testing:+' + testImplementation project(':wpilibNewCommands') +} diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java new file mode 100644 index 0000000000..4481e933fa --- /dev/null +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java @@ -0,0 +1,224 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.TaskEvent; +import com.sun.source.util.TaskListener; +import com.sun.source.util.TreeScanner; +import com.sun.source.util.Trees; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.tools.Diagnostic; +import org.wpilib.annotation.NoDiscard; + +/** Checks for usages of methods that require their return values to be used. */ +public class ReturnValueUsedListener implements TaskListener { + private final JavacTask m_task; + private final Set m_visitedCUs = new HashSet<>(); + + public ReturnValueUsedListener(JavacTask task) { + m_task = task; + } + + @Override + public void finished(TaskEvent e) { + // We override `finished` instead of `started` because we want to run after the + // ANALYZE attribution phase has completed and assigned types to elements in the AST + // Track the visited CUs to avoid re-processing the same CU multiple times when we call + // `Trees.getElement()` on a tree path. + if (e.getKind() == TaskEvent.Kind.ANALYZE && m_visitedCUs.add(e.getCompilationUnit())) { + e.getCompilationUnit().accept(new Scanner(e.getCompilationUnit()), null); + } + } + + private final class Scanner extends TreeScanner { + private final CompilationUnitTree m_root; + private final Trees m_trees; + + Scanner(CompilationUnitTree compilationUnit) { + m_root = compilationUnit; + m_trees = Trees.instance(m_task); + } + + @Override + public Void visitMethodInvocation(MethodInvocationTree node, Void unused) { + checkIgnoredExpression(node); + return super.visitMethodInvocation(node, unused); + } + + @Override + public Void visitNewClass(NewClassTree node, Void unused) { + checkIgnoredExpression(node); + return super.visitNewClass(node, unused); + } + + /** + * Common logic for both method invocations and constructor calls when they appear as + * stand-alone expression statements (i.e., their result is ignored). + */ + private void checkIgnoredExpression(Tree node) { + var path = m_trees.getPath(m_root, node); + + // Walk the tree upwards to see if the node is directly or indirectly annotated with + // @SuppressWarnings("NoDiscard") or @SuppressWarnings("all"). If so, then we ignore any + // @NoDiscard messages for this node + for (var currentPath = path; currentPath != null; currentPath = currentPath.getParentPath()) { + var element = m_trees.getElement(currentPath); + if (element == null) { + continue; + } + + if (element.getAnnotation(SuppressWarnings.class) != null) { + String[] suppressions = element.getAnnotation(SuppressWarnings.class).value(); + for (String suppression : suppressions) { + if ("NoDiscard".equals(suppression) || "all".equals(suppression)) { + return; + } + } + } + } + + var parentPath = (path == null) ? null : path.getParentPath(); + if (parentPath == null || parentPath.getLeaf().getKind() != Tree.Kind.EXPRESSION_STATEMENT) { + // If the parent node is an expression statement, then the value is ignored. + // Otherwise, the value is used and we can ignore this site. + return; + } + + // Resolve the static type of the expression + TypeMirror type = getType(node); + if (type == null || type.getKind() == TypeKind.VOID) { + // Skip void (e.g., void-returning methods) + return; + } + + // Check @NoDiscard on the invoked executable (method or constructor) + var invoked = getInvokedExecutable(node); + if (invoked != null) { + List messages = getNoDiscardMessages(invoked); + for (String msg : messages) { + m_trees.printMessage(Diagnostic.Kind.ERROR, msg, node, m_root); + } + } + } + + private TypeMirror getType(Tree node) { + var path = m_trees.getPath(m_root, node); + if (path == null) { + return null; + } + // Requires running after ANALYZE attribution has completed for this CU. + return m_trees.getTypeMirror(path); + } + + private ExecutableElement getInvokedExecutable(Tree node) { + var path = m_trees.getPath(m_root, node); + if (path == null) { + return null; + } + var el = m_trees.getElement(path); + return (el instanceof ExecutableElement ee) ? ee : null; + } + + /** + * Collects all @NoDiscard messages applicable to the given executable: - The method/constructor + * itself (if annotated) - The return type (if declared) including its superclasses and all + * implemented interfaces Returns formatted diagnostics messages ready to print. + */ + private List getNoDiscardMessages(ExecutableElement method) { + List messages = new ArrayList<>(); + + // 1) Method-level @NoDiscard + var methodNoDiscard = method.getAnnotation(NoDiscard.class); + if (methodNoDiscard != null) { + String msg = methodNoDiscard.value(); + if (msg.isEmpty()) { + messages.add("Result of @NoDiscard method is ignored"); + } else { + messages.add(msg); + } + } + + // 2) Type-level @NoDiscard (classes + interfaces recursively) + TypeElement targetType = null; + if (method.getKind() == ElementKind.CONSTRUCTOR) { + // For constructors, the "return type" is the enclosing type + var enclosing = method.getEnclosingElement(); + if (enclosing instanceof TypeElement te) { + targetType = te; + } + } else { + var returnType = method.getReturnType(); + if (returnType instanceof DeclaredType dt && dt.asElement() instanceof TypeElement te) { + targetType = te; + } + } + if (targetType != null) { + Set seen = new HashSet<>(); + collectNoDiscardMessagesFromTypeHierarchy(targetType, seen, messages); + } + + return messages; + } + + /** + * Searches for @NoDiscard on the provided type element, its superclasses, and all implemented + * interfaces (recursively). Appends formatted messages to the provided list for every match. + * + * @param type The type element to search + * @param seen A set of type elements that have already been searched + * @param out The list to append messages to + */ + private void collectNoDiscardMessagesFromTypeHierarchy( + TypeElement type, Set seen, List out) { + if (type == null || !seen.add(type)) { + return; + } + + // Check this type directly + var typeNoDiscard = type.getAnnotation(NoDiscard.class); + if (typeNoDiscard != null) { + String message = typeNoDiscard.value(); + if (message.isEmpty()) { + out.add( + "Result of method returning @NoDiscard type %s is ignored" + .formatted(type.getQualifiedName())); + } else { + out.add(message); + } + } + + // Check superclass chain + var superMirror = type.getSuperclass(); + if (superMirror != null && superMirror.getKind() != TypeKind.NONE) { + var superEl = m_task.getTypes().asElement(superMirror); + if (superEl instanceof TypeElement ste) { + collectNoDiscardMessagesFromTypeHierarchy(ste, seen, out); + } + } + + // Check all implemented interfaces (recursively) + for (var iface : type.getInterfaces()) { + var ifaceEl = m_task.getTypes().asElement(iface); + if (ifaceEl instanceof TypeElement ite) { + collectNoDiscardMessagesFromTypeHierarchy(ite, seen, out); + } + } + } + } +} diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java new file mode 100644 index 0000000000..87bc8f43f4 --- /dev/null +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java @@ -0,0 +1,31 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import com.sun.source.util.JavacTask; +import com.sun.source.util.Plugin; + +/** + * A javac compiler plugin that adds compiler warnings for incorrect usage of WPILib types. Also + * supports WPILib's custom annotations like @NoDiscard. + */ +public class WPILibJavacPlugin implements Plugin { + @Override + public String getName() { + return "WPILib"; + } + + @Override + public void init(JavacTask task, String... args) { + task.addTaskListener(new ReturnValueUsedListener(task)); + } + + @Override + public boolean autoStart() { + // autoStart means we don't need to manually pass -Xplugin:WPILib to the javac compiler args + // for the plugin to run + return true; + } +} diff --git a/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin b/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin new file mode 100644 index 0000000000..e1b3dbab39 --- /dev/null +++ b/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin @@ -0,0 +1 @@ +org.wpilib.javacplugin.WPILibJavacPlugin diff --git a/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java b/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java new file mode 100644 index 0000000000..a5356b4c81 --- /dev/null +++ b/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java @@ -0,0 +1,13 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import java.util.List; + +public class CompileTestOptions { + public static final int kJavaVersion = 17; + public static final List kJavaVersionOptions = + List.of("-source", kJavaVersion, "-target", kJavaVersion); +} diff --git a/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java b/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java new file mode 100644 index 0000000000..65a21ce2f0 --- /dev/null +++ b/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java @@ -0,0 +1,638 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import static com.google.testing.compile.CompilationSubject.assertThat; +import static com.google.testing.compile.Compiler.javac; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.wpilib.javacplugin.CompileTestOptions.kJavaVersionOptions; + +import com.google.testing.compile.Compilation; +import com.google.testing.compile.JavaFileObjects; +import org.junit.jupiter.api.Test; + +class ReturnValueUsedListenerTest { + @Test + void nodiscardReturnValueIsUsed() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + int getI() { return 0; } + + void usage() { + int i = getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void nodiscardReturnValueUnused() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Result of @NoDiscard method is ignored", error.getMessage(null)); + } + + @Test + void nodiscardOnClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard + class Example { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Result of method returning @NoDiscard type frc.robot.Example is ignored", + error.getMessage(null)); + } + + @Test + void nodiscardOnClassCustomMessage() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Custom message") + class Example { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Custom message", error.getMessage(null)); + } + + @Test + void nodiscardOnClassAndMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard + class Example { + @NoDiscard + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(2, compilation.errors().size()); + var error1 = compilation.errors().get(0); + var error2 = compilation.errors().get(1); + assertEquals("Result of @NoDiscard method is ignored", error1.getMessage(null)); + assertEquals( + "Result of method returning @NoDiscard type frc.robot.Example is ignored", + error2.getMessage(null)); + } + + @Test + void nodiscardOnInheritedClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects of type `Base` must be used") + abstract class Base { } + + class Example extends Base { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Objects of type `Base` must be used", error.getMessage(null)); + } + + @Test + void nodiscardOnSingleInterface() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects implementing `I` must be used") + interface I { } + + class Example implements I { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Objects implementing `I` must be used", error.getMessage(null)); + } + + @Test + void nodiscardOnMultipleInterfaces() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects implementing `I` must be used") + interface I { } + + @NoDiscard("Objects implementing `I2` must be used") + interface I2 { } + + class Example implements I, I2 { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(2, compilation.errors().size()); + var error1 = compilation.errors().get(0); + var error2 = compilation.errors().get(1); + assertEquals("Objects implementing `I` must be used", error1.getMessage(null)); + assertEquals("Objects implementing `I2` must be used", error2.getMessage(null)); + } + + @Test + void nodiscardCustomMessage() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard("Custom message") + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Custom message", error.getMessage(null)); + } + + @Test + void nodiscardMessageEmptyString() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard("") + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Result of @NoDiscard method is ignored", error.getMessage(null)); + } + + @Test + void nodiscardOnVoidMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + void voidMethod() { } + + void usage() { + voidMethod(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsOnNoDiscardMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + Object get() { return null; } + + @SuppressWarnings("NoDiscard") + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsAllOnNoDiscardMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + Object get() { return null; } + + @SuppressWarnings("all") + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsOnNoDiscardClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @SuppressWarnings("NoDiscard") + class Example { + @NoDiscard + Object get() { return null; } + + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsAllOnNoDiscardClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @SuppressWarnings("all") + class Example { + @NoDiscard + Object get() { return null; } + + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsAssigned() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + Command theCommand = getCommand(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsPassed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + System.out.println(getCommand()); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsChainedAndUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + Command theCommand = getCommand().withName("The name"); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + getCommand(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } + + @Test + void commandsv2CommandFactoryResultIsChainedAndNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + getCommand().withName("The name"); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } + + @Test + void commandsv2NewCommandInstanceNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import edu.wpi.first.wpilibj2.command.WaitCommand; + import org.wpilib.annotation.NoDiscard; + + class Example { + void usage() { + new WaitCommand(1); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } +} diff --git a/settings.gradle b/settings.gradle index 2064a767cd..3c34412c98 100644 --- a/settings.gradle +++ b/settings.gradle @@ -33,6 +33,8 @@ include 'wpilibcIntegrationTests' include 'wpilibjExamples' include 'wpilibjIntegrationTests' include 'wpilibj' +include 'javacPlugin' +include 'wpiannotations' include 'wpiunits' include 'crossConnIntegrationTests' include 'fieldImages' diff --git a/wpiannotations/BUILD.bazel b/wpiannotations/BUILD.bazel new file mode 100644 index 0000000000..ee4b2401c0 --- /dev/null +++ b/wpiannotations/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "wpiannotations", + srcs = glob(["src/main/java/**/*.java"]), + visibility = ["//visibility:public"], + deps = [], +) diff --git a/wpiannotations/CMakeLists.txt b/wpiannotations/CMakeLists.txt new file mode 100644 index 0000000000..11eabdbc29 --- /dev/null +++ b/wpiannotations/CMakeLists.txt @@ -0,0 +1,37 @@ +project(wpiannotations) + +# Java bindings +if(WITH_JAVA) + include(UseJava) + + file(GLOB_RECURSE JAVA_SOURCES src/main/java/*.java) + + add_jar( + wpiannotations_jar + ${JAVA_SOURCES} + OUTPUT_NAME wpiannotations + OUTPUT_DIR ${WPILIB_BINARY_DIR}/${java_lib_dest} + ) + set_property(TARGET wpiannotations_jar PROPERTY FOLDER "java") + + install_jar(wpiannotations_jar DESTINATION ${java_lib_dest}) + install_jar_exports( + TARGETS wpiannotations_jar + FILE wpiannotations.cmake + DESTINATION share/wpiannotations + ) + install(FILES wpiannotations-config.cmake DESTINATION share/wpiannotations) +endif() + +if(WITH_JAVA_SOURCE) + include(UseJava) + include(CreateSourceJar) + add_source_jar( + wpiannotations_src_jar + BASE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/src/main/java + OUTPUT_NAME wpiannotations-sources + ) + set_property(TARGET wpiannotations_src_jar PROPERTY FOLDER "java") + + install_jar(wpiannotations_src_jar DESTINATION ${java_lib_dest}) +endif() diff --git a/wpiannotations/build.gradle b/wpiannotations/build.gradle new file mode 100644 index 0000000000..2a7a0881d7 --- /dev/null +++ b/wpiannotations/build.gradle @@ -0,0 +1,11 @@ +ext { + useJava = true + useCpp = false + baseId = 'annotations' + groupId = 'org.wpilib' + + nativeName = '' + devMain = '' +} + +apply from: "${rootDir}/shared/java/javacommon.gradle" diff --git a/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java b/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java new file mode 100644 index 0000000000..408d1fb861 --- /dev/null +++ b/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java @@ -0,0 +1,27 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a method as returning a value that must be used. The WPILib compiler plugin will check for + * uses of methods with this annotation and report a compiler error if the value is unused. Marking + * a class or interface as {@code @NoDiscard} will act as if any method that returns that type or + * any subclass or implementor of that type has been marked with {@code @NoDiscard}. + */ +@Target({ElementType.METHOD, ElementType.TYPE}) +@Retention(RetentionPolicy.CLASS) // needs to be stored in the class for use by libraries +public @interface NoDiscard { + /** + * An error message to display if the return value is not used. + * + * @return The error message. + */ + String value() default ""; +} diff --git a/wpiannotations/wpiannotations-config.cmake b/wpiannotations/wpiannotations-config.cmake new file mode 100644 index 0000000000..fa28e4a9c2 --- /dev/null +++ b/wpiannotations/wpiannotations-config.cmake @@ -0,0 +1,2 @@ +get_filename_component(SELF_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH) +include(${SELF_DIR}/wpiannotations.cmake) diff --git a/wpilibNewCommands/BUILD.bazel b/wpilibNewCommands/BUILD.bazel index 6add11137a..fc9326baf2 100644 --- a/wpilibNewCommands/BUILD.bazel +++ b/wpilibNewCommands/BUILD.bazel @@ -37,11 +37,14 @@ cc_library( java_library( name = "wpilibNewCommands-java", srcs = glob(["src/main/java/**/*.java"]) + [":generated_java"], + exported_plugins = ["//javacPlugin:plugin"], + plugins = ["//javacPlugin:plugin"], visibility = ["//visibility:public"], deps = [ "//cscore:cscore-java", "//hal:hal-java", "//ntcore:networktables-java", + "//wpiannotations", "//wpilibj", "//wpimath:wpimath-java", "//wpinet:wpinet-java", @@ -79,8 +82,10 @@ java_binary( srcs = ["src/dev/java/edu/wpi/first/wpilibj2/commands/DevMain.java"], main_class = "edu.wpi.first.wpilibj2.commands.DevMain", deps = [ + ":wpilibNewCommands-java", "//hal:hal-java", "//ntcore:networktables-java", + "//wpiannotations", "//wpimath:wpimath-java", "//wpiutil:wpiutil-java", ], diff --git a/wpilibNewCommands/CMakeLists.txt b/wpilibNewCommands/CMakeLists.txt index f7c664802e..f7959bff80 100644 --- a/wpilibNewCommands/CMakeLists.txt +++ b/wpilibNewCommands/CMakeLists.txt @@ -22,6 +22,7 @@ if(WITH_JAVA) wpiunits_jar wpiutil_jar wpilibj_jar + wpiannotations_jar OUTPUT_NAME wpilibNewCommands OUTPUT_DIR ${WPILIB_BINARY_DIR}/${java_lib_dest} ) diff --git a/wpilibNewCommands/build.gradle b/wpilibNewCommands/build.gradle index 884c18b65c..fb3cf2dbb3 100644 --- a/wpilibNewCommands/build.gradle +++ b/wpilibNewCommands/build.gradle @@ -21,7 +21,9 @@ dependencies { implementation project(':hal') implementation project(':wpimath') implementation project(':wpilibj') + implementation project(':wpiannotations') testImplementation 'org.mockito:mockito-core:4.1.0' + annotationProcessor project(':javacPlugin') } sourceSets.main.java.srcDir "${projectDir}/src/generated/main/java" diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java index d46ce313c6..68944440e0 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Set; import java.util.function.BooleanSupplier; +import org.wpilib.annotation.NoDiscard; /** * A state machine representing a complete action to be performed by the robot. Commands are run by @@ -27,6 +28,7 @@ import java.util.function.BooleanSupplier; * *

This class is provided by the NewCommands VendorDep */ +@NoDiscard("Commands must be used! Did you mean to bind it to a trigger?") public abstract class Command implements Sendable { /** Requirements set. */ private final Set m_requirements = new HashSet<>();