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

Decorators are not covered when replacing decorated definitions #55

Open
isidentical opened this issue Aug 16, 2022 · 2 comments
Open

Comments

@isidentical
Copy link
Owner

isidentical commented Aug 16, 2022

Problem

Replace() action depends on the positions that are present in the original AST node. But for definitions like FunctionDef (and all other decorated ones), the lineno points to the introducer keyword (like def in FunctionDef) instead of the first decorator. So when we are replacing the whole function, we leave decorators behind which causes a lot of problems. We should account for this in Replace (and others where we care about the position).

Example Source Code

import ast
from refactor import Rule, Replace, run, common

class MakeFunctionsAsync(Rule):
    def match(self, node: ast.AST) -> Replace:
        assert isinstance(node, ast.FunctionDef)

        new_node = common.clone(node)
        new_node.__class__ = ast.AsyncFunctionDef
        return Replace(node, new_node)


if __name__ == "__main__":
    run([MakeFunctionsAsync])
def foo():
    pass


@deco
def bar():
    pass
@wizpresso-steve-cy-fan
Copy link

We need this feature as well, given we have some manual while loop based retry code can be converted to a retry decorator

@MementoRC
Copy link
Contributor

MementoRC commented Dec 2, 2022

Adding some observation (bear in mind I may not fully understand the issue). When comparing the original FunctionDef to the updated AsyncFunctionDef they seem to have the same decorator_list:

FunctionDef(
    name='test_xxx',
    args=arguments(
        posonlyargs=[],
        args=[
            arg(arg='self'),
            arg(arg='mocked_api')],
        kwonlyargs=[],
        kw_defaults=[],
        defaults=[]),
    body=[
    ...
    decorator_list=[
        Call(
            func=Name(id='aioresponses', ctx=Load()),
            args=[],
            keywords=[])])

and

AsyncFunctionDef(
    name='test_xxx',
    args=arguments(
        posonlyargs=[],
        args=[
            arg(arg='self'),
            arg(arg='mocked_api')],
        kwonlyargs=[],
        kw_defaults=[],
        defaults=[]),
    body=[
    ...
    decorator_list=[
        Call(
            func=Name(id='aioresponses', ctx=Load()),
            args=[],
            keywords=[])])

But

@@ -181,72 +180,61 @@
         return data
 
     @aioresponses()
-    def test_xxx(self, mocked_api):
+    @aioresponses()
+    async def test_xxx(self, mocked_api):

Based on the diff, the decorator would be duplicated

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

3 participants