From fcaf39e0ad0e8bc386d33afdf2c773d1c9015086 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 5 Sep 2023 12:17:05 -0700 Subject: [PATCH] Treat parameter of generated Record.equals() methods as @Nullable (#825) Fixes #619 --- .../nullaway/jdk17/NullAwayRecordTests.java | 32 +++++++++++++++++++ .../main/java/com/uber/nullaway/Nullness.java | 28 ++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java b/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java index 8135deb27b..d60e264a77 100644 --- a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java +++ b/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java @@ -178,4 +178,36 @@ public void testLocalRecord() { "}") .doTest(); } + + @Test + public void recordEqualsNull() { + defaultCompilationHelper + .addSourceLines( + "Records.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "class Records {", + " public void recordEqualsNull() {", + " record Rec() {", + " void print(Object o) { System.out.println(o.toString()); }", + " void equals(Integer i1, Integer i2) { }", + " boolean equals(String i1, String i2) { return false; }", + " boolean equals(Long l1) { return false; }", + " }", + " Object o = null;", + " // null can be passed to the generated equals() method taking an Object parameter", + " new Rec().equals(o);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", + " new Rec().print(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", + " new Rec().equals(null, Integer.valueOf(100));", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", + " new Rec().equals(\"hello\", null);", + " Long l = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'l'", + " new Rec().equals(l);", + " }", + "}") + .doTest(); + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 1e96a7abd6..c1b8198de0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -19,6 +19,8 @@ package com.uber.nullaway; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.util.List; import java.util.stream.Stream; import javax.lang.model.element.AnnotationMirror; import org.checkerframework.nullaway.dataflow.analysis.AbstractValue; @@ -210,10 +212,36 @@ public static boolean hasNullableAnnotation(Symbol symbol, Config config) { */ public static boolean paramHasNullableAnnotation( Symbol.MethodSymbol symbol, int paramInd, Config config) { + // We treat the (generated) equals() method of record types to have a @Nullable parameter, as + // the generated implementation handles null (as required by the contract of Object.equals()) + if (isRecordEqualsParam(symbol, paramInd)) { + return true; + } return hasNullableAnnotation( NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd), config); } + private static boolean isRecordEqualsParam(Symbol.MethodSymbol symbol, int paramInd) { + // Here we compare with toString() to preserve compatibility with JDK 11 (records only + // introduced in JDK 16) + if (!symbol.owner.getKind().toString().equals("RECORD")) { + return false; + } + if (!symbol.getSimpleName().contentEquals("equals")) { + return false; + } + // Check for a boolean return type and a single parameter of type java.lang.Object + Type type = symbol.type; + List parameterTypes = type.getParameterTypes(); + if (!(type.getReturnType().toString().equals("boolean") + && parameterTypes != null + && parameterTypes.size() == 1 + && parameterTypes.get(0).toString().equals("java.lang.Object"))) { + return false; + } + return paramInd == 0; + } + /** * Does the parameter of {@code symbol} at {@code paramInd} have a {@code @NonNull} declaration or * type-use annotation? This method works for methods defined in either source or class files.