From 10b166bcec5587a4a920adf1ff9c7163f0cc28fc Mon Sep 17 00:00:00 2001 From: MarcoGorelli Date: Sat, 19 Jun 2021 16:46:45 +0100 Subject: [PATCH 1/2] start writing test --- tests/no_single_letter_variables_test.py | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 tests/no_single_letter_variables_test.py diff --git a/tests/no_single_letter_variables_test.py b/tests/no_single_letter_variables_test.py new file mode 100644 index 0000000..b23d43f --- /dev/null +++ b/tests/no_single_letter_variables_test.py @@ -0,0 +1,45 @@ +import ast +import tokenize +from io import StringIO + +import pytest + +from pandas_dev_flaker.__main__ import run + + +def results(s): + return { + "{}:{}: {}".format(*r) + for r in run( + ast.parse(s), + list(tokenize.generate_tokens(StringIO(s).readline)), + ) + } + + +@pytest.mark.parametrize( + "source", + ( + pytest.param( + "ab = 3", + id="Multi-letter assignment", + ), + ), +) +def test_noop(source): + assert not results(source) + + +@pytest.mark.parametrize( + "source, expected", + ( + pytest.param( + "a = 3", + "1:0: PDF023 don't assign to single-letter variables", + id="Single letter variable", + ), + ), +) +def test_violation(source, expected): + (result,) = results(source) + assert result == expected From 1c6f213fc4e420946bf210e00f52f62066f3902b Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Sat, 19 Jun 2021 20:17:23 +0100 Subject: [PATCH 2/2] No single letter variables This allows `_` to be used as a variable name, since it is conventionally used for throwaway variables, but does not allow any other single letter variables. The `target` method of `ast.Assigns` will normally contain a list of nodes, however for an unpacking assignment (such as `'a,b = c'`) it will contain an `ast.Tuple` or an `ast.List` so we need to cover for all cases. --- README.md | 1 + .../_plugins_tree/single_letter_variables.py | 24 +++++++++++++++++++ ...est.py => single_letter_variables_test.py} | 20 +++++++++++++++- tests/string_to_concatenate_test.py | 6 ++--- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 pandas_dev_flaker/_plugins_tree/single_letter_variables.py rename tests/{no_single_letter_variables_test.py => single_letter_variables_test.py} (55%) diff --git a/README.md b/README.md index 792dc45..5ec40ee 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ a linter for pandas usage, please see [pandas-vet](https://github.com/deppen8/pa | PDF020 | found private import across modules | | PDF021 | found 'np.bool' or 'np.object' (use 'np.bool_' or 'np.object_' instead) | | PDF022 | found import from 'numpy.random' | +| PDF023 | found assignment to single-letter variable | ## contributing See `contributing.md` for how to get started. diff --git a/pandas_dev_flaker/_plugins_tree/single_letter_variables.py b/pandas_dev_flaker/_plugins_tree/single_letter_variables.py new file mode 100644 index 0000000..8222579 --- /dev/null +++ b/pandas_dev_flaker/_plugins_tree/single_letter_variables.py @@ -0,0 +1,24 @@ +import ast +from typing import Iterator, Tuple + +from pandas_dev_flaker._data_tree import State, register + +MSG = "PDF023 found assignment to single-letter variable" + + +@register(ast.Assign) +def visit_Assign( + state: State, + node: ast.Assign, + parent: ast.AST, +) -> Iterator[Tuple[int, int, str]]: + + # Unpacking is represented by putting a Tuple or List within targets + if isinstance(node.targets[0], (ast.Tuple, ast.List)): + assignment_names = node.targets[0].elts + else: + assignment_names = node.targets + + for item in assignment_names: + if isinstance(item, ast.Name) and item.id != "_" and len(item.id) == 1: + yield item.lineno, item.col_offset, MSG diff --git a/tests/no_single_letter_variables_test.py b/tests/single_letter_variables_test.py similarity index 55% rename from tests/no_single_letter_variables_test.py rename to tests/single_letter_variables_test.py index b23d43f..ff152ba 100644 --- a/tests/no_single_letter_variables_test.py +++ b/tests/single_letter_variables_test.py @@ -24,6 +24,14 @@ def results(s): "ab = 3", id="Multi-letter assignment", ), + pytest.param( + "_ = 3", + id="Underscore assignment", + ), + pytest.param( + "ab, cd, _ = (1, 2, 3)", + id="Unpacking including an underscore", + ), ), ) def test_noop(source): @@ -35,9 +43,19 @@ def test_noop(source): ( pytest.param( "a = 3", - "1:0: PDF023 don't assign to single-letter variables", + "1:0: PDF023 found assignment to single-letter variable", id="Single letter variable", ), + pytest.param( + "bar = a = 1", + "1:6: PDF023 found assignment to single-letter variable", + id="Multiple assignment", + ), + pytest.param( + "a, bar = (3, 5)", + "1:0: PDF023 found assignment to single-letter variable", + id="Unpacking", + ), ), ) def test_violation(source, expected): diff --git a/tests/string_to_concatenate_test.py b/tests/string_to_concatenate_test.py index 3afb9ee..85aaf0e 100644 --- a/tests/string_to_concatenate_test.py +++ b/tests/string_to_concatenate_test.py @@ -21,7 +21,7 @@ def results(s): "source", ( pytest.param( - "a = (\n" " 'foo'\n" " 'bar'\n" ")", + "var = (\n" " 'foo'\n" " 'bar'\n" ")", id="separate lines", ), ), @@ -34,8 +34,8 @@ def test_noop(source): "source, expected", ( pytest.param( - "a = 'foo''bar'", - "1:4: PDF007 line split in two unnecessarily by 'black' formatter", + "var = 'foo''bar'", + "1:6: PDF007 line split in two unnecessarily by 'black' formatter", id="consecutive strings", ), ),