From 487d0ef78fc235ee6b2f7c6c24811b2cfc90a6aa Mon Sep 17 00:00:00 2001 From: cushon Date: Thu, 18 May 2017 09:05:00 -0700 Subject: [PATCH] New check to detect tranposing the arguments to nCopies This is similar to StringBuilderInitWithChar and IndexOfChar. Calling nCopies('a', 10) returns a list with 97 copies of 10, not a list with 10 copies of 'a'. RELNOTES: New check to detect tranposing the arguments to nCopies ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=156437827 --- .../errorprone/bugpatterns/NCopiesOfChar.java | 68 +++++++++++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../bugpatterns/NCopiesOfCharTest.java | 55 +++++++++++++++ docs/bugpattern/NCopiesOfChar.md | 2 + 4 files changed, 127 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/NCopiesOfChar.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/NCopiesOfCharTest.java create mode 100644 docs/bugpattern/NCopiesOfChar.md diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NCopiesOfChar.java b/core/src/main/java/com/google/errorprone/bugpatterns/NCopiesOfChar.java new file mode 100644 index 00000000000..53334f7cf9a --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/NCopiesOfChar.java @@ -0,0 +1,68 @@ +/* + * Copyright 2017 Google Inc. All Rights Reserved. + * + * 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.bugpatterns; + +import static com.google.errorprone.BugPattern.Category.JDK; +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; +import static com.google.errorprone.util.ASTHelpers.getType; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Symtab; +import com.sun.tools.javac.code.Types; +import java.util.List; + +/** @author cushon@google.com (Liam Miller-Cushon) */ +@BugPattern( + name = "NCopiesOfChar", + category = JDK, + summary = + "The first argument to nCopies is the number of copies, and the second is the item to copy", + severity = ERROR +) +public class NCopiesOfChar extends BugChecker implements MethodInvocationTreeMatcher { + private static final Matcher MATCHER = + staticMethod().onClass("java.util.Collections").named("nCopies"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!MATCHER.matches(tree, state)) { + return NO_MATCH; + } + List arguments = tree.getArguments(); + Symtab syms = state.getSymtab(); + Types types = state.getTypes(); + if (types.isSameType(types.unboxedTypeOrType(getType(arguments.get(1))), syms.intType) + && types.isSameType(types.unboxedTypeOrType(getType(arguments.get(0))), syms.charType)) { + return describeMatch( + tree, + SuggestedFix.builder() + .replace(arguments.get(0), state.getSourceForNode(arguments.get(1))) + .replace(arguments.get(1), state.getSourceForNode(arguments.get(0))) + .build()); + } + return NO_MATCH; + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index b6d94871a33..2a582aa8c65 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -112,6 +112,7 @@ import com.google.errorprone.bugpatterns.MultipleTopLevelClasses; import com.google.errorprone.bugpatterns.MustBeClosedChecker; import com.google.errorprone.bugpatterns.MutableConstantField; +import com.google.errorprone.bugpatterns.NCopiesOfChar; import com.google.errorprone.bugpatterns.NarrowingCompoundAssignment; import com.google.errorprone.bugpatterns.NoAllocationChecker; import com.google.errorprone.bugpatterns.NonAtomicVolatileUpdate; @@ -340,6 +341,7 @@ public static ScannerSupplier errorChecks() { MoreThanOneInjectableConstructor.class, MoreThanOneScopeAnnotationOnClass.class, MustBeClosedChecker.class, + NCopiesOfChar.class, NonCanonicalStaticImport.class, NonFinalCompileTimeConstant.class, NonRuntimeAnnotation.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/NCopiesOfCharTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/NCopiesOfCharTest.java new file mode 100644 index 00000000000..6a9357482c0 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/NCopiesOfCharTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2017 Google Inc. All Rights Reserved. + * + * 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.bugpatterns; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** {@link NCopiesOfChar}Test */ +@RunWith(JUnit4.class) +public class NCopiesOfCharTest { + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(NCopiesOfChar.class, getClass()); + + @Test + public void positive() throws Exception { + compilationHelper + .addSourceLines( + "Test.java", // + "import java.util.Collections;", + "class Test {{", + " // BUG: Diagnostic contains: nCopies(10, ' ');", + " Collections.nCopies(' ', 10);", + "}}") + .doTest(); + } + + @Test + public void negative() throws Exception { + compilationHelper + .addSourceLines( + "Test.java", // + "import java.util.Collections;", + "class Test {{", + " Collections.nCopies(10, ' ');", + "}}") + .doTest(); + } +} diff --git a/docs/bugpattern/NCopiesOfChar.md b/docs/bugpattern/NCopiesOfChar.md new file mode 100644 index 00000000000..08b39a193e4 --- /dev/null +++ b/docs/bugpattern/NCopiesOfChar.md @@ -0,0 +1,2 @@ +Calling `nCopies('a', 10)` returns a list with 97 copies of 10, not a list with +10 copies of 'a'.