Add javac plugin for detecting common error cases at compile time (#8196)

Adds a `@NoDiscard` annotation that can be placed on methods to guarantee their return values are used and on types to guarantee that any method returning that type uses the return value. Methods that call `@NoDiscard`-annotated functions can add a `@SuppressWarnings("NoDiscard")` or `@SuppressWarnings("all")` annotation (or annotation on the class declaring that method) to silence the compiler error warnings.
2026-06-21 01:01:43 +00:00 · 2025-10-03 20:42:47 -04:00
parent 871769c815
commit 3972b01c51
19 changed files with 1039 additions and 0 deletions
--- a/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java
+++ 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<Object> kJavaVersionOptions =
+      List.of("-source", kJavaVersion, "-target", kJavaVersion);
+}
--- a/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java
+++ 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));
+  }
+}