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

Piranha hangs infinitely when removing a feature flag from Kotlin closure #701

Open
daveight opened this issue Oct 3, 2024 · 9 comments

Comments

@daveight
Copy link

daveight commented Oct 3, 2024

class Sample {
   val lambda1 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda2 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda3 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
}

"stale_flag_name" => "STALE_FLAG"
"treated" => "true"

rules:

(call_expression
    (navigation_expression
      (simple_identifier)
      (navigation_suffix
        (simple_identifier) @fn_name
      )
    )
    (call_suffix
      (value_arguments
        (value_argument
           (simple_identifier) @flag_name
        )
     )
  )
) @call_expression
(#eq? @fn_name "isEnabled")
(#eq? @flag_name "@stale_flag_name")

Expected result:

class Sample {
   val lambda1 = {
      print(true)
   }
   val lambda2 = {
      print(true)
   }
   val lambda3 = {
      print(true)
   }
}
@danieltrt
Copy link
Collaborator

Interesting. I will investigate and get back to you

@daveight
Copy link
Author

daveight commented Oct 4, 2024

Let me know if you need more details I can share the complete test-case with you

@danieltrt
Copy link
Collaborator

danieltrt commented Oct 5, 2024

This works fine on my side? Can you share the code you're running, or provide a full test case where this happens?

Try running this:

from polyglot_piranha import execute_piranha, PiranhaArguments, Rule, RuleGraph, OutgoingEdges

# Original code snippet
code = """
class Sample {
   val lambda1 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda2 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda3 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
}
"""

# Define the rule to replace the method call
r1 = Rule(
    name="replace_method",
    query="""((call_expression
    (navigation_expression
      (simple_identifier)
      (navigation_suffix
        (simple_identifier) @fn_name
      )
    )
    (call_suffix
      (value_arguments
        (value_argument
           (simple_identifier) @flag_name
        )
     )
    )
    ) @call_expression
    (#eq? @fn_name "isEnabled")
    (#eq? @flag_name "@stale_flag_name"))
    """, # cs indicates we are using concrete syntax
    replace_node="call_expression",
    replace="true",
    holes={"stale_flag_name"},
    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="kt",
    rule_graph=RuleGraph(rules=[r1], edges=[edge],),
    substitutions={"stale_flag_name": "STALE_FLAG"}

)

# Execute Piranha and print the transformed code
piranha_summary = execute_piranha(piranha_arguments)
print(piranha_summary[0].content)

@daveight
Copy link
Author

daveight commented Oct 7, 2024

Hello @danieltrt . I pasted this code to my Jupyter notebook and the result is the same - Piranha hangs.
I installed the latest version: 0.3.27

I also executed this rule in the Rust debugger and I observe the same behaviour.

@daveight
Copy link
Author

daveight commented Oct 7, 2024

I activated debug logging:

import logging
from logging import info
FORMAT = "%(levelname)s %(name)s %(asctime)-15s %(filename)s:%(lineno)d %(message)s"
logging.basicConfig(format=FORMAT)
logging.getLogger().setLevel(logging.DEBUG)

and from the logs I can see that Piranha has stuck on the following log:

DEBUG polyglot_piranha.models.edit 2024-10-07 19:44:21,168 edit.rs:129 Changed node kind simple_identifier
DEBUG polyglot_piranha.utilities.tree_sitter_utilities 2024-10-07 19:44:21,169 tree_sitter_utilities.rs:205 
 Update code at (Range { start_byte: 45, end_byte: 49, start_point: Point { row: 3, column: 10 }, end_point: Point { row: 3, column: 14 } }) -
 true 
 to 
true
DEBUG polyglot_piranha.models.source_code_unit 2024-10-07 19:44:21,169 source_code_unit.rs:210 Current Rule: replace_identifier_with_value
DEBUG polyglot_piranha.models.source_code_unit 2024-10-07 19:44:21,169 source_code_unit.rs:217

@danieltrt
Copy link
Collaborator

Ah! You're right! I was using my patched version of Piranha, which is why it wasn't hanging. The issue here is that Piranha erroneously propagates the value to the subsequent declarations, causing it to get stuck in the loop with.

Piranha operates at the syntax level and propagates the scope within Parent scopes (3 levels) incorrectly. After the first replacement, it propagates the value erroneously:

class Sample {
   val lambda1 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda2 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
   val lambda3 = {
      val x = featureService.isEnabled(STALE_FLAG)
      print(x)
   }
}

gets transformed to:

class Sample {
   val lambda1 = {
      print(true)
   }
   val lambda2 = {
      val true = featureService.isEnabled(STALE_FLAG) // incorrect propagation
      print(true)
   }
   val lambda3 = {
      val true = featureService.isEnabled(STALE_FLAG)
      print(true)
   }
} 

This causes it to get stuck in the loop of true -> true.

@ketkarameya , do you know how to resolve this? One potential solution might be to use a new scope or reduce the parent scope from 3 to 2 or 1?

@danieltrt
Copy link
Collaborator

@daveight

I don’t think we can solve this as of now. But if you want a workaround you can try patching Piranha and see if it works for your use case

pub fn default_number_of_ancestors_in_parent_scope() -> u8 {

If you lower the value in the config file you can config Piranha to not propagate the value erroneously outside the lambda by being more conservative in the Parent scope applications.

I think if you set this value to 1 or 2, this problem should go away

@daveight
Copy link
Author

daveight commented Oct 10, 2024

Thanks for the response @danieltrt . Would it be ok if I at some point in the future I will try to solve it and create a PR with the fix? I played around with adding a new scope Lambda as you have mentioned earlier. Thanks

@danieltrt
Copy link
Collaborator

Absolutely! Let me know if you need any help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants