From b9a87dc04004479a5a2c5031ab1f6bd762d85258 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 13 Mar 2024 22:04:51 +1100 Subject: [PATCH 1/5] Handle function parsing better, use ast parser instead of Worder --- rope/contrib/generate.py | 2 +- rope/refactor/functionutils.py | 100 ++++++++++++++++++++++++++------ ropetest/refactor/inlinetest.py | 96 ++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 19 deletions(-) diff --git a/rope/contrib/generate.py b/rope/contrib/generate.py index 2e6c81b49..a3391565a 100644 --- a/rope/contrib/generate.py +++ b/rope/contrib/generate.py @@ -401,7 +401,7 @@ def get_passed_args(self): start, end = finder.get_primary_range(self.offset) parens_start, parens_end = finder.get_word_parens_range(end - 1) call = source[start:parens_end] - parser = functionutils._FunctionParser(call, False) + parser = functionutils._FunctionCallParser(call, False) args, keywords = parser.get_parameters() for arg in args: if self._is_id(arg): diff --git a/rope/refactor/functionutils.py b/rope/refactor/functionutils.py index 2dfa4e7e1..61f09200a 100644 --- a/rope/refactor/functionutils.py +++ b/rope/refactor/functionutils.py @@ -1,5 +1,9 @@ +import ast +from typing import Tuple, List + from rope.base import pyobjects, worder from rope.base.builtins import Lambda +from rope.base.codeanalyze import SourceLinesAdapter class DefinitionInfo: @@ -33,7 +37,7 @@ def _read(pyfunction, code): kind = pyfunction.get_kind() is_method = kind == "method" is_lambda = kind == "lambda" - info = _FunctionParser(code, is_method, is_lambda) + info = _FunctionDefParser(code, is_method, is_lambda) args, keywords = info.get_parameters() args_arg = None keywords_arg = None @@ -108,7 +112,7 @@ def read(primary, pyname, definition_info, code): is_method_call = CallInfo._is_method_call(primary, pyname) is_constructor = CallInfo._is_class(pyname) is_classmethod = CallInfo._is_classmethod(pyname) - info = _FunctionParser(code, is_method_call or is_classmethod) + info = _FunctionCallParser(code, is_method_call or is_classmethod) args, keywords = info.get_parameters() args_arg = None keywords_arg = None @@ -202,8 +206,11 @@ def to_call_info(self, definition_info): ) -class _FunctionParser: - def __init__(self, call, implicit_arg, is_lambda=False): +class _BaseFunctionParser: + call: str + implicit_arg: bool + + def __init__(self, call: str, implicit_arg: bool, is_lambda: bool = False): self.call = call self.implicit_arg = implicit_arg self.word_finder = worder.Worder(self.call) @@ -213,26 +220,83 @@ def __init__(self, call, implicit_arg, is_lambda=False): self.last_parens = self.call.rindex(")") self.first_parens = self.word_finder._find_parens_start(self.last_parens) - def get_parameters(self): - args, keywords = self.word_finder.get_parameters( - self.first_parens, self.last_parens - ) + def get_function_name(self): if self.is_called_as_a_method(): - instance = self.call[: self.call.rindex(".", 0, self.first_parens)] - args.insert(0, instance.strip()) - return args, keywords + return self.word_finder.get_word_at(self.first_parens - 1) + else: + return self.word_finder.get_primary_at(self.first_parens - 1) + + def is_called_as_a_method(self): + return self.implicit_arg and "." in self.call[: self.first_parens] + + def _get_source_range(self, tree): + start = self._lines.get_line_start(tree.lineno) + tree.col_offset + end = self._lines.get_line_start(tree.end_lineno) + tree.end_col_offset + return self._lines.code[start:end] def get_instance(self): + # UNUSED if self.is_called_as_a_method(): return self.word_finder.get_primary_at( self.call.rindex(".", 0, self.first_parens) - 1 ) - def get_function_name(self): - if self.is_called_as_a_method(): - return self.word_finder.get_word_at(self.first_parens - 1) - else: - return self.word_finder.get_primary_at(self.first_parens - 1) - def is_called_as_a_method(self): - return self.implicit_arg and "." in self.call[: self.first_parens] +class _FunctionDefParser(_BaseFunctionParser): + _lines: SourceLinesAdapter + def __init__(self, call, implicit_arg, is_lambda=False): + super().__init__(call, implicit_arg, is_lambda=False) + _modified_call = "def " + call.rstrip(":") + ": pass" + self._lines = SourceLinesAdapter(_modified_call) + self.ast = ast.parse(_modified_call).body[0] + assert isinstance(self.ast, ast.FunctionDef) + + def get_parameters(self) -> Tuple[List[str], List[Tuple[str, str]]]: + # FIXME: the weird parsing here is because we're replicating what + # _FunctionParser originally did, which was designed before the + # existence of posonlyargs and kwonlyargs, we'll want to rewrite this + # properly to handle them properly at some point + args = [] + kwargs = [] + args += [arg.arg for arg in self.ast.args.posonlyargs] + args += [arg.arg for arg in self.ast.args.args] + if self.ast.args.vararg is not None: + args += ["*" + self.ast.args.vararg.arg] + if len(self.ast.args.defaults) > 0: + defaults = self.ast.args.defaults + kwargs += [(name, self._get_source_range(value)) for name, value in zip(args[-len(defaults):], defaults)] + del args[-len(self.ast.args.defaults):] + if self.ast.args.kwarg is not None: + args += ["**" + self.ast.args.kwarg.arg] + if self.ast.args.kwonlyargs: + args += ["*"] + [arg.arg for arg in self.ast.args.kwonlyargs] + if len(self.ast.args.kw_defaults) > 0: + kw_defaults = self.ast.args.kw_defaults + kwargs += [(name, self._get_source_range(value)) for name, value in zip(kwargs[-len(kw_defaults):], kw_defaults)] + del args[-len(self.ast.args.kw_defaults):] + return args, kwargs + + +class _FunctionCallParser(_BaseFunctionParser): + _lines: SourceLinesAdapter + ast: ast.Call + def __init__(self, call, implicit_arg, is_lambda=False): + super().__init__(call, implicit_arg, is_lambda=False) + self._lines = SourceLinesAdapter(call) + self.ast = ast.parse(call).body[0].value + assert isinstance(self.ast, ast.Call) + + def get_parameters(self) -> Tuple[List[str], List[Tuple[str, str]]]: + args = [] + for arg in self.ast.args: + arg_value = self._get_source_range(arg) + args.append(arg_value) + kwargs = [] + for kw in self.ast.keywords: + kw_value = self._get_source_range(kw.value) + assert kw.arg + kwargs.append((kw.arg, kw_value)) + if self.is_called_as_a_method(): + instance = self.call[: self.call.rindex(".", 0, self.first_parens)] + args.insert(0, instance.strip()) + return args, kwargs diff --git a/ropetest/refactor/inlinetest.py b/ropetest/refactor/inlinetest.py index f581f4548..585214b41 100644 --- a/ropetest/refactor/inlinetest.py +++ b/ropetest/refactor/inlinetest.py @@ -1425,3 +1425,99 @@ def test_dictionary_with_inline_comment(self): }) """) self.assertEqual(expected, refactored) + + def test_function_call_with_callsite_inline_comment(self): + code = dedent("""\ + def a_func(arg1, arg2): + return arg1 + arg2 + 1 + myvar = a_func( + 1, # some comment + 2, # another comment + ) + """) + refactored = self._inline(code, code.index("a_func") + 1) + expected = dedent("""\ + myvar = 1 + 2 + 1 + """) + self.assertEqual(expected, refactored) + + def test_function_call_with_callsite_interline_comment(self): + code = dedent("""\ + def a_func(arg1, arg2): + return arg1 + arg2 + 1 + myvar = a_func( + # some comment + 1, + # another comment + 2, + # another comment + ) + """) + refactored = self._inline(code, code.index("a_func") + 1) + expected = dedent("""\ + myvar = 1 + 2 + 1 + """) + self.assertEqual(expected, refactored) + + def test_function_call_with_defsite_inline_comment(self): + code = dedent("""\ + def a_func( + arg1, # noqa + arg2, + ): + return arg1 + arg2 + 1 + myvar = a_func(1, 2) + """) + refactored = self._inline(code, code.index("a_func") + 1) + expected = dedent("""\ + myvar = 1 + 2 + 1 + """) + self.assertEqual(expected, refactored) + + def test_function_call_with_defsite_interline_comment(self): + code = dedent("""\ + def a_func( + # blah + arg1, + # blah blah + arg2, + # blah blah + ): + return arg1 + arg2 + 1 + myvar = a_func(1, 2) + """) + refactored = self._inline(code, code.index("a_func") + 1) + expected = dedent("""\ + myvar = 1 + 2 + 1 + """) + self.assertEqual(expected, refactored) + + def test_function_call_with_posonlyargs(self): + code = dedent("""\ + def a_func(arg1, /, arg2): + return arg1 + arg2 + 1 + myvar = a_func(1, 2) + """) + refactored = self._inline(code, code.index("a_func") + 1) + expected = dedent("""\ + myvar = 1 + 2 + 1 + """) + self.assertEqual(expected, refactored) + + def test_function_call_with_kwonlyargs(self): + code = dedent("""\ + def a_func(arg1, *, arg2): + return arg1 + arg2 + 1 + myvar = a_func(1, arg2=2) + """) + with self.assertRaises(rope.base.exceptions.RefactoringError): + refactored = self._inline(code, code.index("a_func") + 1) + + def test_function_call_with_kwonlyargs2(self): + code = dedent("""\ + def a_func(arg1, *, arg2=2): + return arg1 + arg2 + 1 + myvar = a_func(1) + """) + with self.assertRaises(rope.base.exceptions.RefactoringError): + refactored = self._inline(code, code.index("a_func") + 1) From 2352d23bc24c4fd51fa24a6e541cde45c13d4d15 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 13 Mar 2024 22:13:23 +1100 Subject: [PATCH 2/5] Remove unused function --- rope/refactor/functionutils.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/rope/refactor/functionutils.py b/rope/refactor/functionutils.py index 61f09200a..7a0934b44 100644 --- a/rope/refactor/functionutils.py +++ b/rope/refactor/functionutils.py @@ -234,13 +234,6 @@ def _get_source_range(self, tree): end = self._lines.get_line_start(tree.end_lineno) + tree.end_col_offset return self._lines.code[start:end] - def get_instance(self): - # UNUSED - if self.is_called_as_a_method(): - return self.word_finder.get_primary_at( - self.call.rindex(".", 0, self.first_parens) - 1 - ) - class _FunctionDefParser(_BaseFunctionParser): _lines: SourceLinesAdapter From 0446b48ec7e6800d71030a039d7da0082eb214e6 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 13 Mar 2024 22:19:50 +1100 Subject: [PATCH 3/5] Updated CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfa7baf67..3fcfe3f2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ # **Upcoming release** -- Check for ast.Attributes when finding occurrences in fstrings (@sandratsy) +- #751 Check for ast.Attributes when finding occurrences in fstrings (@sandratsy) - #777, #698 add validation to refuse Rename refactoring to a python keyword - #730 Match on module aliases for autoimport suggestions - #755 Remove dependency on `build` package being installed while running tests +- #780 Improved function parser to use ast parser instead of Worder # Release 1.12.0 From b4f82ac0ca6c9be4da7d75dee9cc893649c07851 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 13 Mar 2024 22:20:39 +1100 Subject: [PATCH 4/5] Blacken --- rope/refactor/functionutils.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/rope/refactor/functionutils.py b/rope/refactor/functionutils.py index 7a0934b44..5bd5e5ad7 100644 --- a/rope/refactor/functionutils.py +++ b/rope/refactor/functionutils.py @@ -237,6 +237,7 @@ def _get_source_range(self, tree): class _FunctionDefParser(_BaseFunctionParser): _lines: SourceLinesAdapter + def __init__(self, call, implicit_arg, is_lambda=False): super().__init__(call, implicit_arg, is_lambda=False) _modified_call = "def " + call.rstrip(":") + ": pass" @@ -257,22 +258,29 @@ def get_parameters(self) -> Tuple[List[str], List[Tuple[str, str]]]: args += ["*" + self.ast.args.vararg.arg] if len(self.ast.args.defaults) > 0: defaults = self.ast.args.defaults - kwargs += [(name, self._get_source_range(value)) for name, value in zip(args[-len(defaults):], defaults)] - del args[-len(self.ast.args.defaults):] + kwargs += [ + (name, self._get_source_range(value)) + for name, value in zip(args[-len(defaults) :], defaults) + ] + del args[-len(self.ast.args.defaults) :] if self.ast.args.kwarg is not None: args += ["**" + self.ast.args.kwarg.arg] if self.ast.args.kwonlyargs: args += ["*"] + [arg.arg for arg in self.ast.args.kwonlyargs] if len(self.ast.args.kw_defaults) > 0: kw_defaults = self.ast.args.kw_defaults - kwargs += [(name, self._get_source_range(value)) for name, value in zip(kwargs[-len(kw_defaults):], kw_defaults)] - del args[-len(self.ast.args.kw_defaults):] + kwargs += [ + (name, self._get_source_range(value)) + for name, value in zip(kwargs[-len(kw_defaults) :], kw_defaults) + ] + del args[-len(self.ast.args.kw_defaults) :] return args, kwargs class _FunctionCallParser(_BaseFunctionParser): _lines: SourceLinesAdapter ast: ast.Call + def __init__(self, call, implicit_arg, is_lambda=False): super().__init__(call, implicit_arg, is_lambda=False) self._lines = SourceLinesAdapter(call) From db2ec0c94941699e71f8c4ff0e80f4d22d817e21 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Wed, 13 Mar 2024 22:21:55 +1100 Subject: [PATCH 5/5] Update testing matrix to use release version of Python 3.12 --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 79cd90f81..403c5a3f4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,7 @@ jobs: strategy: matrix: - python-version: ['3.8', '3.9', '3.10', '3.11', '3.12.0-rc.3'] + python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] os: [ubuntu-latest, windows-latest, macos-latest] fail-fast: false steps: