uber / piranha

A tool for refactoring code related to feature flag APIs
Apache License 2.0
2.3k stars 198 forks source link

Seeking help on feature cleanup #693

Closed rajeshchinnam closed 1 month ago

rajeshchinnam commented 2 months ago

Hi Team,

Thanks in advance We have features defined in enum file eg:

FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY("feature.2024.09.gpd.35102-warranty-sequence-number")

and we are using it

public String get(String requestNumberPrefix, String request) { if (Boolean.parseBoolean(tenantConfiguration.get(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY))) { log.info("request number prefix is {}", requestNumberPrefix); Long nextSequenceId = getNextSequenceIdAndSave(requestNumberPrefix, request); String sequenceNumber = String.format(REQUEST_NUMBER_FORMAT, nextSequenceId); return requestNumberPrefix + sequenceNumber; } else { return generateRandomNumber(); } }

now this feature FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY is enabled I want to clean up this feature

I have below rules created

remove_feature_toggle_if_else = Rule( name="remove_feature_toggle_if_else", query="""( (if_statement condition: (identifier) @id consequence: (block) @if_block alternative: (block) @else_block ) @if_stmt (#eq? @id "FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY") )""", # Tree-sitter query to find the if-else block checking the feature toggle replace_node="else_statement", replace="", # Remove the entire if-else block is_seed_rule=True )

Set up Piranha arguments

piranha_arguments = PiranhaArguments( code_snippet=path_to_codebase, language="java", rule_graph=RuleGraph(rules=[remove_feature_toggle_if_else], edges=[]) )

Execute Piranha

Execute Piranha

try: piranha_summary = execute_piranha(piranha_arguments)

# Print the summary to see the changes
print("Piranha Summary:", piranha_summary)

# Assuming piranha_summary contains information about modified files
for file_info in piranha_summary.modified_files:
    file_path = file_info.path
    with open(file_path, 'r') as file:
        print(f"\nModified Code in {file_path}:\n")
        print(file.read())

except Exception as e: print("Error executing Piranha:", e)

when running python run_piranha.py I am getting below error

thread '' panicked at src\models\source_code_unit.rs:344:5: Produced syntactically incorrect source code

I could not able to find what is the issue here and any help is much appreciated

danieltrt commented 2 months ago

Can you post what code piranha produced by enabling debug mode? Looking at your rule, I can see a potential problem

remove_feature_toggle_if_else = Rule(
name="remove_feature_toggle_if_else",
query="""(
(if_statement
condition: (identifier) @id
consequence: (block) @if_block
alternative: (block) @else_block
) @if_stmt
(#eq? @id "FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY")
)""", 
replace_node="else_statement", # <——— this is not defined, you probably meant ‘else_block’
replace=""
is_seed_rule=True
)

I suggest you use concrete syntax over tree sitter queries for simpler rules

remove_feature_toggle_if_else = Rule(
name="remove_feature_toggle_if_else",
query="""cs if(FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY) :[if_block] else :[else_block]""", 
replace_node="*", # everything 
replace=":[if_block]"
is_seed_rule=True
)

Take a look at our demo examples to get the gist of how it works (e.g., https://github.com/uber/piranha/blob/master/demo/stale_feature_flag_cleanup_demos_using_py_api.py)

rajeshchinnam commented 2 months ago

@danieltrt Thanks for your help

I have below code

from os.path import join, dirname, getmtime, exists from polyglot_piranha import Rule, RuleGraph, execute_piranha, PiranhaArguments import logging from logging import info

feature_flag_dir = dirname(file)

def run_java_ff_demo(): info("Running the stale feature flag cleanup demo for Java")

directory_path = feature_flag_dir

remove_feature_toggle_if_else = Rule(
name="remove_feature_toggle_if_else",
query="cs if(isToggleEnabled(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)) :[if_block] else :[else_block]", 
replace_node="*",  # everything 
replace=":[if_block]",
is_seed_rule=True

)

# Rule to remove empty braces
remove_unnecessary_braces = Rule(
    name="remove_unnecessary_braces",
    query="cs { :[content] }",
    replace_node="*",
    replace=":[content]",
    is_seed_rule=False

)

args = PiranhaArguments(
    "java",
    paths_to_codebase=[directory_path],
    rule_graph=RuleGraph(rules=[remove_feature_toggle_if_else, remove_unnecessary_braces], edges=[])
)

output_summaries = execute_piranha(args)
print("Output Summaries:", output_summaries)

run_java_ff_demo() print("Completed running the stale feature flag cleanup demo")

code before

public void testFeatureToggle() { if (isToggleEnabled(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)) { // Code to keep System.out.println("Feature is enabled"); } else { // Code to remove System.out.println("Feature is disabled"); } }

after

public void testFeatureToggle() { { // Code to keep System.out.println("Feature is enabled"); } } I want to remove extra braces can you please help

danieltrt commented 2 months ago

To get rid of the extra braces, you need to unroll the pattern a bit more. Try replacing the query with this new one:

Rule(...,
query=""" cs 
if(isToggleEnabled(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)) {
    :[if_block+] 
} else {
    :[else_block+]
}", 
...)

Alternatively, simply replace with true, and add an edge to boolean_literal_cleanup (like in the landing page example)

# Define the rule to replace the method call
r1 = Rule(
    name="replace_method",
    query="cs isToggleEnabled(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)", # cs indicates we are using concrete syntax
    replace_node="*",
    replace="true",
    is_seed_rule=True
)

# Define the edges for the rule graph. 
# In this case, boolean_literal_cleanup is already defined for java [see src/cleanup_rules]
edge = OutgoingEdges("replace_method", to=["boolean_literal_cleanup"], scope="Parent")

# Create Piranha arguments
piranha_arguments = PiranhaArguments(
    code_snippet=code,
    language="java",
    rule_graph=RuleGraph(rules=[r1], edges=[edge])
)
rajeshchinnam commented 2 months ago

Thanks @danieltrt Below rule worked for me

remove_feature_toggle_if_else = Rule( name="remove_feature_toggle_if_else", query='cs if (Boolean.parseBoolean(tenantConfiguration.get(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY))) {:[if_block+] } else {:[else_block+]}', replace_node="*", replace=":[if_block]", is_seed_rule=True )

I have another use case where in i want to remove test methods when my FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY is false

@Test @Tag("UnitTest") @DisplayName("Should get random value if feature toggle is disabled") void shouldGetRandomValueIfFeatureIsDisabled() {

when(tenantConfiguration.get(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)).thenReturn("false");
String result = documentInfoService.get(WARRANTY_REGISTRATION_NUMBER_PREFIX, "testRequest");
assertTrue(result.matches("\\d+"));

}

and in below test I want to remove the when line @Test @Tag("UnitTest") @DisplayName("Should not get and save for invaliad document info") void testGetWithInvalidRequestNumberFormat() { when(tenantConfiguration.get(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)).thenReturn("true");

documentInfoService.get("INVALID_FORMAT", "testRequest");

verify(documentInfoRepository, never()).getMaxWarrantyRegistrationNumber();
verify(documentInfoRepository, never()).save(any(DocumentInfo.class));

} and lastly I want to remove enum declaration

Can you please suggest the rule

rajeshchinnam commented 2 months ago

@danieltrt I am having challenges to match my rule with Code

@Test void shouldGetRandomValueIfFeatureIsDisabled() { when(tenantConfiguration.get(TenantConfigurationKey.FEATURE_SEQUENTIAL_NUMBERS_FOR_WARRANTY)).thenReturn("false"); // test code }

test_method_match = Rule( name="test_method_match", query='cs @Test void :[method_name]IfFeatureIsDisabled() { :[body+] }', replace_node="*", replace="", is_seed_rule=True )

when I am run piranha this rule can not able to match the code

can you tell me what is the issue an how to verify my rule matches the code