mirror of
https://github.com/wpilibsuite/allwpilib
synced 2026-06-19 00:41:43 +00:00
[javac] Add compiler check for integer division in floating-point contexts (#8885)
Useful for catching bad behavior when doing math or configuring ratios
with integers. Errors can be turned off with
`@SuppressWarnings("IntegerDivision")`
```java
double kV = 12 / 6000; // error: integer division in a floating-point context
double kV = 12. / 6000; // OK
double kV = 12 / 6000.; // OK
int kV = 12 / 6000; // OK, but evaluates to 0
double ratio = 42 / 12; // error: integer division in a floating-point context
@SuppressWarnings("IntegerDivision")
double ratio = 42 / 12; // OK, evaluates to 2
```
```
Execution failed for task ':developerRobot:compileJava'.
> Compilation failed; see the compiler output below.
/Users/sam/code/wpilib/allwpilib/developerRobot/src/main/java/wpilib/robot/Robot.java:10: error: integer division in a floating-point context
double kV = 12 / 6000;
^
1 error
```
Also adds the compiler plugin as a dependency to the developerRobot
project for dev testing
This commit is contained in:
@@ -58,6 +58,7 @@ dependencies {
|
|||||||
implementation project(':wpiunits')
|
implementation project(':wpiunits')
|
||||||
implementation project(':epilogue-runtime')
|
implementation project(':epilogue-runtime')
|
||||||
annotationProcessor project(':epilogue-processor')
|
annotationProcessor project(':epilogue-processor')
|
||||||
|
annotationProcessor project(':javacPlugin')
|
||||||
}
|
}
|
||||||
|
|
||||||
tasks.withType(JavaExec).configureEach {
|
tasks.withType(JavaExec).configureEach {
|
||||||
|
|||||||
@@ -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.
|
||||||
|
*
|
||||||
|
* <pre>{@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)
|
||||||
|
* }</pre>
|
||||||
|
*/
|
||||||
|
public class IntegerDivisionDetector implements TaskListener {
|
||||||
|
private final JavacTask m_task;
|
||||||
|
private final Set<CompilationUnitTree> 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<Void, Void> {
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -26,6 +26,7 @@ public class WPILibJavacPlugin implements Plugin {
|
|||||||
task.addTaskListener(new CoroutineYieldInLoopDetector(task));
|
task.addTaskListener(new CoroutineYieldInLoopDetector(task));
|
||||||
task.addTaskListener(new CodeAfterCoroutineParkDetector(task));
|
task.addTaskListener(new CodeAfterCoroutineParkDetector(task));
|
||||||
task.addTaskListener(new IncorrectCoroutineUseDetector(task));
|
task.addTaskListener(new IncorrectCoroutineUseDetector(task));
|
||||||
|
task.addTaskListener(new IntegerDivisionDetector(task));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
@@ -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();
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user