diff --git a/developerRobot/build.gradle b/developerRobot/build.gradle index da12ffe0d2..41be11dfcf 100644 --- a/developerRobot/build.gradle +++ b/developerRobot/build.gradle @@ -58,6 +58,7 @@ dependencies { implementation project(':wpiunits') implementation project(':epilogue-runtime') annotationProcessor project(':epilogue-processor') + annotationProcessor project(':javacPlugin') } tasks.withType(JavaExec).configureEach { diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/IntegerDivisionDetector.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/IntegerDivisionDetector.java new file mode 100644 index 0000000000..17c4937f94 --- /dev/null +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/IntegerDivisionDetector.java @@ -0,0 +1,163 @@ +// 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.AssignmentTree; +import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ParenthesizedTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.TypeCastTree; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.TaskEvent; +import com.sun.source.util.TaskListener; +import com.sun.source.util.TreePath; +import com.sun.source.util.TreeScanner; +import com.sun.source.util.Trees; +import java.util.HashSet; +import java.util.Set; +import javax.lang.model.type.TypeMirror; +import javax.tools.Diagnostic; + +/** + * Detects integer division operations in Java source code and marks them as errors. + * + *
{@code
+ * double x = 1 / 2; // -> error: integer division for a floating-point number
+ * double x = (double) 1 / 2; // -> ok (casting an operand to a double)
+ * double x = (double) (1 / 2); // -> error: integer division for a floating-point number
+ * int x = 1 / 2; // -> ok (in an integer context)
+ * }
+ */ +public class IntegerDivisionDetector implements TaskListener { + private final JavacTask m_task; + private final Set m_visitedCUs = new HashSet<>(); + + public IntegerDivisionDetector(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 visitBinary(BinaryTree node, Void unused) { + if (node.getKind() != Kind.DIVIDE) { + return super.visitBinary(node, unused); + } + + TypeMirror type = m_trees.getTypeMirror(m_trees.getPath(m_root, node)); + if (!isIntegerType(type)) { + return super.visitBinary(node, unused); + } + + // It's integer division. Now check the context. + if (isFloatContext(m_trees.getPath(m_root, node))) { + if (Suppressions.hasSuppression( + m_trees, m_trees.getPath(m_root, node), "IntegerDivision")) { + return super.visitBinary(node, unused); + } + m_trees.printMessage( + Diagnostic.Kind.ERROR, "integer division in a floating-point context", node, m_root); + } + + return super.visitBinary(node, unused); + } + + private boolean isIntegerType(TypeMirror type) { + if (type == null) { + return false; + } + + return switch (type.getKind()) { + case BYTE, CHAR, SHORT, INT, LONG -> true; + default -> false; + }; + } + + private boolean isFloatType(TypeMirror type) { + if (type == null) { + return false; + } + + return switch (type.getKind()) { + case FLOAT, DOUBLE -> true; + default -> false; + }; + } + + private boolean isFloatContext(TreePath path) { + TreePath parentPath = path.getParentPath(); + while (parentPath != null && parentPath.getLeaf() instanceof ParenthesizedTree) { + parentPath = parentPath.getParentPath(); + } + if (parentPath == null) { + return false; + } + Tree parent = parentPath.getLeaf(); + + switch (parent) { + case AssignmentTree assignment -> { + if (getExpression(assignment.getExpression()) == path.getLeaf()) { + return isFloatType( + m_trees.getTypeMirror(m_trees.getPath(m_root, assignment.getVariable()))); + } + } + case VariableTree variable -> { + if (variable.getInitializer() != null + && getExpression(variable.getInitializer()) == path.getLeaf()) { + return isFloatType(m_trees.getTypeMirror(m_trees.getPath(m_root, variable.getType()))); + } + } + case ReturnTree _ -> { + TreePath p = parentPath; + while (p != null && !(p.getLeaf() instanceof MethodTree)) { + p = p.getParentPath(); + } + if (p != null && p.getLeaf() instanceof MethodTree method) { + return isFloatType( + m_trees.getTypeMirror(m_trees.getPath(m_root, method.getReturnType()))); + } + } + case TypeCastTree cast -> { + return isFloatType(m_trees.getTypeMirror(m_trees.getPath(m_root, cast.getType()))); + } + case null, default -> { + return false; + } + } + return false; + } + + private Tree getExpression(ExpressionTree tree) { + while (tree instanceof ParenthesizedTree p) { + tree = p.getExpression(); + } + return tree; + } + } +} diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java index 52a93a08eb..6d31f956ba 100644 --- a/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java @@ -26,6 +26,7 @@ public class WPILibJavacPlugin implements Plugin { task.addTaskListener(new CoroutineYieldInLoopDetector(task)); task.addTaskListener(new CodeAfterCoroutineParkDetector(task)); task.addTaskListener(new IncorrectCoroutineUseDetector(task)); + task.addTaskListener(new IntegerDivisionDetector(task)); } @Override diff --git a/javacPlugin/src/test/java/org/wpilib/javacplugin/IntegerDivisionDetectorTest.java b/javacPlugin/src/test/java/org/wpilib/javacplugin/IntegerDivisionDetectorTest.java new file mode 100644 index 0000000000..b5fde2ce3c --- /dev/null +++ b/javacPlugin/src/test/java/org/wpilib/javacplugin/IntegerDivisionDetectorTest.java @@ -0,0 +1,243 @@ +// 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 com.google.testing.compile.JavaFileObjects; +import org.junit.jupiter.api.Test; + +class IntegerDivisionDetectorTest { + private static final String[] types = {"byte", "char", "short", "int", "long", "float", "double"}; + + private static boolean isInteger(int t) { + return t <= 4; + } + + private static boolean isFloat(int t) { + return t >= 5; + } + + private static int promote(int t1, int t2) { + if (t1 == 6 || t2 == 6) { + return 6; + } + if (t1 == 5 || t2 == 5) { + return 5; + } + if (t1 == 4 || t2 == 4) { + return 4; + } + return 3; // int + } + + private static boolean canAssign(int from, int to) { + if (from == to) { + return true; + } + // byte -> short, int, long, float, double + if (from == 0) { + return to == 2 || to == 3 || to == 4 || to == 5 || to == 6; + } + // char -> int, long, float, double + if (from == 1) { + return to == 3 || to == 4 || to == 5 || to == 6; + } + // short -> int, long, float, double + if (from == 2) { + return to == 3 || to == 4 || to == 5 || to == 6; + } + // int -> long, float, double + if (from == 3) { + return to == 4 || to == 5 || to == 6; + } + // long -> float, double + if (from == 4) { + return to == 5 || to == 6; + } + // float -> double + return from == 5 && to == 6; + } + + @Test + void comprehensiveTest() { + StringBuilder code = new StringBuilder(512); + code.append("package wpilib.robot;\n\nclass Example {\n"); + + // Define variables for each type + for (int i = 0; i < types.length; i++) { + code.append(" ").append(types[i]).append(" v").append(i).append(" = 1;\n"); + } + + int expectedErrors = 0; + int testCount = 0; + for (int t1 = 0; t1 < 7; t1++) { + for (int t2 = 0; t2 < 7; t2++) { + int res = promote(t1, t2); + for (int t3 = 0; t3 < 7; t3++) { + if (canAssign(res, t3)) { + // Variation A: T3 v = (T1) a / (T2) b; + code.append(" ") + .append(types[t3]) + .append(" varA_") + .append(testCount) + .append(" = (") + .append(types[t1]) + .append(") v") + .append(t1) + .append(" / (") + .append(types[t2]) + .append(") v") + .append(t2) + .append(";\n "); + if (isInteger(t1) && isInteger(t2) && isFloat(t3)) { + expectedErrors++; + } + testCount++; + + // Variation B: T3 v = (T3) ((T1) a / (T2) b); + code.append(types[t3]) + .append(" varB_") + .append(testCount) + .append(" = (") + .append(types[t3]) + .append(") ((") + .append(types[t1]) + .append(") v") + .append(t1) + .append(" / (") + .append(types[t2]) + .append(") v") + .append(t2) + .append(");\n "); + if (isInteger(t1) && isInteger(t2) && isFloat(t3)) { + expectedErrors++; + } + testCount++; + + // Variation C: T3 v = ((T3) (T1) a) / (T2) b; + code.append(types[t3]) + .append(" varC_") + .append(testCount) + .append(" = ((") + .append(types[t3]) + .append(") (") + .append(types[t1]) + .append(") v") + .append(t1) + .append(") / (") + .append(types[t2]) + .append(") v") + .append(t2) + .append(";\n"); + // Variation C should never fail + testCount++; + } + } + } + } + code.append("}\n"); + + final int finalExpectedErrors = expectedErrors; + final String finalSource = code.toString(); + var compilation = + javac() + .withOptions(CompileTestUtils.kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("wpilib.robot.Example", finalSource)); + + var errors = compilation.errors(); + assertEquals( + finalExpectedErrors, + errors.size(), + () -> + "Expected " + + finalExpectedErrors + + " errors, but got " + + errors.size() + + ". Source:\n" + + finalSource); + + for (var error : errors) { + assertEquals("integer division in a floating-point context", error.getMessage(null)); + } + } + + @Test + void integerDivisionForIntegerContext() { + String source = + """ + package wpilib.robot; + + class Example { + int i = 1 / 2; + } + """; + + var compilation = + javac() + .withOptions(CompileTestUtils.kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("wpilib.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void integerDivisionForFloatContext() { + String source = + """ + package wpilib.robot; + + class Example { + float f = 1 / 2; + } + """; + + var compilation = + javac() + .withOptions(CompileTestUtils.kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("wpilib.robot.Example", source)); + + assertThat(compilation).failed(); + var errors = compilation.errors(); + assertEquals(1, errors.size()); + var error = errors.get(0); + assertEquals("integer division in a floating-point context", error.getMessage(null)); + } + + @Test + void suppressWarnings() { + String source = + """ + package wpilib.robot; + + class Example { + @SuppressWarnings("IntegerDivision") + float f = 1 / 2; + + void method() { + @SuppressWarnings("IntegerDivision") + double d = 3 / 4; + + @SuppressWarnings("all") + float f2 = 5 / 6; + } + + @SuppressWarnings("IntegerDivision") + double method2() { + return 7 / 8; + } + } + """; + + var compilation = + javac() + .withOptions(CompileTestUtils.kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("wpilib.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } +}