mirror of
https://github.com/wpilibsuite/allwpilib
synced 2026-06-23 01:21:42 +00:00
[javac plugin] Add compile-time checks for unsafe or incorrect coroutine usage (#8289)
CoroutineYieldInLoopDetector
This checks for while loops where coroutines are in scope but without calling a blocking method on at least one of those coroutines:
```
drivetrain.run(theCoroutine -> {
while (drivetrain.getDistance() < 10) { // ERROR: "Missing call to `theCoroutine.yield()` inside loop"
drivetrain.setSpeed(1);
}
});
```
Note that, because we assume most looping constructs in commands will use while loops, we don't check for-loops, for-each loops, or do-while loops.
This check can be disabled with `@SuppressWarnings("CoroutineYieldInLoop")`
CodeAfterCoroutineParkDetector
Essentially acts like the Java compiler's check for code after a while (true) loop, but for coroutine.park():
```
drivetrain.run(theCoroutine -> {
drivetrain.setSpeed(1.0);
theCoroutine.park();
drivetrain.setSpeed(0.0); // ERROR: "Unreachable statement: `theCoroutine.park()` will never exit"
});
```
This check can be disabled with `@SuppressWarnings("CodeAfterCoroutinePark")`
IncorrectCoroutineUseDetector
Checks for usage of captured (outer) coroutine parameters and assignments to fields.
```
drivetrain.run(outer -> {
outer.await(arm.run(inner -> {
outer.yield(); // ERROR: "Coroutine `outer` may not be in scope. Consider using `inner`"
}))
});
```
This check can be disabled with `@SuppressWarnings("CoroutineMayNotBeInScope")`
```
private Coroutine coroutineField;
drivetrain.run(co -> coroutineField = co); // ERROR: "Captured coroutines may not be stored in fields"
```
This check can be disabled with `@SuppressWarnings("CoroutineCapture")`
This commit is contained in:
Sam Carlberg
@@ -0,0 +1,240 @@
|
||||
// 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.CompileTestUtils.getErrorSource;
|
||||
import static org.wpilib.javacplugin.CompileTestUtils.kJavaVersionOptions;
|
||||
|
||||
import com.google.testing.compile.Compilation;
|
||||
import com.google.testing.compile.JavaFileObjects;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
class CodeAfterCoroutineParkDetectorTest {
|
||||
private static final String kCoroutineSource =
|
||||
"""
|
||||
package org.wpilib.commands3;
|
||||
|
||||
public interface Coroutine {
|
||||
void park();
|
||||
}
|
||||
""";
|
||||
|
||||
@Test
|
||||
void soloPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void printAfterPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();
|
||||
System.out.println("Unreachable 1"); // this line should get an error
|
||||
System.out.println("Unreachable 2"); // but not this one
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Unreachable statement: `coroutine.park()` will never exit", error.getMessage(null));
|
||||
assertEquals("System.out.println(\"Unreachable 1\"); ", getErrorSource(error));
|
||||
}
|
||||
|
||||
@Test
|
||||
void printAfterParkInBlock() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
{
|
||||
// Only .park() calls at the top level are detected
|
||||
coroutine.park();
|
||||
}
|
||||
System.out.println("Unreachable 1");
|
||||
System.out.println("Unreachable 2");
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void loopAfterPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();
|
||||
for (;;) {
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Unreachable statement: `coroutine.park()` will never exit", error.getMessage(null));
|
||||
assertEquals("for (;;) {\n }\n", getErrorSource(error));
|
||||
}
|
||||
|
||||
@Test
|
||||
void blockAfterPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();
|
||||
{
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Unreachable statement: `coroutine.park()` will never exit", error.getMessage(null));
|
||||
assertEquals("{\n }\n", getErrorSource(error));
|
||||
}
|
||||
|
||||
@Test
|
||||
void emptyStatementAfterPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();;
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void printAfterEmptyStatementAfterPark() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutine.park();;
|
||||
System.out.println("Unreachable");
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Unreachable statement: `coroutine.park()` will never exit", error.getMessage(null));
|
||||
assertEquals("System.out.println(\"Unreachable\");\n", getErrorSource(error));
|
||||
}
|
||||
}
|
||||
@@ -1,13 +0,0 @@
|
||||
// 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);
|
||||
}
|
||||
@@ -0,0 +1,48 @@
|
||||
// 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.io.IOException;
|
||||
import java.util.List;
|
||||
import javax.tools.Diagnostic;
|
||||
import javax.tools.JavaFileObject;
|
||||
|
||||
public final class CompileTestUtils {
|
||||
public static final int kJavaVersion = 17;
|
||||
public static final List<Object> kJavaVersionOptions =
|
||||
List.of("-source", kJavaVersion, "-target", kJavaVersion);
|
||||
|
||||
private CompileTestUtils() {
|
||||
// Utility class
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the source code for a given compiler error.
|
||||
*
|
||||
* @param diagnostic The diagnostic to get the source for.
|
||||
* @return The source code for the given diagnostic.
|
||||
*/
|
||||
public static String getErrorSource(Diagnostic<? extends JavaFileObject> diagnostic) {
|
||||
try (var reader = diagnostic.getSource().openReader(true)) {
|
||||
int sourceLength = (int) (diagnostic.getEndPosition() - diagnostic.getStartPosition() + 1);
|
||||
char[] buf = new char[sourceLength];
|
||||
long skipCnt = reader.skip(diagnostic.getStartPosition());
|
||||
if (skipCnt != diagnostic.getStartPosition()) {
|
||||
// Didn't skip to the expected position; bail
|
||||
return "<unknown source>";
|
||||
}
|
||||
|
||||
int readCnt = reader.read(buf);
|
||||
if (readCnt != sourceLength) {
|
||||
// Didn't read the expected length of text; bail
|
||||
return "<unknown source>";
|
||||
}
|
||||
|
||||
return new String(buf);
|
||||
} catch (IOException e) {
|
||||
return "<unknown source>";
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,456 @@
|
||||
// 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.CompileTestUtils.kJavaVersionOptions;
|
||||
|
||||
import com.google.testing.compile.Compilation;
|
||||
import com.google.testing.compile.JavaFileObjects;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
class CoroutineInLoopListenerTest {
|
||||
private static final String kCoroutineSource =
|
||||
"""
|
||||
package org.wpilib.commands3;
|
||||
|
||||
public interface Coroutine {
|
||||
void yield();
|
||||
}
|
||||
""";
|
||||
|
||||
@Test
|
||||
void noYieldInLoopWithoutCoroutines() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
class Example {
|
||||
Runnable lambda = () -> {
|
||||
while (true) {
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void basicYieldInLoopInLambda() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (true) {
|
||||
coroutine.yield();
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void basicYieldInLoopInMethod() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
void useCoroutine(Coroutine coroutine) {
|
||||
while (true) {
|
||||
coroutine.yield();
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldInLoopInLambda() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (true) {
|
||||
// No yield
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void yieldInLoopInRunnableInLambda() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
Runnable r = () -> {
|
||||
while (true) {
|
||||
coroutine.yield();
|
||||
}
|
||||
};
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
// TODO: Should we make it an error to yield (or invoke any methods on) a captured coroutine?
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldInMethodWithManyCoroutineParams() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
void manyCoroutineParams(
|
||||
Coroutine firstCoroutine,
|
||||
Coroutine c1,
|
||||
Coroutine next,
|
||||
Coroutine thisMightBeTheLastOne) {
|
||||
while (true) {
|
||||
// No yield
|
||||
}
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Missing call to "
|
||||
+ "`firstCoroutine.yield()`, "
|
||||
+ "`c1.yield()`, "
|
||||
+ "`next.yield()`, or "
|
||||
+ "`thisMightBeTheLastOne.yield()` inside loop",
|
||||
error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldsInNestedLambda() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
Consumer<Coroutine> lambda2 = innerCoroutine -> {
|
||||
while (true) {
|
||||
// No yield
|
||||
}
|
||||
};
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `innerCoroutine.yield()` inside loop", error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void nestedLambdaYieldsToOuter() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
Consumer<Coroutine> lambda2 = innerCoroutine -> {
|
||||
while (true) {
|
||||
outerCoroutine.yield();
|
||||
}
|
||||
};
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
|
||||
// One error for not calling a yielding function on the most local coroutine object,
|
||||
// and another error for calling a method on a captured coroutine (from a different analyzer)
|
||||
assertEquals(2, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `innerCoroutine.yield()` inside loop", error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldsInNestedLoops() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (0 == 0) {
|
||||
while (1 == 1) {
|
||||
// inner - no yield
|
||||
}
|
||||
// outer - also no yield
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(2, compilation.errors().size());
|
||||
|
||||
var error1 = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error1.getMessage(null));
|
||||
assertEquals(8, error1.getLineNumber());
|
||||
|
||||
var error2 = compilation.errors().get(1);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error2.getMessage(null));
|
||||
assertEquals(9, error2.getLineNumber());
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldInOuterLoopButYieldInInnerLoop() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (true) {
|
||||
coroutine.yield();
|
||||
while (true) {
|
||||
// No yields
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error.getMessage(null));
|
||||
assertEquals(10, error.getLineNumber());
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldInInnerLoopButYieldInOuterLoop() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (true) {
|
||||
while (true) {
|
||||
coroutine.yield();
|
||||
}
|
||||
// No yields
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error.getMessage(null));
|
||||
assertEquals(8, error.getLineNumber());
|
||||
}
|
||||
|
||||
@Test
|
||||
void noYieldsInDeeplyNestedLoops() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import java.util.function.Consumer;
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
while (0 == 0) {
|
||||
while (1 == 1) {
|
||||
while (2 == 2) {
|
||||
while (3 == 3) {
|
||||
while (4 == 4) {
|
||||
while (5 == 5) {
|
||||
if (true) {
|
||||
// nested if statement shouldn't mess with error detection or reporting
|
||||
while (6 == 6) {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(7, compilation.errors().size());
|
||||
|
||||
var error1 = compilation.errors().get(0);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error1.getMessage(null));
|
||||
assertEquals(8, error1.getLineNumber());
|
||||
|
||||
var error2 = compilation.errors().get(1);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error2.getMessage(null));
|
||||
assertEquals(9, error2.getLineNumber());
|
||||
|
||||
var error3 = compilation.errors().get(2);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error3.getMessage(null));
|
||||
assertEquals(10, error3.getLineNumber());
|
||||
|
||||
var error4 = compilation.errors().get(3);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error4.getMessage(null));
|
||||
assertEquals(11, error4.getLineNumber());
|
||||
|
||||
var error5 = compilation.errors().get(4);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error5.getMessage(null));
|
||||
assertEquals(12, error5.getLineNumber());
|
||||
|
||||
var error6 = compilation.errors().get(5);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error6.getMessage(null));
|
||||
assertEquals(13, error6.getLineNumber());
|
||||
|
||||
var error7 = compilation.errors().get(6);
|
||||
assertEquals("Missing call to `coroutine.yield()` inside loop", error7.getMessage(null));
|
||||
assertEquals(16, error7.getLineNumber());
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,303 @@
|
||||
// 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.CompileTestUtils.getErrorSource;
|
||||
import static org.wpilib.javacplugin.CompileTestUtils.kJavaVersionOptions;
|
||||
|
||||
import com.google.testing.compile.Compilation;
|
||||
import com.google.testing.compile.JavaFileObjects;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
class IncorrectCoroutineUseDetectorTest {
|
||||
private static final String kCoroutineSource =
|
||||
"""
|
||||
package org.wpilib.commands3;
|
||||
|
||||
public interface Coroutine {
|
||||
void yield();
|
||||
}
|
||||
""";
|
||||
|
||||
@Test
|
||||
void methodCalledOnNonlocalCoroutine() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
Consumer<Coroutine> lambda2 = innerCoroutine -> {
|
||||
outerCoroutine.yield();
|
||||
};
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Coroutine `outerCoroutine` may not be in scope. Consider using `innerCoroutine`",
|
||||
error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void nonlocalCoroutinePassedToMethod() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
Consumer<Coroutine> lambda2 = innerCoroutine -> {
|
||||
method(outerCoroutine);
|
||||
};
|
||||
};
|
||||
|
||||
void method(Coroutine foo) { }
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Coroutine `outerCoroutine` may not be in scope. Consider using `innerCoroutine`",
|
||||
error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void twoLocalCoroutines() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.BiConsumer;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
BiConsumer<Coroutine, Coroutine> lambda2 = (a, b) -> {
|
||||
method(outerCoroutine);
|
||||
};
|
||||
};
|
||||
|
||||
void method(Coroutine foo) { }
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Coroutine `outerCoroutine` may not be in scope. Consider using `a` or `b`",
|
||||
error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void threeLocalCoroutines() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
@FunctionalInterface
|
||||
interface TriConsumer<T, U, V> {
|
||||
void accept(T t, U u, V v);
|
||||
}
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
TriConsumer<Coroutine, Coroutine, Coroutine> lambda2 = (a, b, c) -> {
|
||||
method(outerCoroutine);
|
||||
};
|
||||
};
|
||||
|
||||
void method(Coroutine foo) { }
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals(
|
||||
"Coroutine `outerCoroutine` may not be in scope. Consider using `a`, `b`, or `c`",
|
||||
error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void coroutineSavedToFieldErrors() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Coroutine coroutineField;
|
||||
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
coroutineField = coroutine;
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(1, compilation.errors().size());
|
||||
var error = compilation.errors().get(0);
|
||||
assertEquals("Captured coroutines may not be stored in fields", error.getMessage(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void outerCoroutineSavedToFieldErrors() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Coroutine coroutineField;
|
||||
|
||||
Consumer<Coroutine> lambda = outerCoroutine -> {
|
||||
Consumer<Coroutine> lambda2 = innerCoroutine -> {
|
||||
coroutineField = outerCoroutine;
|
||||
coroutineField = innerCoroutine;
|
||||
};
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).failed();
|
||||
assertEquals(2, compilation.errors().size());
|
||||
|
||||
var error1 = compilation.errors().get(0);
|
||||
assertEquals("Captured coroutines may not be stored in fields", error1.getMessage(null));
|
||||
assertEquals(11, error1.getLineNumber());
|
||||
assertEquals("coroutineField = outerCoroutine;", getErrorSource(error1));
|
||||
|
||||
var error2 = compilation.errors().get(1);
|
||||
assertEquals("Captured coroutines may not be stored in fields", error2.getMessage(null));
|
||||
assertEquals("coroutineField = innerCoroutine;", getErrorSource(error2));
|
||||
assertEquals(12, error2.getLineNumber());
|
||||
}
|
||||
|
||||
@Test
|
||||
void coroutineSavedToVariableIsAllowed() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
Coroutine local = coroutine;
|
||||
};
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
|
||||
// This test is really to make it clear that the compiler plugin can't feasibly check for
|
||||
// everything, rather than this being a usecase we actually want to support.
|
||||
@Test
|
||||
void coroutineSavedToFieldViaMethodCannotBeDetected() {
|
||||
String source =
|
||||
"""
|
||||
package frc.robot;
|
||||
|
||||
import org.wpilib.commands3.Coroutine;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
class Example {
|
||||
private Coroutine coroutineField;
|
||||
|
||||
Consumer<Coroutine> lambda = coroutine -> {
|
||||
saveCoroutine(coroutine);
|
||||
};
|
||||
|
||||
void saveCoroutine(Coroutine coroutine) {
|
||||
coroutineField = coroutine;
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
Compilation compilation =
|
||||
javac()
|
||||
.withOptions(kJavaVersionOptions)
|
||||
.compile(
|
||||
JavaFileObjects.forSourceString("org.wpilib.commands3.Coroutine", kCoroutineSource),
|
||||
JavaFileObjects.forSourceString("frc.robot.Example", source));
|
||||
|
||||
assertThat(compilation).succeededWithoutWarnings();
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,7 @@ 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 static org.wpilib.javacplugin.CompileTestUtils.kJavaVersionOptions;
|
||||
|
||||
import com.google.testing.compile.Compilation;
|
||||
import com.google.testing.compile.JavaFileObjects;
|
||||
|
||||
Reference in New Issue
Block a user