Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PiranhaJava // config: add returnType, receiverType and argumentIndex; move to properties.json #39

Merged
merged 11 commits into from Jun 15, 2020
79 changes: 59 additions & 20 deletions java/README.md
Expand Up @@ -38,7 +38,7 @@ tasks.withType(JavaCompile) {
// and final treatment condition.
options.errorprone.errorproneArgs << "-XepOpt:Piranha:FlagName=SAMPLE_STALE_FLAG"
options.errorprone.errorproneArgs << "-XepOpt:Piranha:IsTreated=true"
options.errorprone.errorproneArgs << "-XepOpt:Piranha:Config=config/piranha.properties"
options.errorprone.errorproneArgs << "-XepOpt:Piranha:Config=config/properties.json"
}
```

Expand All @@ -49,17 +49,34 @@ In the `tasks.withType(JavaCompile)` section, we pass some configuration options
The properties file has the following template:

```
treatedMethods=treated,flagEnabled
controlMethods=flagDisabled
emptyMethods=enableFlag,disableFlag
treatmentGroupMethods=isToggleInGroup
annotations=FlagTesting
linkURL=<provide_your_url>
{
"methodProperties":
[
{
"methodName": "isToggleEnabled",
"flagType": "treated",
"returnType": "boolean",
"receiverType": "com.uber.piranha.XPTest",
"argumentIndex": 0
},
...
],
"linkURL": "<provide_your_url>",
"annotations": ["ToggleTesting"]
}
```

The `treatedMethods` are the APIs which correspond to the treatment behavior of the flag, `controlMethods` correspond to the control behavior of the flag. In the above example, the API `flagEnabled` corresponds to treatment behavior. Hence, when `IsTreated` flag is set to `true`, `flagEnabled(SAMPLE_STALE_FLAG)` will be evaluated to `true`. Similarly, `flagDisabled(SAMPLE_STALE_FLAG)` which corresponds to the control behavior will evaluate to `false`.
The required top-level field is `methodProperties`.
Within that, there is an array of JSON objects, having the required fields `methodName`, `flagType` and `argumentIndex`.
The optional fields are `returnType`, `receiverType`.

The `flagType` with `treated` are the APIs which correspond to the treatment behavior of the flag, `control` correspond to the control behavior of the flag. In the above example, the API `flagEnabled` corresponds to treatment behavior. Hence, when the `IsTreated` Piranha argument is set to `true`, `flagEnabled(SAMPLE_STALE_FLAG)` will be evaluated to `true`. Similarly, `flagDisabled(SAMPLE_STALE_FLAG)` which corresponds to the control behavior will evaluate to `false`.
The `flagType` with `empty` specifies the APIs which need to be discarded from the code. For example, if `enableFlag` is listed as a method in properties.json, with `flagType=empty`, a statement `enableFlag(SAMPLE_STALE_FLAG);` will be deleted from the code.

The `emptyMethods` specify the APIs which need to be discarded from the code. For example, a statement `enableFlag(SAMPLE_STALE_FLAG);` will be deleted from the code.
The `argumentIndex` field specifies where to look for the flag name (given by `-XepOpt:Piranha:FlagName`) in the method's arguments. We follow 0 based indexing.
If your toggle methods take no arguments, or if you want to delete all occurrences of a given `methodName` irrespective of their arguments, you can set the `Piranha:ArgumentIndexOptional` to `true` to make specifying `argumentIndex` optional.

For `returnType` and `receiverType`, types should be written as `boolean` or `void` for primitive types, and fully qualified for custom defined types. eg: `com.uber.piranha.XPTest` or `java.lang.String` (not case-sensitive)

The `annotations` specify the annotations used (e.g., in unit testing) to determine treatment or control behavior. For example:

Expand Down Expand Up @@ -106,16 +123,38 @@ and the following arguments to Piranha
```
options.errorprone.errorproneArgs << "-XepOpt:Piranha:FlagName=SAMPLE_STALE_FLAG"
options.errorprone.errorproneArgs << "-XepOpt:Piranha:IsTreated=true"
options.errorprone.errorproneArgs << "-XepOpt:Piranha:Config=config/piranha.properties
```
where `piranha.properties` contains the following,

```
treatedMethods=treated,flagEnabled
controlMethods=flagDisabled
emptyMethods=enableFlag,disableFlag
annotations=FlagTesting
linkURL=<provide_your_url>
options.errorprone.errorproneArgs << "-XepOpt:Piranha:Config=config/properties.json
```
where `properties.json` contains the following,

```
{
"methodProperties":
[
{
"methodName": "flagEnabled",
"flagType": "treated",
"argumentIndex": 0
},
{
"methodName": "flagDisabled",
"flagType": "control",
"argumentIndex": 0
},
{
"methodName": "enableFlag",
"flagType": "empty",
"argumentIndex": 0
},
{
"methodName": "disableFlag",
"flagType": "empty",
"argumentIndex": 0
}
],
"linkURL": "<provide_your_url>",
"annotations": ["FlagTesting"]
}
```

the refactored output will be
Expand Down Expand Up @@ -168,7 +207,7 @@ This example is present in the [sample](https://github.com/uber/piranha/tree/mas
<showWarnings>true</showWarnings>
<compilerArgs>
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne -Xep:Piranha:WARN -XepPatchChecks:Piranha -XepPatchLocation:IN_PLACE -XepOpt:Piranha:FlagName=SAMPLE_STALE_FLAG -XepOpt:Piranha:IsTreated=true -XepOpt:Piranha:Config=config/piranha.properties</arg>
<arg>-Xplugin:ErrorProne -Xep:Piranha:WARN -XepPatchChecks:Piranha -XepPatchLocation:IN_PLACE -XepOpt:Piranha:FlagName=SAMPLE_STALE_FLAG -XepOpt:Piranha:IsTreated=true -XepOpt:Piranha:Config=config/properties.json</arg>
</compilerArgs>
<annotationProcessorPaths>
<path>
Expand Down
1 change: 1 addition & 0 deletions java/gradle/dependencies.gradle
Expand Up @@ -29,6 +29,7 @@ def build = [
errorProneTestHelpers : "com.google.errorprone:error_prone_test_helpers:${versions.errorProne}",
gradleErrorPronePlugin : "net.ltgt.gradle:gradle-errorprone-plugin:0.0.16",
jsr305Annotations : "com.google.code.findbugs:jsr305:3.0.2",
googleJsonSimple : "com.googlecode.json-simple:json-simple:1.1.1",
]

def test = [
Expand Down
1 change: 1 addition & 0 deletions java/piranha/build.gradle
Expand Up @@ -30,6 +30,7 @@ dependencies {
annotationProcessor deps.apt.autoService

compileOnly deps.build.errorProneCheckApi
implementation deps.build.googleJsonSimple
pranavsb marked this conversation as resolved.
Show resolved Hide resolved

testCompile deps.test.junit4
testCompile deps.test.daggerAnnotations
Expand Down
5 changes: 0 additions & 5 deletions java/piranha/config/fewer-piranha.properties

This file was deleted.

6 changes: 0 additions & 6 deletions java/piranha/config/piranha.properties

This file was deleted.

45 changes: 45 additions & 0 deletions java/piranha/config/properties.json
@@ -0,0 +1,45 @@
{
"methodProperties": [
{
"methodName": "isToggleDisabled",
"flagType": "control",
"argumentIndex": 0
},
{
"methodName": "isFlagTreated",
"flagType": "treated",
"returnType": "boolean",
"argumentIndex": 0
},
{
"methodName": "isToggleEnabled",
"flagType": "treated",
"returnType": "boolean",
"argumentIndex": 0
},
{
"methodName": "putToggleEnabled",
"flagType": "empty",
"argumentIndex": 0
},
{
"methodName": "putToggleDisabled",
"flagType": "empty",
"argumentIndex": 0
},
{
"methodName": "includeEvent",
"flagType": "empty",
"argumentIndex": 0
},
{
"methodName": "isToggleInGroup",
"flagType": "treatmentGroup",
"argumentIndex": 0
}
],
"linkURL": "<provide_your_url>",
"annotations": [
"ToggleTesting"
]
}