From b568f51aa73923c1f17ccbddafcaba95e151e1de Mon Sep 17 00:00:00 2001 From: Harsh Mehta Date: Fri, 12 Jun 2026 02:52:22 -0700 Subject: [PATCH] Fix NPE in `VarWithPrimitive` when lambda parameters have inferred primitive types ## Problem Fixes #5857 `VarWithPrimitive` throws a `NullPointerException` when a lambda has an implicit parameter whose inferred type is a primitive: ```java byte[] bar = new byte[6]; Map> indicesMap = IntStream.range(0, 6) .mapToObj(n -> n) .collect(Collectors.groupingBy( n -> bar[n], Collectors.mapping( n -> (byte) n.intValue(), Collectors.toList()))); // Exception: java.lang.NullPointerException: Cannot invoke "com.sun.tools.javac.tree.JCTree.getStartPosition()" because "tree" is null ``` ## Root Cause `hasImplicitType()` returns `true` for both: - `var` declarations - Implicit lambda parameters (when `VariableTree.getType()` is `null`, per JDK-8268850) `VarWithPrimitive` used `hasImplicitType()` as its guard, so lambda parameters with inferred primitive types passed the check and reached `replaceVariableType()`. Inside `replaceVariableType()`, `tree.getType()` returns `null` for these parameters. `hasExplicitSource(null, state)` was then called unconditionally, which invokes: ```java getStartPosition(null) ``` resulting in a `NullPointerException`. ## Fix Add a null guard for `type` in `SuggestedFixes.replaceVariableType()` at the actual crash site. If `type == null`: - Skip the `hasExplicitSource()` call. - Fall through to `getStartPosition(tree)`. The subsequent token scan finds no `var` keyword for implicit lambda parameters and correctly returns `Optional.empty()`, producing `NO_MATCH` without throwing an exception. This is the minimal fix because: - It addresses the actual crash site. - It is defensive against any other callers that may pass a `VariableTree` with a null type. ## Tests ### `implicitLambdaParameter_noMatch` Regression test reproducing the exact code from the bug report: - Single-parameter lambdas - Inferred `int` and `byte` types - Verifies `NO_MATCH` and no exception ### `multiParamImplicitLambda_noMatch` Tests a multi-parameter implicit lambda: ```java IntBinaryOperator op = (a, b) -> a + b; ``` - Both parameters inferred as `int` - Verifies `NO_MATCH` and no exception ### `explicitLambdaParam_noMatch` Tests an explicit lambda parameter: ```java (int n) -> n ``` - `hasImplicitType()` returns `false` - Verifies no match is reported ### `enhancedForLoop` Tests: ```java for (var x : intArray) { // ... } ``` Expected fix: ```java for (int x : intArray) { // ... } ``` ### `forLoopInitializer` Tests: ```java for (var i = 0; i < 10; i++) { // ... } ``` Expected fix: ```java for (int i = 0; i < 10; i++) { // ... } ``` Fixes #5868 FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/error-prone/pull/5868 from HarshMehta112:master 58810695d420a1ccc867867dc643797e5a87d192 PiperOrigin-RevId: 931039338 --- .../errorprone/fixes/SuggestedFixes.java | 4 +- .../bugpatterns/VarWithPrimitiveTest.java | 229 ++++++++++++++++++ 2 files changed, 231 insertions(+), 2 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java index 7c384c113a3..5fa79321507 100644 --- a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java +++ b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java @@ -1863,10 +1863,10 @@ public static String castTree(ExpressionTree expressionTree, String toType, Visi public static Optional replaceVariableType( VariableTree tree, String replacementType, VisitorState state) { Tree type = tree.getType(); - if (hasExplicitSource(type, state)) { + if (type != null && hasExplicitSource(type, state)) { return Optional.of(SuggestedFix.replace(type, replacementType)); } - int pos = getStartPosition(type); + int pos = type != null ? getStartPosition(type) : Position.NOPOS; if (pos == Position.NOPOS) { pos = getStartPosition(tree); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/VarWithPrimitiveTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/VarWithPrimitiveTest.java index 1eab280f126..280e82f26b8 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/VarWithPrimitiveTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/VarWithPrimitiveTest.java @@ -16,7 +16,10 @@ package com.google.errorprone.bugpatterns; +import static com.google.common.truth.TruthJUnit.assume; + import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.CompilationTestHelper; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -27,6 +30,9 @@ public final class VarWithPrimitiveTest { private final BugCheckerRefactoringTestHelper refactoringHelper = BugCheckerRefactoringTestHelper.newInstance(VarWithPrimitive.class, getClass()); + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(VarWithPrimitive.class, getClass()); + // from https://openjdk.org/projects/amber/guides/lvti-style-guide#G7 @Test public void lvtiExamples() { @@ -194,6 +200,229 @@ long getAgeAsLong() { .doTest(); } + @Test + public void enhancedForLoop() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + void t() { + int[] arr = {1, 2, 3}; + for (var x : arr) { + System.out.println(x); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + void t() { + int[] arr = {1, 2, 3}; + for (int x : arr) { + System.out.println(x); + } + } + } + """) + .doTest(); + } + + @Test + public void forLoopInitializer() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + void t() { + for (var i = 0; i < 10; i++) { + System.out.println(i); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + void t() { + for (int i = 0; i < 10; i++) { + System.out.println(i); + } + } + } + """) + .doTest(); + } + + @Test + public void implicitLambdaParameter_noMatch() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.List; + import java.util.Map; + import java.util.stream.Collectors; + import java.util.stream.IntStream; + class Test { + void foo() { + byte[] bar = new byte[6]; + Map> indicesMap = + IntStream.range(0, 6) + .mapToObj(n -> n) + .collect( + Collectors.groupingBy( + n -> bar[n], + Collectors.mapping( + n -> (byte) n.intValue(), Collectors.toList()))); + } + } + """) + .expectUnchanged() + .doTest(); + } + + @Test + public void varLambdaParam() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.IntUnaryOperator; + class Test { + void t() { + IntUnaryOperator op = (var n) -> n * 2; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.IntUnaryOperator; + class Test { + void t() { + IntUnaryOperator op = (int n) -> n * 2; + } + } + """) + .doTest(); + } + + @Test + public void annotatedVarLambdaParam() { + refactoringHelper + .addInputLines("A.java", "@interface A { int var() default 0; }") + .expectUnchanged() + .addInputLines( + "Test.java", + """ + import java.util.function.IntUnaryOperator; + class Test { + void t() { + IntUnaryOperator op = (@A(var = 0) var n) -> n * 2; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.IntUnaryOperator; + class Test { + void t() { + IntUnaryOperator op = (@A(var = 0) int n) -> n * 2; + } + } + """) + .doTest(); + } + + @Test + public void annotatedVarLocalVariable_beforeJdk27() { + // Before JDK 27 there is no AST node for the var type, so the token scan finds two 'var' + // tokens (@A(var=0) attribute + type keyword) and safely returns no fix. + assume().that(Runtime.version().feature()).isLessThan(27); + refactoringHelper + .addInputLines("A.java", "@interface A { int var() default 0; }") + .expectUnchanged() + .addInputLines( + "Test.java", + """ + class Test { + void t() { + @A(var = 0) var n = 5; + } + } + """) + .expectUnchanged() + .doTest(); + } + + @Test + public void annotatedVarLocalVariable_jdk27AndAbove() { + // On JDK 27+ there is an AST node for the var type with source positions, so hasExplicitSource + // returns true and the replacement targets the type keyword directly, preserving the + // annotation. + assume().that(Runtime.version().feature()).isAtLeast(27); + refactoringHelper + .addInputLines("A.java", "@interface A { int var() default 0; }") + .expectUnchanged() + .addInputLines( + "Test.java", + """ + class Test { + void t() { + @A(var = 0) var n = 5; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + void t() { + @A(var = 0) int n = 5; + } + } + """) + .doTest(); + } + + @Test + public void multiParamImplicitLambda_noMatch() { + compilationHelper + .addSourceLines( + "Test.java", + """ + import java.util.function.IntBinaryOperator; + class Test { + void t() { + IntBinaryOperator op = (a, b) -> a + b; + } + } + """) + .doTest(); + } + + @Test + public void explicitLambdaParam_noMatch() { + compilationHelper + .addSourceLines( + "Test.java", + """ + import java.util.function.IntUnaryOperator; + class Test { + void t() { + IntUnaryOperator op = (int n) -> n * 2; + } + } + """) + .doTest(); + } + @Test public void explicitType() { refactoringHelper