From 03fd050158042d9f079779a3e1dfead656d38e7f Mon Sep 17 00:00:00 2001 From: Sathyamurthy Date: Thu, 29 Oct 2020 22:22:00 +0530 Subject: [PATCH] adding support for mocks in test cleanup --- java/piranha/build.gradle | 1 + java/piranha/config/properties.json | 7 +++ .../java/com/uber/piranha/XPFlagCleaner.java | 57 +++++++++++++++++++ .../com/uber/piranha/XPFlagCleanerTest.java | 1 + 4 files changed, 66 insertions(+) diff --git a/java/piranha/build.gradle b/java/piranha/build.gradle index 21ed287e7..da1844044 100644 --- a/java/piranha/build.gradle +++ b/java/piranha/build.gradle @@ -35,6 +35,7 @@ dependencies { implementation deps.build.googleJsonSimple testCompile deps.test.junit4 + testCompile 'org.mockito:mockito-core:2.7.22' testCompile deps.test.daggerAnnotations testCompile(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" diff --git a/java/piranha/config/properties.json b/java/piranha/config/properties.json index 4f2db06a7..068f97953 100644 --- a/java/piranha/config/properties.json +++ b/java/piranha/config/properties.json @@ -41,5 +41,12 @@ "linkURL": "", "annotations": [ "ToggleTesting" + ], + "testMethodProperties" : [ + { + "methodName": "when", + "flagType": "empty", + "argumentIndex": 0 + } ] } 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 da48ebf22..6906f04d0 100644 --- a/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java +++ b/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java @@ -69,6 +69,7 @@ import java.util.Set; import javax.annotation.Nullable; import javax.lang.model.element.ElementKind; + import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; import org.json.simple.parser.ParseException; @@ -186,6 +187,8 @@ enum API { */ private ImmutableMultimap configMethodProperties; + private ImmutableMultimap configTestMethodProperties; + private final HashSet handledAnnotations = new HashSet<>(); private String linkURL = PIRANHA_DEFAULT_URL; @@ -221,6 +224,7 @@ void init(ErrorProneFlags flags) throws PiranhaConfigurationException { // No configuration present at all, disable Piranha checker disabled = true; configMethodProperties = ImmutableMultimap.of(); + configTestMethodProperties = ImmutableMultimap.of(); return; } @@ -274,6 +278,19 @@ void init(ErrorProneFlags flags) throws PiranhaConfigurationException { builder.put(methodRecord.getMethodName(), methodRecord); } configMethodProperties = builder.build(); + + // Add test method configuration + Set> testMethodProperties = new HashSet<>(); + if (propertiesJson.get("testMethodProperties") != null) { + testMethodProperties.addAll((List>) propertiesJson.get("testMethodProperties")); + } + ImmutableMultimap.Builder testPropertiesBuilder = new ImmutableMultimap.Builder<>(); + for (Map testMethodProperty : testMethodProperties) { + PiranhaMethodRecord methodRecord = PiranhaMethodRecord.parseFromJSONPropertyEntryMap(testMethodProperty, isArgumentIndexOptional); + testPropertiesBuilder.put(methodRecord.getMethodName(), methodRecord); + } + configTestMethodProperties = testPropertiesBuilder.build(); + } catch (IOException fnfe) { throw new PiranhaConfigurationException( "Error reading config file " + Paths.get(configFile).toAbsolutePath() + " : " + fnfe); @@ -293,6 +310,7 @@ void init(ErrorProneFlags flags) throws PiranhaConfigurationException { // Already in the right format, re-throw throw pce; } catch (Exception e) { + e.getStackTrace(); throw new PiranhaConfigurationException("Some other exception thrown while parsing config"); } } else { @@ -881,6 +899,45 @@ private boolean isCheckedXPFlagName(ExpressionTree tree) { public Description matchMethod(MethodTree tree, VisitorState state) { if (disabled) return Description.NO_MATCH; + if (!configTestMethodProperties.isEmpty() && tree != null && tree.getBody() != null && tree.getBody().getStatements() != null) { + List bt = tree.getBody().getStatements(); + for (StatementTree st : bt) { + if (st != null && st.getKind().equals(Kind.EXPRESSION_STATEMENT)) { + ExpressionStatementTree est = (ExpressionStatementTree) st; + if (est != null && est.getExpression() instanceof MethodInvocationTree) { + MethodInvocationTree mit = (MethodInvocationTree) est.getExpression(); + if (mit != null && mit.getKind().equals(Kind.METHOD_INVOCATION)) { + ExpressionTree exp = ((MemberSelectTree) mit.getMethodSelect()).getExpression(); + if (exp != null && exp.getKind().equals(Kind.METHOD_INVOCATION)) { + ExpressionTree exp2 = ((MethodInvocationTree)exp).getMethodSelect(); + if (exp2 != null && exp2.getKind().equals(Kind.MEMBER_SELECT)) { + String methodName = ((MemberSelectTree) exp2).getIdentifier().toString(); + if (configTestMethodProperties.containsKey(methodName)) { + ImmutableCollection methodRecords = configTestMethodProperties.get(methodName); + for (PiranhaMethodRecord methodRecord : methodRecords) { + Optional optionalArgumentIdx = methodRecord.getArgumentIdx(); + if (optionalArgumentIdx.isPresent()) { + Value value = evalExpr(((MethodInvocationTree) exp).getArguments().get(optionalArgumentIdx.get()), state); + if (value == Value.TRUE || value == Value.FALSE) { + Description.Builder builder = buildDescription(est); + SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); + fixBuilder.delete(est); + decrementAllSymbolUsages(est, state, fixBuilder); + builder.addFix(fixBuilder.build()); + endPos = state.getEndPosition(est); + return builder.build(); + } + } + } + } + } + } + } + } + } + } + } + for (String name : handledAnnotations) { AnnotationTree at = ASTHelpers.getAnnotationWithSimpleName(tree.getModifiers().getAnnotations(), name); diff --git a/java/piranha/src/test/java/com/uber/piranha/XPFlagCleanerTest.java b/java/piranha/src/test/java/com/uber/piranha/XPFlagCleanerTest.java index fd67e4fba..70adf13cf 100644 --- a/java/piranha/src/test/java/com/uber/piranha/XPFlagCleanerTest.java +++ b/java/piranha/src/test/java/com/uber/piranha/XPFlagCleanerTest.java @@ -137,6 +137,7 @@ public void positiveSpecificTreatmentGroup() throws IOException { " private XPTest experimentation;", " public String groupToString() {", " // BUG: Diagnostic contains: Cleans stale XP flags", + " org.mockito.Mockito.when(experimentation.isToggleDisabled(STALE_FLAG)).thenReturn(false);", " if (experimentation.isToggleDisabled(STALE_FLAG)) { return \"\"; }", " else if (experimentation.isToggleInGroup(", " STALE_FLAG,GROUP_A)) { ",