diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java b/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java deleted file mode 100644 index 6b57f5f6989..00000000000 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java +++ /dev/null @@ -1,325 +0,0 @@ -/* - * Copyright 2024 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.bugpatterns; - -import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; -import static com.google.errorprone.matchers.Description.NO_MATCH; -import static com.google.errorprone.matchers.Matchers.hasModifier; -import static com.google.errorprone.util.ASTHelpers.getSymbol; -import static java.beans.Introspector.decapitalize; - -import com.google.auto.value.AutoValue; -import com.google.common.base.Ascii; -import com.google.common.collect.ImmutableList; -import com.google.errorprone.BugPattern; -import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; -import com.google.errorprone.dataflow.nullnesspropagation.Nullness; -import com.google.errorprone.dataflow.nullnesspropagation.NullnessAnnotations; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; -import com.google.errorprone.matchers.Matcher; -import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.ClassTree; -import com.sun.source.tree.IdentifierTree; -import com.sun.source.tree.MethodTree; -import com.sun.source.tree.NewClassTree; -import com.sun.source.tree.ReturnTree; -import com.sun.source.tree.Tree; -import com.sun.tools.javac.code.Type; -import com.sun.tools.javac.code.TypeTag; -import java.util.List; -import java.util.Optional; -import javax.lang.model.element.Modifier; -import javax.lang.model.type.TypeKind; - -/** See summary for details. */ -@BugPattern( - summary = - "AutoValue instances should not usually contain boxed types that are not Nullable. We" - + " recommend removing the unnecessary boxing.", - severity = WARNING) -public class AutoValueBoxedValues extends BugChecker implements ClassTreeMatcher { - private static final Matcher ABSTRACT_MATCHER = hasModifier(Modifier.ABSTRACT); - private static final Matcher STATIC_MATCHER = hasModifier(Modifier.STATIC); - - @Override - public Description matchClass(ClassTree tree, VisitorState state) { - if (!ASTHelpers.hasAnnotation(tree, AutoValue.class.getName(), state)) { - return NO_MATCH; - } - - // Identify and potentially fix the getters. - ImmutableList getters = handleGetterMethods(tree, state); - - // If we haven't modified any getter, it's ok to stop. - if (getters.stream().allMatch(getter -> getter.fix().isEmpty())) { - return NO_MATCH; - } - - // Handle the Builder class, if there is one. - boolean builderFound = false; - for (Tree memberTree : tree.getMembers()) { - if (memberTree instanceof ClassTree - && ASTHelpers.hasAnnotation(memberTree, AutoValue.Builder.class.getName(), state)) { - handleSetterMethods((ClassTree) memberTree, state, getters); - builderFound = true; - break; - } - } - - // If a builder was not found, handle the factory methods. - if (!builderFound) { - handleFactoryMethods(tree, state, getters); - } - - getters.stream() - .filter(getter -> !getter.fix().isEmpty()) - .forEach(getter -> state.reportMatch(describeMatch(getter.method(), getter.fix().build()))); - - return NO_MATCH; - } - - /** - * Returns the {@link List} of {@link Getter} in the {@link AutoValue} class along with the fixes - * to be applied. - * - * @param classTree The {@link AutoValue} class tree. - * @param state The visitor state. - * @return The list of {@link Getter} in the class. - */ - private ImmutableList handleGetterMethods(ClassTree classTree, VisitorState state) { - return classTree.getMembers().stream() - .filter(MethodTree.class::isInstance) - .map(memberTree -> (MethodTree) memberTree) - .filter( - methodTree -> - ABSTRACT_MATCHER.matches(methodTree, state) && methodTree.getParameters().isEmpty()) - .map(methodTree -> maybeFixGetter(methodTree, state)) - .collect(toImmutableList()); - } - - /** - * Converts the {@link MethodTree} of a getter to a {@link Getter}. If the getter needs to be - * fixed, it returns a {@link Getter} with a non-empty {@link SuggestedFix}. - */ - private Getter maybeFixGetter(MethodTree method, VisitorState state) { - Getter getter = Getter.of(method); - Type type = ASTHelpers.getType(method.getReturnType()); - if (!isSuppressed(method, state) - && !hasNullableAnnotation(method) - && isBoxedPrimitive(state, type)) { - suggestRemoveUnnecessaryBoxing(method.getReturnType(), state, type, getter.fix()); - } - return getter; - } - - /** - * Identifies and fixes the setters in the {@link AutoValue.Builder} class. - * - * @param classTree The {@link AutoValue.Builder} class tree. - * @param state The visitor state. - * @param getters The {@link List} of {@link Getter} in the {@link AutoValue} class. - */ - private void handleSetterMethods(ClassTree classTree, VisitorState state, List getters) { - classTree.getMembers().stream() - .filter(MethodTree.class::isInstance) - .map(memberTree -> (MethodTree) memberTree) - .filter( - methodTree -> - ABSTRACT_MATCHER.matches(methodTree, state) - && methodTree.getParameters().size() == 1) - .forEach(methodTree -> maybeFixSetter(methodTree, state, getters)); - } - - /** Given a setter, it tries to apply a fix if the corresponding getter was also fixed. */ - private void maybeFixSetter(MethodTree methodTree, VisitorState state, List getters) { - if (isSuppressed(methodTree, state)) { - return; - } - boolean allGettersPrefixed = allGettersPrefixed(getters); - Optional fixedGetter = - getters.stream() - .filter( - getter -> - !getter.fix().isEmpty() - && matchGetterAndSetter(getter.method(), methodTree, allGettersPrefixed)) - .findAny(); - if (fixedGetter.isPresent()) { - var parameter = methodTree.getParameters().get(0); - Type type = ASTHelpers.getType(parameter); - if (isBoxedPrimitive(state, type) && !hasNullableAnnotation(parameter)) { - suggestRemoveUnnecessaryBoxing(parameter.getType(), state, type, fixedGetter.get().fix()); - } - } - } - - /** - * Identifies and fixes the factory method in the {@link AutoValue} class. - * - *

This method only handles the case of "trivial" factory methods, i.e. methods that have one - * argument for each getter in the class and contains a single return statement passing all the - * arguments to the constructor in the same order. - * - * @param classTree The {@link AutoValue} class tree. - * @param state The visitor state. - * @param getters The {@link List} of {@link Getter} in the {@link AutoValue} class. - */ - private void handleFactoryMethods(ClassTree classTree, VisitorState state, List getters) { - Optional trivialFactoryMethod = - classTree.getMembers().stream() - .filter(MethodTree.class::isInstance) - .map(memberTree -> (MethodTree) memberTree) - .filter( - methodTree -> - STATIC_MATCHER.matches(methodTree, state) - && ASTHelpers.isSameType( - ASTHelpers.getType(methodTree.getReturnType()), - ASTHelpers.getType(classTree), - state) - && isTrivialFactoryMethod(methodTree, getters.size())) - .findAny(); - if (trivialFactoryMethod.isEmpty()) { - return; - } - for (int idx = 0; idx < getters.size(); idx++) { - Getter getter = getters.get(idx); - if (!getter.fix().isEmpty()) { - var parameter = trivialFactoryMethod.get().getParameters().get(idx); - Type type = ASTHelpers.getType(parameter); - if (isBoxedPrimitive(state, type) && !hasNullableAnnotation(parameter)) { - suggestRemoveUnnecessaryBoxing(parameter.getType(), state, type, getter.fix()); - } - } - } - } - - /** Returns true if the given tree has a {@code Nullable} annotation. */ - private static boolean hasNullableAnnotation(Tree tree) { - return NullnessAnnotations.fromAnnotationsOn(getSymbol(tree)).orElse(null) == Nullness.NULLABLE; - } - - /** Returns the primitive type corresponding to a boxed type. */ - private static Type unbox(VisitorState state, Type type) { - return state.getTypes().unboxedType(type); - } - - /** Returns true if the value of {@link Type} is a boxed primitive. */ - private static boolean isBoxedPrimitive(VisitorState state, Type type) { - if (type.isPrimitive()) { - return false; - } - Type unboxed = unbox(state, type); - return unboxed != null && unboxed.getTag() != TypeTag.NONE && unboxed.getTag() != TypeTag.VOID; - } - - private static boolean allGettersPrefixed(List getters) { - return getters.stream().allMatch(getter -> !getterPrefix(getter.method()).isEmpty()); - } - - private static String getterPrefix(MethodTree getterMethod) { - String name = getterMethod.getName().toString(); - if (name.startsWith("get") && !name.equals("get")) { - return "get"; - } else if (name.startsWith("is") - && !name.equals("is") - && ASTHelpers.getType(getterMethod.getReturnType()).getKind() == TypeKind.BOOLEAN) { - return "is"; - } - return ""; - } - - /** Returns true if the getter and the setter are for the same field. */ - private static boolean matchGetterAndSetter( - MethodTree getter, MethodTree setter, boolean allGettersPrefixed) { - String getterName = getter.getName().toString(); - if (allGettersPrefixed) { - String prefix = getterPrefix(getter); - getterName = decapitalize(getterName.substring(prefix.length())); - } - String setterName = setter.getName().toString(); - return getterName.equals(setterName) - || setterName.equals( - "set" + Ascii.toUpperCase(getterName.charAt(0)) + getterName.substring(1)); - } - - /** - * Returns true if the method is a trivial factory method. - * - *

A trivial factory method is a static method that has one argument for each getter in the - * class and contains a single return statement passing all the arguments to the constructor in - * the same order. - * - * @param methodTree The method tree to be checked. - * @param gettersCount The total number of getters in the class. - * @return True if the method is a trivial factory method, false otherwise. - */ - private static boolean isTrivialFactoryMethod(MethodTree methodTree, int gettersCount) { - var params = methodTree.getParameters(); - var statements = methodTree.getBody().getStatements(); - - // Trivial factory method must have one argument for each getter and a single return statement. - if (params.size() != gettersCount - || statements.size() != 1 - || !(statements.get(0) instanceof ReturnTree)) { - return false; - } - // Trivial factory method must return a new instance. - ReturnTree returnTree = (ReturnTree) statements.get(0); - if (!(returnTree.getExpression() instanceof NewClassTree)) { - return false; - } - // Trivial factory method must pass all the arguments to the constructor. - NewClassTree newClassTree = (NewClassTree) returnTree.getExpression(); - if (newClassTree.getArguments().stream().anyMatch(r -> !(r instanceof IdentifierTree))) { - return false; - } - // Compare the arguments passed to the method to those passed to the constructor. - var paramsNames = params.stream().map(p -> p.getName().toString()).collect(toImmutableList()); - var constructorArgs = - newClassTree.getArguments().stream() - .map(r -> ((IdentifierTree) r).getName().toString()) - .collect(toImmutableList()); - return paramsNames.equals(constructorArgs); - } - - /** - * Suggests a fix to replace the boxed type with the unboxed type. - * - *

For example, if the type is `Integer`, the fix would be to replace `Integer` with `int`. - * - * @param tree The tree to be replaced, which can be either a return value or a parameter. - * @param state The visitor state. - * @param type The boxed type to be replaced. - */ - private static void suggestRemoveUnnecessaryBoxing( - Tree tree, VisitorState state, Type type, SuggestedFix.Builder fix) { - fix.replace(tree, unbox(state, type).tsym.getSimpleName().toString()); - } - - @AutoValue - abstract static class Getter { - abstract MethodTree method(); - - abstract SuggestedFix.Builder fix(); - - static Getter of(MethodTree method) { - return new AutoValue_AutoValueBoxedValues_Getter(method, SuggestedFix.builder()); - } - } -} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java index cf3a3ca05c7..39b5b7585d0 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -36,7 +36,6 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.google.errorprone.BugPattern; import com.google.errorprone.ErrorProneFlags; @@ -585,7 +584,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( groupedCaseCommentsAccumulator.length() == 0 ? null : groupedCaseCommentsAccumulator.toString(), - transformedBlockSource.isEmpty() ? null : transformedBlockSource, + transformedBlockSource.isEmpty() ? null : transformedBlockSource.trim(), commentsBeforeRemovedBreak.orElse(null), commentsAfterCaseOptional.orElse(null)); } @@ -902,7 +901,7 @@ && getStatements(caseTree).size() > filteredStatements.size()) { state .getSourceCode() .subSequence( - state.getEndPosition(Iterables.getLast(filteredStatements)), + state.getEndPosition(getLast(filteredStatements)), getStartPosition(getStatements(caseTree).get(getStatements(caseTree).size() - 1))) .toString() .trim(); @@ -952,11 +951,10 @@ private static String transformBlock( StringBuilder transformedBlockBuilder = new StringBuilder(); int codeBlockStart = extractLhsComments(caseTree, state, transformedBlockBuilder); - int codeBlockEnd = - filteredStatements.isEmpty() - ? getBlockEnd(state, caseTree) - : state.getEndPosition(Streams.findLast(filteredStatements.stream()).get()); - transformedBlockBuilder.append(state.getSourceCode(), codeBlockStart, codeBlockEnd); + if (!filteredStatements.isEmpty()) { + int codeBlockEnd = state.getEndPosition(getLast(filteredStatements)); + transformedBlockBuilder.append(state.getSourceCode(), codeBlockStart, codeBlockEnd); + } return transformedBlockBuilder.toString(); } 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 e6b874b486b..e02825e352a 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -42,7 +42,6 @@ import com.google.errorprone.bugpatterns.AsyncCallableReturnsNull; import com.google.errorprone.bugpatterns.AsyncFunctionReturnsNull; import com.google.errorprone.bugpatterns.AttemptedNegativeZero; -import com.google.errorprone.bugpatterns.AutoValueBoxedValues; import com.google.errorprone.bugpatterns.AutoValueBuilderDefaultsInConstructor; import com.google.errorprone.bugpatterns.AutoValueFinalMethods; import com.google.errorprone.bugpatterns.AutoValueImmutableFields; @@ -1129,7 +1128,6 @@ public static ScannerSupplier warningChecks() { AssertFalse.class, AssistedInjectAndInjectOnConstructors.class, AutoFactoryAtInject.class, - AutoValueBoxedValues.class, AvoidObjectArrays.class, BanClassLoader.class, BanSerializableRead.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java deleted file mode 100644 index 392692d391f..00000000000 --- a/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java +++ /dev/null @@ -1,590 +0,0 @@ -/* - * Copyright 2024 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.bugpatterns; - -import static java.util.Arrays.stream; - -import com.google.auto.value.processor.AutoValueProcessor; -import com.google.common.collect.ImmutableList; -import com.google.errorprone.BugCheckerRefactoringTestHelper; -import com.google.errorprone.CompilationTestHelper; -import com.google.testing.junit.testparameterinjector.TestParameter; -import com.google.testing.junit.testparameterinjector.TestParameterInjector; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** Unit tests for {@link AutoValueBoxedValues}. */ -@RunWith(TestParameterInjector.class) -public class AutoValueBoxedValuesTest { - private final CompilationTestHelper compilationHelper = - CompilationTestHelper.newInstance(AutoValueBoxedValues.class, getClass()) - .setArgs(ImmutableList.of("-processor", AutoValueProcessor.class.getName())); - ; - private final BugCheckerRefactoringTestHelper refactoringHelper = - BugCheckerRefactoringTestHelper.newInstance(AutoValueBoxedValues.class, getClass()) - .setArgs(ImmutableList.of("-processor", AutoValueProcessor.class.getName())); - - @TestParameter private boolean withBuilder; - - @Test - public void unnecessaryBoxedTypes_refactoring() { - refactoringHelper - .addInputLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract Long longId();", - " public abstract Integer intId();", - " public abstract Byte byteId();", - " public abstract Short shortId();", - " public abstract Float floatId();", - " public abstract Double doubleId();", - " public abstract Boolean booleanId();", - " public abstract Character charId();"), - linesWithoutBuilder( - " static Test create(", - " Long longId, Integer intId, Byte byteId, Short shortId,", - " Float floatId, Double doubleId, Boolean booleanId, Character charId) {", - " return new AutoValue_Test(longId, intId, byteId, shortId, floatId,", - " doubleId, booleanId, charId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(Long value);", - " abstract Builder setIntId(Integer value);", - " abstract Builder setByteId(Byte value);", - " abstract Builder setShortId(Short value);", - " abstract Builder setFloatId(Float value);", - " abstract Builder setDoubleId(Double value);", - " abstract Builder setBooleanId(Boolean value);", - " abstract Builder setCharId(Character value);", - " abstract Test build();", - " }"), - lines("}"))) - .addOutputLines( - "out/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract long longId();", - " public abstract int intId();", - " public abstract byte byteId();", - " public abstract short shortId();", - " public abstract float floatId();", - " public abstract double doubleId();", - " public abstract boolean booleanId();", - " public abstract char charId();"), - linesWithoutBuilder( - " static Test create(", - " long longId, int intId, byte byteId, short shortId,", - " float floatId, double doubleId, boolean booleanId, char charId) {", - " return new AutoValue_Test(longId, intId, byteId, shortId, floatId,", - " doubleId, booleanId, charId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(long value);", - " abstract Builder setIntId(int value);", - " abstract Builder setByteId(byte value);", - " abstract Builder setShortId(short value);", - " abstract Builder setFloatId(float value);", - " abstract Builder setDoubleId(double value);", - " abstract Builder setBooleanId(boolean value);", - " abstract Builder setCharId(char value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void nullableBoxedTypes() { - compilationHelper - .addSourceLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "import javax.annotation.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract @Nullable Long longId();", - " public abstract @Nullable Integer intId();", - " public abstract @Nullable Byte byteId();", - " public abstract @Nullable Short shortId();", - " public abstract @Nullable Float floatId();", - " public abstract @Nullable Double doubleId();", - " public abstract @Nullable Boolean booleanId();", - " public abstract @Nullable Character charId();"), - linesWithoutBuilder( - " static Test create(", - " @Nullable Long longId, @Nullable Integer intId, @Nullable Byte byteId,", - " @Nullable Short shortId, @Nullable Float floatId,", - " @Nullable Double doubleId, @Nullable Boolean booleanId,", - " @Nullable Character charId) {", - " return new AutoValue_Test(longId, intId, byteId, shortId, floatId,", - " doubleId, booleanId, charId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(@Nullable Long value);", - " abstract Builder setIntId(@Nullable Integer value);", - " abstract Builder setByteId(@Nullable Byte value);", - " abstract Builder setShortId(@Nullable Short value);", - " abstract Builder setFloatId(@Nullable Float value);", - " abstract Builder setDoubleId(@Nullable Double value);", - " abstract Builder setBooleanId(@Nullable Boolean value);", - " abstract Builder setCharId(@Nullable Character value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void genericNullableBoxedTypes() { - compilationHelper - .addSourceLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract @Nullable Long longId();", - " public abstract @Nullable Integer intId();", - " public abstract @Nullable Byte byteId();", - " public abstract @Nullable Short shortId();", - " public abstract @Nullable Float floatId();", - " public abstract @Nullable Double doubleId();", - " public abstract @Nullable Boolean booleanId();", - " public abstract @Nullable Character charId();"), - linesWithoutBuilder( - " static Test create(", - " @Nullable Long longId, @Nullable Integer intId, @Nullable Byte byteId,", - " @Nullable Short shortId, @Nullable Float floatId,", - " @Nullable Double doubleId, @Nullable Boolean booleanId,", - " @Nullable Character charId) {", - " return new AutoValue_Test(longId, intId, byteId, shortId, floatId,", - " doubleId, booleanId, charId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(@Nullable Long value);", - " abstract Builder setIntId(@Nullable Integer value);", - " abstract Builder setByteId(@Nullable Byte value);", - " abstract Builder setShortId(@Nullable Short value);", - " abstract Builder setFloatId(@Nullable Float value);", - " abstract Builder setDoubleId(@Nullable Double value);", - " abstract Builder setBooleanId(@Nullable Boolean value);", - " abstract Builder setCharId(@Nullable Character value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void primitiveTypes() { - compilationHelper - .addSourceLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract long longId();", - " public abstract int intId();", - " public abstract byte byteId();", - " public abstract short shortId();", - " public abstract float floatId();", - " public abstract double doubleId();", - " public abstract boolean booleanId();", - " public abstract char charId();"), - linesWithoutBuilder( - " static Test create(", - " long longId, int intId, byte byteId, short shortId,", - " float floatId, double doubleId, boolean booleanId, char charId) {", - " return new AutoValue_Test(longId, intId, byteId, shortId, floatId,", - " doubleId, booleanId, charId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(long value);", - " abstract Builder setIntId(int value);", - " abstract Builder setByteId(byte value);", - " abstract Builder setShortId(short value);", - " abstract Builder setFloatId(float value);", - " abstract Builder setDoubleId(double value);", - " abstract Builder setBooleanId(boolean value);", - " abstract Builder setCharId(char value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void nonBoxableTypes() { - compilationHelper - .addSourceLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "import javax.annotation.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract String stringId();", - " public @Nullable abstract String nullableStringId();"), - linesWithoutBuilder( - " static Test create(String stringId, @Nullable String nullableStringId) {", - " return new AutoValue_Test(stringId, nullableStringId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setStringId(String value);", - " abstract Builder setNullableStringId(@Nullable String value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void mixedTypes_refactoring() { - refactoringHelper - .addInputLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "import javax.annotation.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract @Nullable Long nullableId();", - " public abstract Long unnecessaryBoxedId();", - " public abstract long primitiveId();", - " public abstract String nonBoxableId();"), - linesWithoutBuilder( - " static Test create(", - " @Nullable Long nullableId, Long unnecessaryBoxedId,", - " long primitiveId, String nonBoxableId) {", - " return new AutoValue_Test(", - " nullableId, unnecessaryBoxedId, primitiveId, nonBoxableId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setNullableId(@Nullable Long value);", - " abstract Builder setUnnecessaryBoxedId(Long value);", - " abstract Builder setPrimitiveId(long value);", - " abstract Builder setNonBoxableId(String value);", - " abstract Test build();", - " }"), - lines("}"))) - .addOutputLines( - "out/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "import javax.annotation.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract @Nullable Long nullableId();", - " public abstract long unnecessaryBoxedId();", - " public abstract long primitiveId();", - " public abstract String nonBoxableId();"), - linesWithoutBuilder( - " static Test create(", - " @Nullable Long nullableId, long unnecessaryBoxedId,", - " long primitiveId, String nonBoxableId) {", - " return new AutoValue_Test(", - " nullableId, unnecessaryBoxedId, primitiveId, nonBoxableId);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setNullableId(@Nullable Long value);", - " abstract Builder setUnnecessaryBoxedId(long value);", - " abstract Builder setPrimitiveId(long value);", - " abstract Builder setNonBoxableId(String value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void unnecessaryBoxedTypes_suppressWarnings() { - refactoringHelper - .addInputLines( - "in/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract Long longId();", - " @SuppressWarnings(\"AutoValueBoxedValues\")", - " public abstract Long longIdSuppressWarnings();"), - linesWithoutBuilder( - " static Test create(Long longId, Long longIdSuppressWarnings) {", - " return new AutoValue_Test(longId, longIdSuppressWarnings);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(Long value);", - " @SuppressWarnings(\"AutoValueBoxedValues\")", - " abstract Builder setLongIdSuppressWarnings(Long value);", - " abstract Test build();", - " }"), - lines("}"))) - .addOutputLines( - "out/Test.java", - mergeLines( - lines( - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract long longId();", - " @SuppressWarnings(\"AutoValueBoxedValues\")", - " public abstract Long longIdSuppressWarnings();"), - linesWithoutBuilder( - " static Test create(long longId, Long longIdSuppressWarnings) {", - " return new AutoValue_Test(longId, longIdSuppressWarnings);", - " }"), - linesWithBuilder( - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(long value);", - " @SuppressWarnings(\"AutoValueBoxedValues\")", - " abstract Builder setLongIdSuppressWarnings(Long value);", - " abstract Test build();", - " }"), - lines("}"))) - .doTest(); - } - - @Test - public void nullableGettersWithNonNullableSetters_noChange() { - if (!withBuilder) { - return; - } - compilationHelper - .addSourceLines( - "in/Test.java", - "import com.google.auto.value.AutoValue;", - "import javax.annotation.Nullable;", - "@AutoValue", - "abstract class Test {", - " public abstract @Nullable Long longId();", - " public abstract @Nullable Integer intId();", - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(Long value);", - " abstract Builder setIntId(Integer value);", - " abstract Test build();", - " }", - "}") - .doTest(); - } - - @Test - public void nonTrivialFactoryMethods_refectoring() { - if (withBuilder) { - return; - } - refactoringHelper - .addInputLines( - "in/Test.java", - "package test;", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " abstract int foo();", - " abstract Long bar();", - " static Test createTrivial(int foo, Long bar) {", - " return new AutoValue_Test(foo, bar);", - " }", - " static String notFactoryMethod(int foo, Long bar) {", - " return String.format(\"foo: %d, bar: %d\", foo, bar);", - " }", - " static Test createWrongOrder(Long bar, int foo) {", - " return new AutoValue_Test(foo, bar);", - " }", - " static Test createLessArguments(int foo) {", - " return new AutoValue_Test(foo, 0L);", - " }", - " static Test createMoreArguments(int foo, Long bar, Long baz) {", - " return new AutoValue_Test(foo, bar + baz);", - " }", - " static Test createWithValidation(int foo, Long bar) {", - " if (bar == null) { throw new AssertionError(); }", - " return new AutoValue_Test(foo, bar);", - " }", - " static Test createModifyArgs(int foo, Long bar) {", - " return new AutoValue_Test(foo + 1, bar);", - " }", - " static Test createModifyArgsIfNull(int foo, Long bar) {", - " return new AutoValue_Test(foo, bar == null ? 0L : bar);", - " }", - "}") - .addOutputLines( - "out/Test.java", - "package test;", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " abstract int foo();", - " abstract long bar();", - " static Test createTrivial(int foo, long bar) {", - " return new AutoValue_Test(foo, bar);", - " }", - " static String notFactoryMethod(int foo, Long bar) {", - " return String.format(\"foo: %d, bar: %d\", foo, bar);", - " }", - " static Test createWrongOrder(Long bar, int foo) {", - " return new AutoValue_Test(foo, bar);", - " }", - " static Test createLessArguments(int foo) {", - " return new AutoValue_Test(foo, 0L);", - " }", - " static Test createMoreArguments(int foo, Long bar, Long baz) {", - " return new AutoValue_Test(foo, bar + baz);", - " }", - " static Test createWithValidation(int foo, Long bar) {", - " if (bar == null) { throw new AssertionError(); }", - " return new AutoValue_Test(foo, bar);", - " }", - " static Test createModifyArgs(int foo, Long bar) {", - " return new AutoValue_Test(foo + 1, bar);", - " }", - " static Test createModifyArgsIfNull(int foo, Long bar) {", - " return new AutoValue_Test(foo, bar == null ? 0L : bar);", - " }", - "}") - .doTest(); - } - - @Test - public void settersWithoutSetPrefix() { - if (!withBuilder) { - return; - } - refactoringHelper - .addInputLines( - "in/Test.java", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract Long longId();", - " public abstract Boolean booleanId();", - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder longId(Long value);", - " abstract Builder booleanId(Boolean value);", - " abstract Test build();", - " }", - "}") - .addOutputLines( - "out/Test.java", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract long longId();", - " public abstract boolean booleanId();", - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder longId(long value);", - " abstract Builder booleanId(boolean value);", - " abstract Test build();", - " }", - "}") - .doTest(); - } - - @Test - public void allGettersWithPrefix() { - if (!withBuilder) { - return; - } - refactoringHelper - .addInputLines( - "in/Test.java", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract Long getLongId();", - " public abstract boolean isBooleanId();", - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(Long value);", - " abstract Builder setBooleanId(boolean value);", - " abstract Test build();", - " }", - "}") - .addOutputLines( - "out/Test.java", - "import com.google.auto.value.AutoValue;", - "@AutoValue", - "abstract class Test {", - " public abstract long getLongId();", - " public abstract boolean isBooleanId();", - " @AutoValue.Builder", - " abstract static class Builder {", - " abstract Builder setLongId(long value);", - " abstract Builder setBooleanId(boolean value);", - " abstract Test build();", - " }", - "}") - .doTest(); - } - - private static List lines(String... lines) { - return Arrays.asList(lines); - } - - private List linesWithBuilder(String... lines) { - return withBuilder ? Arrays.asList(lines) : new ArrayList<>(); - } - - private List linesWithoutBuilder(String... lines) { - return !withBuilder ? Arrays.asList(lines) : new ArrayList<>(); - } - - private static String[] mergeLines(List... blocks) { - return stream(blocks).flatMap(List::stream).toArray(String[]::new); - } -} diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java index eab2d16735b..be63a77f336 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java @@ -361,7 +361,6 @@ public void switchByEnumCard2_removesRedundantBreaks_error() { " // Post break comment", " case DIAMOND -> {", " // Diamond break comment", - " break;", " }", " case SPADE, CLUB -> System.out.println(\"everything else\");", " }", @@ -1385,7 +1384,6 @@ public void switchByEnum_surroundingBracesCannotRemove_error() { " switch(side) {", " case OBVERSE -> {", " // The quick brown fox, jumps over the lazy dog, etc.", - " break;", " }", " default -> ", " throw new RuntimeException(\"Invalid type.\");", @@ -3623,4 +3621,40 @@ public void i4222() { .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } + + @Test + public void unnecessaryBreaks() { + assumeTrue(RuntimeVersion.isAtLeast14()); + refactoringHelper + .addInputLines( + "Test.java", + "public class Test {", + " public static void main(String[] args) {", + " switch (args.length) {", + " case 0:", + " System.out.println(0);", + " break;", + " default:", + " // hello", + " // world", + " break;", + " }", + " }", + "}") + .addOutputLines( + "Test.java", + "public class Test {", + " public static void main(String[] args) {", + " switch (args.length) {", + " case 0 -> System.out.println(0);", + " default -> {", + " // hello", + " // world", + " }", + " }", + " }", + "}") + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } } diff --git a/docs/bugpattern/AutoValueBoxedValues.md b/docs/bugpattern/AutoValueBoxedValues.md deleted file mode 100644 index 3c90729ad23..00000000000 --- a/docs/bugpattern/AutoValueBoxedValues.md +++ /dev/null @@ -1,11 +0,0 @@ -AutoValue classes reject `null` values, unless the property is annotated with -`@Nullable`. For this reason, the usage of boxed primitives (e.g. `Long`) is -discouraged, except when annotated as `@Nullable`. Otherwise they can be -replaced with the corresponding primitive. There could be some cases where the -usage of a boxed primitive might be intentional to avoid boxing the value again -after invoking the getter. - -## Suppression - -Suppress violations by using `@SuppressWarnings("AutoValueBoxedValues")` on the -relevant `abstract` getter and/or setter.