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 95d0423888f..04828f74599 100644 --- a/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java +++ b/core/src/main/java/com/google/errorprone/refaster/RefasterScanner.java @@ -28,6 +28,7 @@ import com.sun.source.tree.DoWhileLoopTree; import com.sun.source.tree.IfTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.PackageTree; import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.SynchronizedTree; import com.sun.source.tree.Tree; @@ -98,7 +99,7 @@ public Void visitVariable(VariableTree node, Context context) { @Override public Void scan(Tree tree, Context context) { - if (tree == null) { + if (tree == null || tree instanceof PackageTree) { return null; } JCCompilationUnit compilationUnit = context.get(JCCompilationUnit.class); diff --git a/core/src/main/java/com/google/errorprone/refaster/UFreeIdent.java b/core/src/main/java/com/google/errorprone/refaster/UFreeIdent.java index ffe9391f4cf..ce36e1be62e 100644 --- a/core/src/main/java/com/google/errorprone/refaster/UFreeIdent.java +++ b/core/src/main/java/com/google/errorprone/refaster/UFreeIdent.java @@ -23,6 +23,8 @@ import com.sun.source.tree.Tree; import com.sun.source.util.TreeScanner; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Symbol.TypeSymbol; import com.sun.tools.javac.tree.JCTree.JCExpression; import com.sun.tools.javac.util.Names; import javax.annotation.Nullable; @@ -99,7 +101,14 @@ public Boolean visitIdentifier(IdentifierTree ident, Void v) { if (!isGood) { return Choice.none(); } else if (currentBinding == null) { + Symbol currentExprSymbol = ASTHelpers.getSymbol(expression); + if (currentExprSymbol instanceof TypeSymbol + || (currentExprSymbol instanceof MethodSymbol && target instanceof IdentifierTree)) { + // The `JCExpression` by itself does not represent a valid Java expression. + return Choice.none(); + } unifier.putBinding(key(), expression); + return Choice.of(unifier); } else if (currentBinding.toString().equals(expression.toString())) { // TODO(lowasser): try checking types here in a way that doesn't reject 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 e9000acc4be..20c389c62ad 100644 --- a/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java +++ b/core/src/test/java/com/google/errorprone/refaster/TemplateIntegrationTest.java @@ -374,4 +374,9 @@ public void suppressWarnings() throws IOException { public void typeArgumentsMethodInvocation() throws IOException { runTest("TypeArgumentsMethodInvocationTemplate"); } + + @Test + public void onlyBeforeSimpleReturnTemplate() throws IOException { + runTest("OnlyBeforeSimpleReturnTemplate"); + } } diff --git a/core/src/test/java/com/google/errorprone/refaster/UFreeIdentTest.java b/core/src/test/java/com/google/errorprone/refaster/UFreeIdentTest.java index 7954f6888ab..6ce0d80244b 100644 --- a/core/src/test/java/com/google/errorprone/refaster/UFreeIdentTest.java +++ b/core/src/test/java/com/google/errorprone/refaster/UFreeIdentTest.java @@ -17,10 +17,18 @@ package com.google.errorprone.refaster; import static com.google.common.truth.Truth.assertThat; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import com.google.common.testing.EqualsTester; import com.google.common.testing.SerializableTester; -import com.sun.tools.javac.tree.JCTree.JCExpression; +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.util.Context; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -41,10 +49,33 @@ public void inlinesExpression() { @Test public void binds() { - JCExpression expr = parseExpression("\"abcdefg\".charAt(x + 1)"); - UFreeIdent ident = UFreeIdent.create("foo"); - assertThat(ident.unify(expr, unifier)).isNotNull(); - assertThat(unifier.getBindings()).containsExactly(new UFreeIdent.Key("foo"), expr); + CompilationTestHelper.newInstance(UnificationChecker.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " public void bar() {", + " int x = 0;", + " \"abcdefg\".charAt(x + 1);", + " }", + "}") + .doTest(); + } + + @BugPattern( + summary = "Verify that unifying the expression results in the correct binding", + explanation = "For test purposes only", + severity = SUGGESTION) + public static class UnificationChecker extends BugChecker implements MethodInvocationTreeMatcher { + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + Unifier unifier = new Unifier(new Context()); + UFreeIdent ident = UFreeIdent.create("foo"); + + assertThat(ident.unify(tree, unifier)).isNotNull(); + assertThat(unifier.getBindings()).containsExactly(new UFreeIdent.Key("foo"), tree); + + return Description.NO_MATCH; + } } @Test diff --git a/core/src/test/java/com/google/errorprone/refaster/URepeatedTest.java b/core/src/test/java/com/google/errorprone/refaster/URepeatedTest.java index 3e54de57179..b8528b030f1 100644 --- a/core/src/test/java/com/google/errorprone/refaster/URepeatedTest.java +++ b/core/src/test/java/com/google/errorprone/refaster/URepeatedTest.java @@ -17,10 +17,18 @@ package com.google.errorprone.refaster; import static com.google.common.truth.Truth.assertThat; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import com.google.common.testing.EqualsTester; import com.google.common.testing.SerializableTester; -import com.sun.tools.javac.tree.JCTree.JCExpression; +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.util.Context; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -31,10 +39,33 @@ public class URepeatedTest extends AbstractUTreeTest { @Test public void unifies() { - JCExpression expr = parseExpression("\"abcdefg\".charAt(x + 1)"); - URepeated ident = URepeated.create("foo", UFreeIdent.create("foo")); - assertThat(ident.unify(expr, unifier)).isNotNull(); - assertThat(unifier.getBindings()).containsExactly(new UFreeIdent.Key("foo"), expr); + CompilationTestHelper.newInstance(UnificationChecker.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " public void bar() {", + " int x = 0;", + " \"abcdefg\".charAt(x + 1);", + " }", + "}") + .doTest(); + } + + @BugPattern( + summary = "Verify that unifying the expression results in the correct binding", + explanation = "For test purposes only", + severity = SUGGESTION) + public static class UnificationChecker extends BugChecker implements MethodInvocationTreeMatcher { + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + Unifier unifier = new Unifier(new Context()); + URepeated ident = URepeated.create("foo", UFreeIdent.create("foo")); + + assertThat(ident.unify(tree, unifier)).isNotNull(); + assertThat(unifier.getBindings()).containsExactly(new UFreeIdent.Key("foo"), tree); + + return Description.NO_MATCH; + } } @Test diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/input/OnlyBeforeSimpleReturnTemplateExample.java b/core/src/test/java/com/google/errorprone/refaster/testdata/input/OnlyBeforeSimpleReturnTemplateExample.java new file mode 100644 index 00000000000..303cb00c9fc --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/input/OnlyBeforeSimpleReturnTemplateExample.java @@ -0,0 +1,24 @@ +/* + * Copyright 2023 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; + +/** Test data for {@code OnlyBeforeSimpleReturnTemplate}. */ +public class OnlyBeforeSimpleReturnTemplateExample { + public String foo(String s) { + return s; + } +} diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/output/OnlyBeforeSimpleReturnTemplateExample.java b/core/src/test/java/com/google/errorprone/refaster/testdata/output/OnlyBeforeSimpleReturnTemplateExample.java new file mode 100644 index 00000000000..f137caf1e06 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/output/OnlyBeforeSimpleReturnTemplateExample.java @@ -0,0 +1,24 @@ +/* + * Copyright 2023 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; + +/** Test data for {@code OnlyBeforeSimpleReturnTemplate}. */ +public class OnlyBeforeSimpleReturnTemplateExample { + public String foo(String s) { + return /* match found */ s; + } +} diff --git a/core/src/test/java/com/google/errorprone/refaster/testdata/template/OnlyBeforeSimpleReturnTemplate.java b/core/src/test/java/com/google/errorprone/refaster/testdata/template/OnlyBeforeSimpleReturnTemplate.java new file mode 100644 index 00000000000..b30ee04781a --- /dev/null +++ b/core/src/test/java/com/google/errorprone/refaster/testdata/template/OnlyBeforeSimpleReturnTemplate.java @@ -0,0 +1,30 @@ +/* + * Copyright 2023 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.BeforeTemplate; + +/** + * Template that flags all string-typed expressions, irrespective of the syntactic context in which + * they occur. + */ +public class OnlyBeforeSimpleReturnTemplate { + @BeforeTemplate + String before(String s) { + return s; + } +}