diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml index 7a1bacb63..293a55241 100644 --- a/.github/workflows/swift.yml +++ b/.github/workflows/swift.yml @@ -9,7 +9,7 @@ on: jobs: build: - runs-on: macos-11 + runs-on: macos-latest strategy: matrix: xcode: ['13.1'] @@ -19,7 +19,7 @@ jobs: uses: actions/checkout@v2 - name: Set Xcode ${{ matrix.xcode }} run: | - sudo xcode-select -s /Applications/Xcode_13.1.app + sudo xcode-select -s /Applications/Xcode_13.0.app xcodebuild -version swift --version swift package --version diff --git a/java/README.md b/java/README.md index 05715da16..e418756d8 100644 --- a/java/README.md +++ b/java/README.md @@ -135,6 +135,14 @@ Additionally, usages of `STALE_FLAG` will be removed as if the enum itself had b Finally, the setting `linkURL` in the properties file is to provide a URL describing the Piranha tooling and any custom configurations associated with the codebase. +Another top-level optional field is `unnecessaryTestMethodProperties`. Within that, there is an array of JSON objects, +having the fields `methodName`, `argumentIndex`, `receiverType`, `returnType` and `isStatic`. + +The refactored code may contain unnecessary method invocations. For instance, +Piranha changes the statement `when(experimentation.isToggleDisabled("STALE_FLAG")).thenReturn(false);` to `when(true).thenReturn(false);`, +where the invocation `when(true)` is unnecessary, and could be deleted. +This field (`unnecessaryTestMethodProperties`) is used to define such potentially unnecessary method invocations. +Piranha will delete a statement if it invokes one of these pre-defined methods with a stale flag as an argument. ## Example refactoring diff --git a/java/piranha/build.gradle b/java/piranha/build.gradle index f3b4d14f8..ac75d92a7 100644 --- a/java/piranha/build.gradle +++ b/java/piranha/build.gradle @@ -38,6 +38,7 @@ dependencies { testCompile deps.test.junit4 testCompile deps.test.daggerAnnotations testImplementation group: 'org.mockito', name: 'mockito-core', version: '2.1.0' + testImplementation group: 'org.easymock', name: 'easymock', version: '4.3' testCompile(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" } diff --git a/java/piranha/config/properties.json b/java/piranha/config/properties.json index 983765365..99131a45d 100644 --- a/java/piranha/config/properties.json +++ b/java/piranha/config/properties.json @@ -38,6 +38,32 @@ "argumentIndex": 0 } ], + "unnecessaryTestMethodProperties" : [ + { + "methodName": "when", + "argumentIndex": 0, + "receiverType":"org.mockito.Mockito", + "isStatic": "true" + }, + { + "methodName": "expect", + "argumentIndex": 0, + "receiverType":"org.easymock.EasyMock", + "isStatic": "true" + }, + { + "methodName": "assertTrue", + "argumentIndex": 0, + "receiverType":"org.junit.Assert", + "isStatic": "true" + }, + { + "methodName": "assertFalse", + "argumentIndex": 0, + "receiverType":"org.junit.Assert", + "isStatic": "true" + } + ], "linkURL": "", "annotations": [ "ToggleTesting", diff --git a/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java b/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java index b424f2c3f..77c069a24 100644 --- a/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java +++ b/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java @@ -1,4 +1,4 @@ -/** +/* * Copyright (c) 2019 Uber Technologies, Inc. * *

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file @@ -14,13 +14,8 @@ package com.uber.piranha; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; -import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.ALL; -import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.instanceMethod; -import static com.google.errorprone.matchers.Matchers.methodInvocation; -import static com.google.errorprone.matchers.Matchers.receiverOfInvocation; import static com.google.errorprone.matchers.Matchers.staticMethod; -import static com.google.errorprone.matchers.Matchers.toType; import com.facebook.infer.annotation.Initializer; import com.google.auto.service.AutoService; @@ -34,7 +29,6 @@ import com.google.errorprone.bugpatterns.BugChecker; 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.google.errorprone.util.FindIdentifiers; import com.sun.source.tree.AnnotationTree; @@ -78,8 +72,8 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -369,9 +363,9 @@ private API getXPAPI( for (PiranhaMethodRecord methodRecord : methodRecordsForName) { // when argumentIndex is specified, if mit's argument at argIndex doesn't match xpFlagName, // skip to next method property map - Optional optionalArgumentIdx = methodRecord.getArgumentIdx(); + OptionalInt optionalArgumentIdx = methodRecord.getArgumentIdx(); if (optionalArgumentIdx.isPresent()) { - int argumentIndex = optionalArgumentIdx.get().intValue(); + int argumentIndex = optionalArgumentIdx.getAsInt(); if (argumentIndex < mit.getArguments().size()) { ExpressionTree argTree = mit.getArguments().get(argumentIndex); Symbol argSym = ASTHelpers.getSymbol(argTree); @@ -705,30 +699,40 @@ private Description updateCode( return Description.NO_MATCH; } - private static final String MOCKITO_QN = "org.mockito.Mockito"; - private static final String MOCKITO_WHEN = "when"; - private static final Pattern MOCKITO_THEN = Pattern.compile("then\\W*\\w*"); - - private final Matcher MOCKITO_UNNECESSARY_MOCKING_PATTERN = - toType( - MethodInvocationTree.class, - allOf( - instanceMethod().anyClass().withNameMatching(MOCKITO_THEN), - receiverOfInvocation( - methodInvocation( - staticMethod().onClass(MOCKITO_QN).named(MOCKITO_WHEN), - ALL, - (argument, vs) -> { - Value value = evalExpr(argument, vs); - return value == Value.TRUE || value == Value.FALSE; - })))); - + /** + * This method picks up the unnecessary test method as configured in properties.json and converts + * them into a Error-prone AST Matcher and then deletes the containing AST statement. + * + * @param state The visitor state of the statement to be handled + * @return Suggestion Fix for deleting the statement containing a unnecessary test method + * invocation + */ private SuggestedFix.Builder handleSpecificAPIPatterns(VisitorState state) { - ExpressionStatementTree stmt = + MethodInvocationTree enclosingMit = + ASTHelpers.findEnclosingNode(state.getPath(), MethodInvocationTree.class); + + ExpressionStatementTree enclosingEst = ASTHelpers.findEnclosingNode(state.getPath(), ExpressionStatementTree.class); - if (stmt != null && MOCKITO_UNNECESSARY_MOCKING_PATTERN.matches(stmt.getExpression(), state)) { - endPos = state.getEndPosition(stmt); - return SuggestedFix.builder().delete(stmt); + + if (enclosingMit != null + && enclosingEst != null + && config + .getUnnecessaryTestMethodRecords() + .stream() + .map( + method -> + method.isStatic() + ? method + .getReceiverType() + .map(r -> staticMethod().onClass(r)) + .orElseGet(() -> staticMethod().anyClass()) + : method + .getReceiverType() + .map(r -> instanceMethod().onExactClass(r)) + .orElseGet(() -> instanceMethod().anyClass())) + .anyMatch(matcher -> matcher.matches(enclosingMit, state))) { + endPos = state.getEndPosition(enclosingMit); + return SuggestedFix.builder().delete(enclosingEst); } return SuggestedFix.builder(); } diff --git a/java/piranha/src/main/java/com/uber/piranha/config/Config.java b/java/piranha/src/main/java/com/uber/piranha/config/Config.java index 313b4163b..b0071e343 100644 --- a/java/piranha/src/main/java/com/uber/piranha/config/Config.java +++ b/java/piranha/src/main/java/com/uber/piranha/config/Config.java @@ -42,6 +42,7 @@ public final class Config { private static final String LINK_URL_KEY = "linkURL"; private static final String ANNOTATIONS_KEY = "annotations"; private static final String METHODS_KEY = "methodProperties"; + private static final String UNNECESSARY_TEST_METHOD_KEY = "unnecessaryTestMethodProperties"; private static final String CLEANUP_OPTS_KEY = "cleanupOptions"; private static final String ENUMS_KEY = "enumProperties"; @@ -64,14 +65,23 @@ public final class Config { private static final boolean DEFAULT_TESTS_CLEAN_BY_SETTERS_IGNORE_OTHERS = false; /** - * configMethodsMap is a map where key is method name and value is a list where each item in the - * list is a map that corresponds to each method property from properties.json. In most cases, the - * list would have only one element. But if someone reuses the same method name with different + * configMethodProperties is a map where key is method name and value is a list where each item in + * the list is a map that corresponds to each method property from properties.json. In most cases, + * the list would have only one element. But if someone reuses the same method name with different * returnType/receiverType/argumentIndex, the list would have each method property map as one * element. */ private final ImmutableMultimap configMethodProperties; + /** + * unnecessaryTestMethodProperties is a map where key is method name and value is a list where + * each item in the list is a map that corresponds to each test method property from + * properties.json. In most cases, the list would have only one element. But if someone reuses the + * same method name with different returnType/receiverType/argumentIndex, the list would have each + * method property map as one element. Often after the refactoring performed by Piranha, certain + * method invocations in test cases become unnecessary, need to be deleted. + */ + private final ImmutableMultimap unnecessaryTestMethodProperties; /** * configEnumProperties is a map where key is enum name and value is a list where each item in the * list is a map that corresponds to each enum property from properties.json. In most cases, the @@ -100,11 +110,13 @@ public final class Config { // in particular Config.fromJSONFile([properties.json]) private Config( ImmutableMultimap configMethodProperties, + ImmutableMultimap unnecessaryTestMethodProperties, ImmutableMultimap configEnumProperties, TestAnnotationResolver testAnnotationResolver, ImmutableMap cleanupOptions, String linkURL) { this.configMethodProperties = configMethodProperties; + this.unnecessaryTestMethodProperties = unnecessaryTestMethodProperties; this.configEnumProperties = configEnumProperties; this.testAnnotationResolver = testAnnotationResolver; this.cleanupOptions = cleanupOptions; @@ -123,6 +135,17 @@ public ImmutableCollection getMethodRecordsForName(String m ? configMethodProperties.get(methodName) : ImmutableSet.of(); } + /** + * Return all the configured unnecessary test methods (corresponding to the + * unnecessaryTestMethodProperties field) + * + * @return A collection of {@link MethodRecord} objects, representing each method definition in + * the piranha json configuration file (unnecessaryTestMethodProperties field) matching {@code + * methodName}. + */ + public ImmutableCollection getUnnecessaryTestMethodRecords() { + return unnecessaryTestMethodProperties.values(); + } /** * Returns whether any configuration enum records exist. Useful for skipping logic if enum @@ -266,7 +289,9 @@ public static Config fromJSONFile(String configFile, boolean isArgumentIndexOpti } String linkURL = DEFAULT_PIRANHA_URL; - ImmutableMultimap.Builder methodsBuilder = + ImmutableMultimap.Builder unnecessaryTestMethodsBuilder = + ImmutableMultimap.builder(); + ImmutableMultimap.Builder flagMethodsBuilder = ImmutableMultimap.builder(); ImmutableMultimap.Builder enumsBuilder = ImmutableMultimap.builder(); @@ -300,7 +325,15 @@ public static Config fromJSONFile(String configFile, boolean isArgumentIndexOpti PiranhaMethodRecord methodRecord = PiranhaMethodRecord.parseFromJSONPropertyEntryMap( methodProperty, isArgumentIndexOptional); - methodsBuilder.put(methodRecord.getMethodName(), methodRecord); + flagMethodsBuilder.put(methodRecord.getMethodName(), methodRecord); + } + if (propertiesJson.get(UNNECESSARY_TEST_METHOD_KEY) != null) { + for (Map methodProperty : + (List>) propertiesJson.get(UNNECESSARY_TEST_METHOD_KEY)) { + MethodRecord methodRecord = + MethodRecord.parseFromJSONPropertyEntryMap(methodProperty, isArgumentIndexOptional); + unnecessaryTestMethodsBuilder.put(methodRecord.getMethodName(), methodRecord); + } } } else { throw new PiranhaConfigurationException("methodProperties not found, required."); @@ -326,7 +359,8 @@ public static Config fromJSONFile(String configFile, boolean isArgumentIndexOpti }); } return new Config( - methodsBuilder.build(), + flagMethodsBuilder.build(), + unnecessaryTestMethodsBuilder.build(), enumsBuilder.build(), annotationResolverBuilder.build(), cleanupOptionsBuilder.build(), @@ -365,6 +399,7 @@ public static Config fromJSONFile(String configFile, boolean isArgumentIndexOpti */ public static Config emptyConfig() { return new Config( + ImmutableMultimap.of(), ImmutableMultimap.of(), ImmutableMultimap.of(), TestAnnotationResolver.builder().build(), diff --git a/java/piranha/src/main/java/com/uber/piranha/config/MethodRecord.java b/java/piranha/src/main/java/com/uber/piranha/config/MethodRecord.java new file mode 100644 index 000000000..dc0edf82d --- /dev/null +++ b/java/piranha/src/main/java/com/uber/piranha/config/MethodRecord.java @@ -0,0 +1,122 @@ +/** + * Copyright (c) 2021 Uber Technologies, Inc. + * + *

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.uber.piranha.config; + +import static com.uber.piranha.config.PiranhaRecord.getArgumentIndexFromMap; +import static com.uber.piranha.config.PiranhaRecord.getValueBooleanFromMap; +import static com.uber.piranha.config.PiranhaRecord.getValueStringFromMap; + +import java.util.Map; +import java.util.Optional; +import java.util.OptionalInt; +import javax.annotation.Nullable; + +/** A class representing a method configuration record from properties.json */ +public class MethodRecord { + + // Allowed fields for a method property in the config file. + // Entered under the top-level "methodProperties" in properties.json. + // By default, the flagType, methodName and argumentIndex fields are mandatory. + // The returnType and receiverType fields are optional. + protected static final String FLAG_TYPE_KEY = "flagType"; + protected static final String METHOD_NAME_KEY = "methodName"; + protected static final String ARGUMENT_INDEX_KEY = "argumentIndex"; + protected static final String RETURN_TYPE_STRING = "returnType"; + protected static final String RECEIVER_TYPE_STRING = "receiverType"; + protected static final String METHOD_IS_STATIC = "isStatic"; + + /** + * Holds the mapping of flagType string to API. Eg: "treated" -> API.IS_TREATED. Is initialized + * once and then accessed without updating. + */ + private final String methodName; + + @Nullable private final Integer argumentIdx; + @Nullable private final String receiverType; + @Nullable private final String returnType; + private final Boolean isStatic; + + MethodRecord( + String methodName, + @Nullable Integer argumentIdx, + @Nullable String receiverType, + @Nullable String returnType, + @Nullable Boolean isStatic) { + this.methodName = methodName; + this.argumentIdx = argumentIdx; + this.receiverType = receiverType; + this.returnType = returnType; + this.isStatic = isStatic != null && isStatic; + } + + public String getMethodName() { + return methodName; + } + + public OptionalInt getArgumentIdx() { + return Optional.ofNullable(argumentIdx).map(OptionalInt::of).orElseGet(OptionalInt::empty); + } + + public Optional getReceiverType() { + return Optional.ofNullable(receiverType); + } + + public Optional getReturnType() { + return Optional.ofNullable(returnType); + } + + /** + * Parse the entry for a single method from piranha.json that has been previously decoded into a + * map + * + * @param methodPropertyEntry The decoded json entry (as a Map of property names to values) + * @param isArgumentIndexOptional Whether argumentIdx should be treated as optional + * @return A PiranhaMethodRecord corresponding to the given map/json record. + * @throws PiranhaConfigurationException if there was any issue reading or parsing the + * configuration file. + */ + static MethodRecord parseFromJSONPropertyEntryMap( + Map methodPropertyEntry, boolean isArgumentIndexOptional) + throws PiranhaConfigurationException { + String methodName = getValueStringFromMap(methodPropertyEntry, METHOD_NAME_KEY); + Integer argumentIndexInteger = getArgumentIndexFromMap(methodPropertyEntry, ARGUMENT_INDEX_KEY); + if (methodName == null) { + throw new PiranhaConfigurationException( + "methodProperty is missing mandatory methodName field. Check:\n" + methodPropertyEntry); + } else if (!isArgumentIndexOptional && argumentIndexInteger == null) { + throw new PiranhaConfigurationException( + "methodProperty did not have argumentIndex. By default, Piranha requires an argument index for flag " + + "APIs, to which the flag name/symbol will be passed. This is to avoid over-deletion of all " + + "occurrences of a flag API method. If you are sure you want to delete all instances of the " + + "method below, consider using Piranha:ArgumentIndexOptional=true to override this behavior. " + + "Check:\n" + + methodPropertyEntry); + } else if (argumentIndexInteger != null && argumentIndexInteger < 0) { + throw new PiranhaConfigurationException( + "Invalid argumentIndex field. Arguments are zero indexed. Check:\n" + + methodPropertyEntry); + } + + return new MethodRecord( + methodName, + argumentIndexInteger, + getValueStringFromMap(methodPropertyEntry, RECEIVER_TYPE_STRING), + getValueStringFromMap(methodPropertyEntry, RETURN_TYPE_STRING), + getValueBooleanFromMap(methodPropertyEntry, METHOD_IS_STATIC)); + } + + public boolean isStatic() { + return isStatic; + } +} diff --git a/java/piranha/src/main/java/com/uber/piranha/config/PiranhaMethodRecord.java b/java/piranha/src/main/java/com/uber/piranha/config/PiranhaMethodRecord.java index adae61828..ae0f3e4f2 100644 --- a/java/piranha/src/main/java/com/uber/piranha/config/PiranhaMethodRecord.java +++ b/java/piranha/src/main/java/com/uber/piranha/config/PiranhaMethodRecord.java @@ -14,25 +14,16 @@ package com.uber.piranha.config; import static com.uber.piranha.config.PiranhaRecord.getArgumentIndexFromMap; +import static com.uber.piranha.config.PiranhaRecord.getValueBooleanFromMap; import static com.uber.piranha.config.PiranhaRecord.getValueStringFromMap; import com.google.common.collect.ImmutableMap; import com.uber.piranha.XPFlagCleaner; import java.util.Map; -import java.util.Optional; +import javax.annotation.Nullable; /** A class representing a method configuration record from properties.json */ -public final class PiranhaMethodRecord { - - // Allowed fields for a method property in the config file. - // Entered under the top-level "methodProperties" in properties.json. - // By default, the flagType, methodName and argumentIndex fields are mandatory. - // The returnType and receiverType fields are optional. - private static final String FLAG_TYPE_KEY = "flagType"; - private static final String METHOD_NAME_KEY = "methodName"; - private static final String ARGUMENT_INDEX_KEY = "argumentIndex"; - private static final String RETURN_TYPE_STRING = "returnType"; - private static final String RECEIVER_TYPE_STRING = "receiverType"; +public final class PiranhaMethodRecord extends MethodRecord { /** * Holds the mapping of flagType string to API. Eg: "treated" -> API.IS_TREATED. Is initialized @@ -41,45 +32,23 @@ public final class PiranhaMethodRecord { private static final ImmutableMap flagTypeToAPIMap = initializeFlagTypeToAPIMap(); - private final String methodName; private final XPFlagCleaner.API apiType; - private final Optional argumentIdx; - private final Optional receiverType; - private final Optional returnType; PiranhaMethodRecord( String methodName, String flagTypeString, - Optional argumentIdx, - Optional receiverType, - Optional returnType) { - this.methodName = methodName; + @Nullable Integer argumentIdx, + @Nullable String receiverType, + @Nullable String returnType, + @Nullable Boolean isStatic) { + super(methodName, argumentIdx, receiverType, returnType, isStatic); this.apiType = flagTypeToAPIMap.getOrDefault(flagTypeString, XPFlagCleaner.API.UNKNOWN); - this.argumentIdx = argumentIdx; - this.receiverType = receiverType; - this.returnType = returnType; - } - - public String getMethodName() { - return methodName; } public XPFlagCleaner.API getApiType() { return apiType; } - public Optional getArgumentIdx() { - return argumentIdx; - } - - public Optional getReceiverType() { - return receiverType; - } - - public Optional getReturnType() { - return returnType; - } - private static ImmutableMap initializeFlagTypeToAPIMap() { ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); builder.put("treated", XPFlagCleaner.API.IS_TREATED); @@ -121,7 +90,7 @@ static PiranhaMethodRecord parseFromJSONPropertyEntryMap( + "method below, consider using Piranha:ArgumentIndexOptional=true to override this behavior. " + "Check:\n" + methodPropertyEntry); - } else if (argumentIndexInteger != null && argumentIndexInteger.intValue() < 0) { + } else if (argumentIndexInteger != null && argumentIndexInteger < 0) { throw new PiranhaConfigurationException( "Invalid argumentIndex field. Arguments are zero indexed. Check:\n" + methodPropertyEntry); @@ -130,8 +99,9 @@ static PiranhaMethodRecord parseFromJSONPropertyEntryMap( return new PiranhaMethodRecord( methodName, flagType, - Optional.ofNullable(argumentIndexInteger), - Optional.ofNullable(getValueStringFromMap(methodPropertyEntry, RECEIVER_TYPE_STRING)), - Optional.ofNullable(getValueStringFromMap(methodPropertyEntry, RETURN_TYPE_STRING))); + argumentIndexInteger, + getValueStringFromMap(methodPropertyEntry, RECEIVER_TYPE_STRING), + getValueStringFromMap(methodPropertyEntry, RETURN_TYPE_STRING), + getValueBooleanFromMap(methodPropertyEntry, METHOD_IS_STATIC)); } } diff --git a/java/piranha/src/main/java/com/uber/piranha/config/PiranhaRecord.java b/java/piranha/src/main/java/com/uber/piranha/config/PiranhaRecord.java index 55b5d90a0..baf882e2b 100644 --- a/java/piranha/src/main/java/com/uber/piranha/config/PiranhaRecord.java +++ b/java/piranha/src/main/java/com/uber/piranha/config/PiranhaRecord.java @@ -29,8 +29,9 @@ public class PiranhaRecord { @Nullable static String getValueStringFromMap(Map map, String key) { Object value = map.get(key); - if (value instanceof String && !value.equals("")) { - return String.valueOf(value); + if (value instanceof String) { + String valueStr = String.valueOf(value); + if (!valueStr.isEmpty()) return valueStr; } return null; } @@ -53,4 +54,21 @@ static Integer getArgumentIndexFromMap(Map map, String argumentI } return null; } + + /** + * Utility method. Checks whether the value associated to a given map and given key is a non-empty + * string. + * + * @param map - map corresponding to a method property + * @param key - key to check the corresponding value + * @return the corresponding boolean value + */ + @Nullable + static Boolean getValueBooleanFromMap(Map map, String key) { + Object value = map.get(key); + if (value instanceof String && !value.equals("")) { + return Boolean.parseBoolean(String.valueOf(value)); + } + return null; + } } diff --git a/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java b/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java index 1f4477221..9271f2b1c 100644 --- a/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java +++ b/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java @@ -500,6 +500,203 @@ public void testIgnoresPrefixMatchFlag() throws IOException { .doTest(); } + @Test + public void testRemoveSpecificAPIpatternsMockito() throws IOException { + + ErrorProneFlags.Builder b = ErrorProneFlags.builder(); + b.putFlag("Piranha:FlagName", "STALE_FLAG"); + b.putFlag("Piranha:IsTreated", "false"); + b.putFlag("Piranha:Config", "config/properties.json"); + + BugCheckerRefactoringTestHelper bcr = + BugCheckerRefactoringTestHelper.newInstance(new XPFlagCleaner(b.build()), getClass()); + + bcr = bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath()); + bcr = PiranhaTestingHelpers.addHelperClasses(bcr); + bcr.addInputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import static org.mockito.Mockito.when;", + "import org.mockito.invocation.InvocationOnMock;", + "import org.mockito.stubbing.Answer;", + "import org.mockito.stubbing.OngoingStubbing;", + "class MockitoTest {", + " private XPTest experimentation;", + " public void evaluate() {", + " Answer ans = new Answer() {\n" + + " public Integer answer(InvocationOnMock invocation) throws Throwable {\n" + + " return (Integer) invocation.getArguments()[0];\n" + + " }};", + " when(experimentation.isToggleDisabled(\"STALE_FLAG\")).thenReturn(false);", + " when(experimentation.isToggleEnabled(\"STALE_FLAG\")).thenThrow(new RuntimeException());", + " when(experimentation.isToggleDisabled(\"STALE_FLAG\")).thenCallRealMethod();", + " when(experimentation.isToggleEnabled(\"STALE_FLAG\")).then(ans);", + " boolean b1 = someWrapper(when(experimentation.isToggleDisabled(\"STALE_FLAG\")).thenCallRealMethod());", + " boolean b2 = someWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\")).thenCallRealMethod());", + " someWhenWrapper(when(experimentation.isToggleDisabled(\"STALE_FLAG\"))).thenCallRealMethod();", + " someWhenWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\"))).thenCallRealMethod();", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenReturn(false);", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenThrow(new RuntimeException());", + " when(experimentation.isToggleDisabled(\"OTHER_FLAG\")).thenCallRealMethod();", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).then(ans);", + " when(foobar()).thenReturn(false);", + " when(foobar()).thenAnswer(ans);", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + " public OngoingStubbing someWhenWrapper(OngoingStubbing x) { return x;}", + "}") + .addOutputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import static org.mockito.Mockito.when;", + "import org.mockito.invocation.InvocationOnMock;", + "import org.mockito.stubbing.Answer;", + "import org.mockito.stubbing.OngoingStubbing;", + "class MockitoTest {", + " private XPTest experimentation;", + " public void evaluate() {", + " Answer ans = new Answer() {\n" + + " public Integer answer(InvocationOnMock invocation) throws Throwable {\n" + + " return (Integer) invocation.getArguments()[0];\n" + + " }};", + "", + "", + "", + "", + "", + " boolean b1 = someWrapper(when(true).thenCallRealMethod());", + " boolean b2 = someWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\")).thenCallRealMethod());", + "", + " someWhenWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\"))).thenCallRealMethod();", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenReturn(false);", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenThrow(new RuntimeException());", + " when(experimentation.isToggleDisabled(\"OTHER_FLAG\")).thenCallRealMethod();", + " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).then(ans);", + " when(foobar()).thenReturn(false);", + " when(foobar()).thenAnswer(ans);", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + " public OngoingStubbing someWhenWrapper(OngoingStubbing x) { return x;}", + "}") + .doTest(); + } + + @Test + public void testRemoveSpecificAPIpatternsEasyMock() throws IOException { + + ErrorProneFlags.Builder b = ErrorProneFlags.builder(); + b.putFlag("Piranha:FlagName", "STALE_FLAG"); + b.putFlag("Piranha:IsTreated", "false"); + b.putFlag("Piranha:Config", "config/properties.json"); + + BugCheckerRefactoringTestHelper bcr = + BugCheckerRefactoringTestHelper.newInstance(new XPFlagCleaner(b.build()), getClass()); + + bcr = bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath()); + bcr = PiranhaTestingHelpers.addHelperClasses(bcr); + bcr.addInputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import org.easymock.EasyMock;", + "class EasyMockTest {", + " private XPTest experimentation;", + " public void evaluate() {", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true);", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).asStub();", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(foobar()).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + "}") + .addOutputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import org.easymock.EasyMock;", + "class EasyMockTest {", + " private XPTest experimentation;", + " public void evaluate() {", + "", + "", + "", + "", + "", + "", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(foobar()).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + "}") + .doTest(); + } + + @Test + public void testRemoveSpecificAPIpatternsJUnit() throws IOException { + + ErrorProneFlags.Builder b = ErrorProneFlags.builder(); + b.putFlag("Piranha:FlagName", "STALE_FLAG"); + b.putFlag("Piranha:IsTreated", "false"); + b.putFlag("Piranha:Config", "config/properties.json"); + + BugCheckerRefactoringTestHelper bcr = + BugCheckerRefactoringTestHelper.newInstance(new XPFlagCleaner(b.build()), getClass()); + + bcr = bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath()); + bcr = PiranhaTestingHelpers.addHelperClasses(bcr); + bcr.addInputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import static org.junit.Assert.assertFalse;", + "import static org.junit.Assert.assertTrue;", + "class JUnitTest {", + " private XPTest experimentation;", + " public void evaluate() {", + " assertFalse(experimentation.isToggleDisabled(\"STALE_FLAG\"));", + " assertTrue(experimentation.isToggleEnabled(\"STALE_FLAG\"));", + " assertFalse(experimentation.isToggleEnabled(\"OTHER_FLAG\"));", + " assertTrue(experimentation.isToggleDisabled(\"OTHER_FLAG\"));", + " assertTrue(foobar());", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + "}") + .addOutputLines( + "XPFlagCleanerSinglePositiveCase.java", + "package com.uber.piranha;", + "import static org.junit.Assert.assertFalse;", + "import static org.junit.Assert.assertTrue;", + "class JUnitTest {", + " private XPTest experimentation;", + " public void evaluate() {", + "", + "", + " assertFalse(experimentation.isToggleEnabled(\"OTHER_FLAG\"));", + " assertTrue(experimentation.isToggleDisabled(\"OTHER_FLAG\"));", + " assertTrue(foobar());", + " }", + " public boolean foobar() { return true;}", + " public boolean someWrapper(Object obj) { return true;}", + "}") + .doTest(); + } + @Test public void testRemoveSpecificAPIpatterns() throws IOException { @@ -520,9 +717,13 @@ public void testRemoveSpecificAPIpatterns() throws IOException { "import org.mockito.invocation.InvocationOnMock;", "import org.mockito.stubbing.Answer;", "import org.mockito.stubbing.OngoingStubbing;", + "import org.easymock.EasyMock;", + "import static org.junit.Assert.assertFalse;", + "import static org.junit.Assert.assertTrue;", "class MockitoTest {", " private XPTest experimentation;", " public void evaluate() {", + "// Mockito Test Scenarios", " Answer ans = new Answer() {\n" + " public Integer answer(InvocationOnMock invocation) throws Throwable {\n" + " return (Integer) invocation.getArguments()[0];\n" @@ -541,6 +742,27 @@ public void testRemoveSpecificAPIpatterns() throws IOException { " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).then(ans);", " when(foobar()).thenReturn(false);", " when(foobar()).thenAnswer(ans);", + "// Easymock Test scenarios", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"STALE_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).andReturn(true);", + " EasyMock.expect(experimentation.isToggleEnabled(\"STALE_FLAG\")).asStub();", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(foobar()).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + "// JUnit Assert Test scearios", + " assertFalse(experimentation.isToggleDisabled(\"STALE_FLAG\"));", + " assertTrue(experimentation.isToggleEnabled(\"STALE_FLAG\"));", + " assertFalse(experimentation.isToggleEnabled(\"OTHER_FLAG\"));", + " assertTrue(experimentation.isToggleDisabled(\"OTHER_FLAG\"));", + " assertTrue(foobar());", " }", " public boolean foobar() { return true;}", " public boolean someWrapper(Object obj) { return true;}", @@ -553,9 +775,13 @@ public void testRemoveSpecificAPIpatterns() throws IOException { "import org.mockito.invocation.InvocationOnMock;", "import org.mockito.stubbing.Answer;", "import org.mockito.stubbing.OngoingStubbing;", + "import org.easymock.EasyMock;", + "import static org.junit.Assert.assertFalse;", + "import static org.junit.Assert.assertTrue;", "class MockitoTest {", " private XPTest experimentation;", " public void evaluate() {", + "// Mockito Test Scenarios", " Answer ans = new Answer() {\n" + " public Integer answer(InvocationOnMock invocation) throws Throwable {\n" + " return (Integer) invocation.getArguments()[0];\n" @@ -564,9 +790,10 @@ public void testRemoveSpecificAPIpatterns() throws IOException { "", "", "", + "", " boolean b1 = someWrapper(when(true).thenCallRealMethod());", " boolean b2 = someWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\")).thenCallRealMethod());", - " someWhenWrapper(when(true)).thenCallRealMethod();", + "", " someWhenWrapper(when(experimentation.isToggleDisabled(\"OTHER_FLAG\"))).thenCallRealMethod();", " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenReturn(false);", " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).thenThrow(new RuntimeException());", @@ -574,12 +801,30 @@ public void testRemoveSpecificAPIpatterns() throws IOException { " when(experimentation.isToggleEnabled(\"OTHER_FLAG\")).then(ans);", " when(foobar()).thenReturn(false);", " when(foobar()).thenAnswer(ans);", + "// Easymock Test scenarios", + "", + "", + "", + "", + "", + "", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + " EasyMock.expect(foobar()).andReturn(true).times(5);", + " EasyMock.expect(experimentation.isToggleEnabled(\"OTHER_FLAG\")).andReturn(true).times(5, 6);", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).once();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).atLeastOnce();", + " EasyMock.expect(experimentation.isToggleDisabled(\"OTHER_FLAG\")).andReturn(true).anyTimes();", + "// JUnit Assert Test scearios", + "", + "", + " assertFalse(experimentation.isToggleEnabled(\"OTHER_FLAG\"));", + " assertTrue(experimentation.isToggleDisabled(\"OTHER_FLAG\"));", + " assertTrue(foobar());", " }", " public boolean foobar() { return true;}", " public boolean someWrapper(Object obj) { return true;}", " public OngoingStubbing someWhenWrapper(OngoingStubbing x) { return x;}", "}") .doTest(); - // OngoingStubbing w = when(true).the; } }