diff --git a/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java b/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java index b913feccb94..77675601fd1 100644 --- a/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java +++ b/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java @@ -17,6 +17,7 @@ package com.google.errorprone.refaster; import static com.google.errorprone.util.ASTHelpers.stringContainsComments; +import static java.util.logging.Level.FINE; import com.google.auto.value.AutoValue; import com.google.errorprone.BugPattern.SeverityLevel; @@ -37,12 +38,16 @@ import com.sun.source.util.SimpleTreeVisitor; import com.sun.source.util.TreeScanner; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Types; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; +import com.sun.tools.javac.tree.JCTree.JCExpression; import com.sun.tools.javac.tree.JCTree.JCStatement; import com.sun.tools.javac.tree.TreeMaker; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.ListBuffer; +import java.util.logging.Logger; /** * Scanner that outputs suggested fixes generated by a {@code RefasterMatcher}. @@ -52,6 +57,7 @@ @AutoValue abstract class RefasterScanner> extends TreeScanner { + private static final Logger logger = Logger.getLogger(RefasterScanner.class.toString()); static > RefasterScanner create( RefasterRule rule, DescriptionListener listener) { return new AutoValue_RefasterScanner<>(rule, listener); @@ -124,6 +130,9 @@ public Void scan(Tree tree, Context context) { builder.addFix(SuggestedFix.prefixWith(match.getLocation(), "/* match found */ ")); } else { for (T afterTemplate : rule().afterTemplates()) { + if (!isAfterTypeCompatible(beforeTemplate, afterTemplate, match, context)) { + continue; + } builder.addFix(afterTemplate.replace(match)); } } @@ -180,6 +189,38 @@ public Void visitIf(IfTree node, Context context) { return null; } + /** + * Checks that the after template's return type is compatible with the before template's return + * type. This prevents applying replacements where the after template returns a wider type than + * the before template, which could break compilation at the match site. + */ + private static > boolean isAfterTypeCompatible( + T beforeTemplate, T afterTemplate, M match, Context context) { + if (!(beforeTemplate instanceof ExpressionTemplate beforeExprTemplate) + || !(afterTemplate instanceof ExpressionTemplate afterExprTemplate)) { + return true; + } + try { + Inliner inliner = match.createInliner(); + Type beforeReturnType = beforeExprTemplate.returnType().inline(inliner); + Type afterReturnType = afterExprTemplate.returnType().inline(inliner); + Types types = Types.instance(context); + if (!types.isSubtype(types.erasure(afterReturnType), types.erasure(beforeReturnType))) { + logger.log( + FINE, + String.format( + "Skipping match because after template return type %s is not a subtype of before" + + " template return type %s", + afterReturnType, beforeReturnType)); + return false; + } + } catch (CouldNotResolveImportException e) { + logger.log(FINE, "Failure to resolve import in template return type", e); + return false; + } + return true; + } + private boolean isSuppressed(Tree node, Context context) { return RefasterSuppressionHelper.suppressed(rule(), node, context); } diff --git a/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java b/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java index c323ed95f53..68ff273e3fe 100644 --- a/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java +++ b/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java @@ -382,4 +382,9 @@ public void memberSelectAndMethodParameterDisambiguation() throws IOException { public void unqualifiedMethod() throws IOException { runTest("UnqualifiedMethodTemplate"); } + + @Test + public void widerType() throws IOException { + runTest("WiderTypeTemplate"); + } } diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/input/WiderTypeTemplateExample.java b/core/src/test/java/com/google/errorprone/refaster/testdata/input/WiderTypeTemplateExample.java new file mode 100644 index 00000000000..d0c6e2f8567 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/input/WiderTypeTemplateExample.java @@ -0,0 +1,29 @@ +/* + * Copyright 2025 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.refaster.testdata; + +/** + * Example input for {@code WiderTypeTemplate}. + */ +public class WiderTypeTemplateExample { + public void example(Object obj) { + // positive example: result is assigned to a String variable + String str = String.valueOf(obj); + // positive example: String method is called on the result + int len = String.valueOf(obj).length(); + } +} diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/output/WiderTypeTemplateExample.java b/core/src/test/java/com/google/errorprone/refaster/testdata/output/WiderTypeTemplateExample.java new file mode 100644 index 00000000000..d0c6e2f8567 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/output/WiderTypeTemplateExample.java @@ -0,0 +1,29 @@ +/* + * Copyright 2025 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.refaster.testdata; + +/** + * Example input for {@code WiderTypeTemplate}. + */ +public class WiderTypeTemplateExample { + public void example(Object obj) { + // positive example: result is assigned to a String variable + String str = String.valueOf(obj); + // positive example: String method is called on the result + int len = String.valueOf(obj).length(); + } +} diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/template/WiderTypeTemplate.java b/core/src/test/java/com/google/errorprone/refaster/testdata/template/WiderTypeTemplate.java new file mode 100644 index 00000000000..26689d585e6 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/template/WiderTypeTemplate.java @@ -0,0 +1,36 @@ +/* + * Copyright 2025 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.refaster.testdata.template; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +/** + * Sample Refaster template where the {@code @AfterTemplate} returns a wider type than the {@code + * @BeforeTemplate}. + */ +public class WiderTypeTemplate { + @BeforeTemplate + String before(Object obj) { + return String.valueOf(obj); + } + + @AfterTemplate + Object after(Object obj) { + return obj; + } +}