Suppose I want to "lift" some assignments from the body of a function into the arguments e.g.

def foo(a, b):
  c = 5
  print(3 * c)

->

def foo(a, b, c=5):
  print(3 * c)

As far as I can tell, each Rule can only do one thing - either LazyReplace a node or Erase some nodes. But since I need both a LazyReplace and an Erase, where either I sequence these rewrites or that there should be shared context between both rules (either the Erase should pass c to the LazyReplace which inserts c in the args, or the LazyReplace inserts c and then passes it to Erase) but that seems like a bad idea (not sure what invariants you're depending on wrt the AST...

Note that refactor.Erase(a).apply in the context of LazyReplace obviously won't work because i don't have access to the textual source at that point. Currently I'm just doing a del new_node.body[i] in the LazyReplace's build method (where the i is passed in from Rule).

EDIT:

seems

if action := rule.match(node):

could become

if action := rule.match(node):
   if not isinstance(action, (list, tuple)):
       action = [action]
   for action in in rule.match(node):

here.

When running refactor over the following code:

    def __init__(self, *, foo):
        self.foo = foo

I get:

AttributeError: 'NoneType' object has no attribute '_fields'

It looks like the runner in main.py passes options.src to run_files directly https://github.com/isidentical/refactor/blob/master/refactor/main.py#L62 so you must pass in a single python file whereas the unbound runner expands the paths so you can run this on a directory of files: https://github.com/isidentical/refactor/blob/master/refactor/runner.py#L89-L91.

Would you accept a PR to change this behavior? Or was this separation purposeful?

Thanks for building such a great lib on Python AST! However, writing asserts in rules is pretty tedious, I wonder if it's possible to parse a target python AST, and generate the rules automatically?

Also, another question is, now that the contract is based on a single ast.Node, is it possible to do a partial replacement with multiple statements (spans multiple ast.Nodes)?

For example, how do I write the rule to replace all matches of the following 2 statements:

    boxes[:, 0] = boxes_xywhr[:, 0] - half_w
    boxes[:, 1] = boxes_xywhr[:, 1] - half_h

Do I have to do it in a larger scope node (say ast.FunctionDef)? Or is it possible to get some UD-Chains based on the current ast.Node?

Proposing to add the possibility to insert statement before a node. This was found useful when combining 2 methods and needing to conserve the order of execution

Hey,

I am having issues trying to move code by using lineno attribute.

This is my code:

import ast
from dataclasses import dataclass

from refactor import Action, Rule


@dataclass
class SwitchLinesAction(Action):
    node: ast.AST
    lineno: int

    def build(self):
        new_node = self.branch()
        new_node.lineno = self.lineno
        return new_node


class ReplaceTopLevelImportByFunctionLevelImport(Rule):
    """Replace top-level import by function-level import"""

    def match(self, node: ast.AST) -> Action:
        assert isinstance(node, ast.Import)

        return SwitchLinesAction(node, lineno=2)

And this is my test code:

import numpy
1
1

When I print before and after lineno, I can see that the value indeed changes, but the final representation is the same - it does not move the code from line 1 to line 2.

You can already see there what my goal is: "replacing top-level imports by function-level imports"

It is a common need when doing refactors to do simple name resolution, as well as gathering data about the parental nodes. It would be really useful to have built-in representatives to do so.

I seem to hit a conceptualization issue trying to do this. When I reach the FunctionDef, I look for await statements and if there's none, I want to add asyncio.sleep(0) to the last line, I search in the body and try an InsertAfter the last node of the body, but that errors out

Well, the crude method of adding the Expr at the end of body kinda works, but an empty line is inserted as well and it felt clunky

There are several notes:

1. I've removed pre-commit/action, because it is deprecated. I recommend switching to https://pre-commit.ci/
2. I've updated multiple GitHub Actions to newer versions
3. I've also changed how new version is published. It should be only published from master and using 3.9 (not 3.8)

Closes https://github.com/isidentical/refactor/issues/28

Apologies if I've misunderstood how to use refactor. I was trying to run it with

refactor -d test_refactor.py test.py

Where test_refactor.py is a file with my Rule and test.py is some source. I get an AttributeError at https://github.com/isidentical/refactor/blob/master/refactor/main.py#L45. Debugging, I think this should be refactor_file = options.refactor_file?

-> refactor_file = options.refactor_dir
(Pdb) options
Namespace(src=[PosixPath('test.py')], refactor_file=PosixPath('test_refactor.py'), dont_apply=True)

Python 3.9 / refactor 0.4.1.

We could try to reduce the number of .match() calls we are making by giving Rules to register certain types of nodes. This will eliminate the top level assert isinstance(node, <node type>) check from their side and made our refactor loop faster.

Would it be helpful to add an hint as to the action that generated an overlap error with:

            try:
                updated_input = path.execute(previous_tree)
            except AccessFailure:
                raise MaybeOverlappingActions(
                    "When using chained actions, individual actions should not"
                    " overlap with each other."
                    f"   Action attempted: {action}"
                ) from None

Bear with the padawan - This is just for your review and comments. It seems that I always convolute things that can be written much simpler It feels too clunky for just a couples of comments, though

This refactoring, similar to what I actually used:

import ast
import refactor

class WrapF(refactor.Rule):
    def match(self, node: ast.AST) -> refactor.BaseAction:
        assert isinstance(node, ast.Constant)

        # Prevent wrapping F-strings that are already wrapped in F()
        # Otherwise you get infinite F(F(F(F(...))))
        parent = self.context.ancestry.get_parent(node)
        assert not (isinstance(parent, ast.Call) and isinstance(parent.func, ast.Name) and parent.func.id == 'F')

        return refactor.Replace(node, ast.Call(func=ast.Name(id="F"), args=[node], keywords=[]))


refactor.run(rules=[WrapF])

produces this:

 def f():
-    return """
-a
-"""
+    return F("""
+    a
+    """)

This changes the value of the string.

Possibly related is https://github.com/isidentical/refactor/issues/12, but I couldn't reproduce an equivalent problem with just ast.unparse:

import ast

source = '''
def f():
    return """
a
"""
'''

tree = ast.parse(source)
node = tree.body[0].body[0].value
call = ast.Call(func=ast.Name(id="F"), args=[node], keywords=[])
ast.copy_location(call, node)
ast.fix_missing_locations(call)
print(ast.unparse(node))  # '\na\n'
print(ast.unparse(call))  # F('\na\n')

I encountered a situation where I want to remove some decorators (like replacing aiounittest.async_test with IsolatedAsyncioTestCase). refactor throws the InvalidActionError because it finds that I emptied the list of decorators, as it identified the decorator as critical. On the other hand, i don't want to delete the AsyncFunctionDef just because of the decorator_list The workaround to that in my branch is to butcher the is_critical_node in order to invalidate the is_critical_node. Would you suggest a neater way?

Related to: https://github.com/isidentical/refactor/issues/55 Observation:

• When Asyncifing a decorated function, the decorators are duplicated Understanding:
• In apply() method of_ReplaceCodeSegmentAction, the lines exclude the decorators (from position_for())
• Building the replacement is based on the context, which includes the decorators
• Replacing the lines slice with the replacement ends up duplicating the decorators Proposal:
• A possible solution is to include the decorators in the lines since _resyntesize() works with the context that includes the decorators