From fe54800cddd09584905704698d0e6a5fd3f961f2 Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Sat, 23 May 2020 16:44:24 +0300 Subject: [PATCH 1/3] Check all arguments for unparenthesized generator expressions Previously only the first argument on the argument list checked against the generator expressions, now all argumnets are controlled. --- parso/python/errors.py | 103 ++++++++++++++++++------------------- test/test_python_errors.py | 12 +++++ 2 files changed, 62 insertions(+), 53 deletions(-) diff --git a/parso/python/errors.py b/parso/python/errors.py index 5df4750..2142dd1 100644 --- a/parso/python/errors.py +++ b/parso/python/errors.py @@ -779,62 +779,59 @@ class _ArglistRule(SyntaxRule): return "Generator expression must be parenthesized" def is_issue(self, node): - first_arg = node.children[0] - if first_arg.type == 'argument' \ - and first_arg.children[1].type in _COMP_FOR_TYPES: - # e.g. foo(x for x in [], b) - return len(node.children) >= 2 - else: - arg_set = set() - kw_only = False - kw_unpacking_only = False - is_old_starred = False - # In python 3 this would be a bit easier (stars are part of - # argument), but we have to understand both. - for argument in node.children: - if argument == ',': - continue + arg_set = set() + kw_only = False + kw_unpacking_only = False + is_old_starred = False + # In python 3 this would be a bit easier (stars are part of + # argument), but we have to understand both. + for argument in node.children: + if argument == ',': + continue - if argument in ('*', '**'): - # Python < 3.5 has the order engraved in the grammar - # file. No need to do anything here. - is_old_starred = True - continue - if is_old_starred: - is_old_starred = False - continue + if argument in ('*', '**'): + # Python < 3.5 has the order engraved in the grammar + # file. No need to do anything here. + is_old_starred = True + continue + if is_old_starred: + is_old_starred = False + continue - if argument.type == 'argument': - first = argument.children[0] - if first in ('*', '**'): - if first == '*': - if kw_unpacking_only: - # foo(**kwargs, *args) - message = "iterable argument unpacking " \ - "follows keyword argument unpacking" - self.add_issue(argument, message=message) + if argument.type == 'argument': + first = argument.children[0] + if argument.children[1].type in _COMP_FOR_TYPES and len(node.children) >= 2: + # a(a, b for b in c) + return True + if first in ('*', '**'): + if first == '*': + if kw_unpacking_only: + # foo(**kwargs, *args) + message = "iterable argument unpacking " \ + "follows keyword argument unpacking" + self.add_issue(argument, message=message) + else: + kw_unpacking_only = True + else: # Is a keyword argument. + kw_only = True + if first.type == 'name': + if first.value in arg_set: + # f(x=1, x=2) + message = "keyword argument repeated" + if self._normalizer.version >= (3, 9): + message += ": {}".format(first.value) + self.add_issue(first, message=message) else: - kw_unpacking_only = True - else: # Is a keyword argument. - kw_only = True - if first.type == 'name': - if first.value in arg_set: - # f(x=1, x=2) - message = "keyword argument repeated" - if self._normalizer.version >= (3, 9): - message += ": {}".format(first.value) - self.add_issue(first, message=message) - else: - arg_set.add(first.value) - else: - if kw_unpacking_only: - # f(**x, y) - message = "positional argument follows keyword argument unpacking" - self.add_issue(argument, message=message) - elif kw_only: - # f(x=2, y) - message = "positional argument follows keyword argument" - self.add_issue(argument, message=message) + arg_set.add(first.value) + else: + if kw_unpacking_only: + # f(**x, y) + message = "positional argument follows keyword argument unpacking" + self.add_issue(argument, message=message) + elif kw_only: + # f(x=2, y) + message = "positional argument follows keyword argument" + self.add_issue(argument, message=message) @ErrorFinder.register_rule(type='parameters') diff --git a/test/test_python_errors.py b/test/test_python_errors.py index 7131b1b..3eba4af 100644 --- a/test/test_python_errors.py +++ b/test/test_python_errors.py @@ -375,3 +375,15 @@ def test_repeated_kwarg(): _get_error_list("f(q=1, q=2)", version="3.9")[0].message == "SyntaxError: keyword argument repeated: q" ) + + +@pytest.mark.parametrize( + 'source', [ + 'a(a for a in b,)' + 'a(a for a in b, a)', + 'a(a, a for a in b)', + 'a(a, b, a for a in b, c, d)', + ] +) +def test_unparenthesized_genexp(source): + assert _get_error_list(source) From 92396a9a16e5e4fb8a5ef9f3b5ad6109a6302a92 Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Sat, 23 May 2020 17:44:28 +0300 Subject: [PATCH 2/3] allow trailing comma <3.6, test both postive/negative cases --- parso/python/errors.py | 15 ++++++++++++--- test/test_python_errors.py | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/parso/python/errors.py b/parso/python/errors.py index 2142dd1..4d9bff2 100644 --- a/parso/python/errors.py +++ b/parso/python/errors.py @@ -800,9 +800,18 @@ class _ArglistRule(SyntaxRule): if argument.type == 'argument': first = argument.children[0] - if argument.children[1].type in _COMP_FOR_TYPES and len(node.children) >= 2: - # a(a, b for b in c) - return True + if ( + argument.children[1].type in _COMP_FOR_TYPES + and len(node.children) >= 2 + ): + if self._normalizer.version >= (3, 7): + return True + elif len(node.children) == 2 and node.children[1] == ",": + # trailing comma allowed until 3.7 + pass + else: + return True + if first in ('*', '**'): if first == '*': if kw_unpacking_only: diff --git a/test/test_python_errors.py b/test/test_python_errors.py index 3eba4af..94ff0c7 100644 --- a/test/test_python_errors.py +++ b/test/test_python_errors.py @@ -378,12 +378,17 @@ def test_repeated_kwarg(): @pytest.mark.parametrize( - 'source', [ - 'a(a for a in b,)' - 'a(a for a in b, a)', - 'a(a, a for a in b)', - 'a(a, b, a for a in b, c, d)', + ('source', 'no_errors', 'version'), [ + ('a(a for a in b,)', True, '3.6'), + ('a(a for a in b,)', False, '3.7'), + ('a(a for a in b, a)', False, None), + ('a(a, a for a in b)', False, None), + ('a(a, b, a for a in b, c, d)', False, None), + ('a(a for a in b)', True, None), + ('a((a for a in b), c)', True, None), + ('a(c, (a for a in b))', True, None), + ('a(a, (a for a in b), c)', True, None), ] ) -def test_unparenthesized_genexp(source): - assert _get_error_list(source) +def test_unparenthesized_genexp(source, no_errors, version): + assert bool(_get_error_list(source, version=version)) ^ no_errors From 6b0e01c22006d5fe9f819546c3a6107ca008eacb Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Sat, 23 May 2020 21:08:10 +0300 Subject: [PATCH 3/3] Revert trailing comma for 3.6< --- parso/python/errors.py | 15 +++------------ test/test_python_errors.py | 23 +++++++++++------------ 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/parso/python/errors.py b/parso/python/errors.py index 4d9bff2..2142dd1 100644 --- a/parso/python/errors.py +++ b/parso/python/errors.py @@ -800,18 +800,9 @@ class _ArglistRule(SyntaxRule): if argument.type == 'argument': first = argument.children[0] - if ( - argument.children[1].type in _COMP_FOR_TYPES - and len(node.children) >= 2 - ): - if self._normalizer.version >= (3, 7): - return True - elif len(node.children) == 2 and node.children[1] == ",": - # trailing comma allowed until 3.7 - pass - else: - return True - + if argument.children[1].type in _COMP_FOR_TYPES and len(node.children) >= 2: + # a(a, b for b in c) + return True if first in ('*', '**'): if first == '*': if kw_unpacking_only: diff --git a/test/test_python_errors.py b/test/test_python_errors.py index 94ff0c7..743cb25 100644 --- a/test/test_python_errors.py +++ b/test/test_python_errors.py @@ -378,17 +378,16 @@ def test_repeated_kwarg(): @pytest.mark.parametrize( - ('source', 'no_errors', 'version'), [ - ('a(a for a in b,)', True, '3.6'), - ('a(a for a in b,)', False, '3.7'), - ('a(a for a in b, a)', False, None), - ('a(a, a for a in b)', False, None), - ('a(a, b, a for a in b, c, d)', False, None), - ('a(a for a in b)', True, None), - ('a((a for a in b), c)', True, None), - ('a(c, (a for a in b))', True, None), - ('a(a, (a for a in b), c)', True, None), + ('source', 'no_errors'), [ + ('a(a for a in b,)', False), + ('a(a for a in b, a)', False), + ('a(a, a for a in b)', False), + ('a(a, b, a for a in b, c, d)', False), + ('a(a for a in b)', True), + ('a((a for a in b), c)', True), + ('a(c, (a for a in b))', True), + ('a(a, b, (a for a in b), c, d)', True), ] ) -def test_unparenthesized_genexp(source, no_errors, version): - assert bool(_get_error_list(source, version=version)) ^ no_errors +def test_unparenthesized_genexp(source, no_errors): + assert bool(_get_error_list(source)) ^ no_errors