diff --git a/README.md b/README.md index 563b7b8..9594365 100644 --- a/README.md +++ b/README.md @@ -556,6 +556,32 @@ Item.objects.insert(items) Item.objects.create(item) ``` +### PIE810: single-starts-ends-with + +Instead of calling `startswith` or `endswith` on the same string for multiple +prefixes, pass the prefixes as a tuple in a single `startswith` or `endswith` +call. + +```python +# error +foo.startswith("foo") or foo.startswith("bar") + +# error +foo.endswith("foo") or foo.endswith("bar") + +# error +foo.startswith("foo") or str.startswith(foo, "bar") + +# ok +foo.startswith(("foo", "bar")) + +# ok +foo.endswith(("foo", "bar")) + +# ok +foo.startswith("foo") or foo.endswith("bar") +``` + ## development ### examining the AST diff --git a/flake8_pie/__init__.py b/flake8_pie/__init__.py index e689274..7fc2bdd 100644 --- a/flake8_pie/__init__.py +++ b/flake8_pie/__init__.py @@ -41,6 +41,7 @@ from flake8_pie.pie807_pefer_list_builtin import pie807_prefer_list_builtin from flake8_pie.pie808_prefer_simple_range import pie808_prefer_simple_range from flake8_pie.pie809_django_prefer_bulk import pie809_django_prefer_bulk +from flake8_pie.pie810_single_starts_ends_with import pie810_single_starts_ends_with @dataclass(frozen=True) @@ -171,6 +172,9 @@ def visit_Module(self, node: ast.Module) -> None: self._visit_body(node) self.generic_visit(node) + def visit_BoolOp(self, node: ast.BoolOp) -> None: + pie810_single_starts_ends_with(node, self.errors) + def __repr__(self) -> str: return f"<{self.__class__.__name__}: errors={self.errors}>" diff --git a/flake8_pie/pie798_no_unnecessary_class.py b/flake8_pie/pie798_no_unnecessary_class.py index b851869..3fb6c39 100644 --- a/flake8_pie/pie798_no_unnecessary_class.py +++ b/flake8_pie/pie798_no_unnecessary_class.py @@ -12,13 +12,10 @@ def _is_possible_instance_method(func: ast.FunctionDef | ast.AsyncFunctionDef) - if func.args.args and func.args.args[0].arg == "self": return True - if func.decorator_list and any( + return len(func.decorator_list) > 0 and any( not isinstance(dec, ast.Name) or dec.id not in ALLOW_DECORATORS for dec in func.decorator_list - ): - return True - - return False + ) def pie798_no_unnecessary_class(node: ast.ClassDef, errors: list[Error]) -> None: diff --git a/flake8_pie/pie810_single_starts_ends_with.py b/flake8_pie/pie810_single_starts_ends_with.py new file mode 100644 index 0000000..81aa5c8 --- /dev/null +++ b/flake8_pie/pie810_single_starts_ends_with.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +import ast +from collections import defaultdict +from functools import partial +from typing import Any, Type + +from flake8_pie.base import Error + + +def pie810_single_starts_ends_with(node: ast.BoolOp, errors: list[Error]) -> None: + """Check that multiple [starts/ends]with calls in an op operation do not look for + prefixes in the same string. + """ + if not isinstance(node.op, ast.Or): + return + + for method in ("startswith", "endswith"): + seen: dict[Type[ast.AST], Any] = defaultdict(set) + for val_node in node.values: + if ( + isinstance(val_node, ast.Call) + and isinstance(val_node.func, ast.Attribute) + and val_node.func.attr == method + and isinstance(val_node.func.value, ast.Name) + ): + if val_node.func.value.id == "str": + # str.startswith(stack, needle) -> stack + arg = val_node.args[0] + else: + # stack.startswith(needle) -> stack + arg = val_node.func.value + + if not isinstance(arg, (ast.Constant, ast.Name)): + # we cannot check the equivalence of other types + continue + + t = type(arg) + val = arg.value if isinstance(arg, ast.Constant) else arg.id + if val in seen[t]: + errors.append( + PIE810(lineno=node.lineno, col_offset=node.col_offset) + ) + seen[t].add(val) + + +PIE810 = partial( + Error, + message=( + "PIE810 single-starts-ends-with: Call [starts/ends]with once with a tuple " + "instead of calling it multiple times with the same string." + ), +) diff --git a/flake8_pie/tests/test_pie810_single_starts_ends_with.py b/flake8_pie/tests/test_pie810_single_starts_ends_with.py new file mode 100644 index 0000000..57fac8a --- /dev/null +++ b/flake8_pie/tests/test_pie810_single_starts_ends_with.py @@ -0,0 +1,66 @@ +from __future__ import annotations + +import ast +from itertools import combinations_with_replacement + +import pytest + +from flake8_pie import Flake8PieCheck +from flake8_pie.pie810_single_starts_ends_with import PIE810 +from flake8_pie.tests.utils import Error, ex, to_errors + +STMTS = [ + "{arg}.{method}('aaa')", + "str.{method}({arg}, 'aaa')", + "{arg}.{method}(('aaa', 'bbb'))", +] +MTDS = ["startswith", "endswith"] +ARGS = ["foo", "bar"] + +# a single expression is ok +OK_SINGLE = [ + stmt.format(arg=arg, method=method) + for stmt in STMTS + for method in MTDS + for arg in ARGS +] + +# no warning if binary operator is and +OK_AND = [ + f"{stmt1} and {stmt2}" + for stmt1, stmt2 in combinations_with_replacement(OK_SINGLE, 2) +] + +# no warning if we combine startswith and endswith +OK_DIFF_MTDS = [ + f"{stmt1.format(arg='foo', method='startswith')} or {stmt2.format(arg='foo', method='endswith')}" + for stmt1, stmt2 in zip(STMTS, STMTS) +] + +# different args should not result in a warning +OK_DIFF_ARGS = [ + f"{stmt1.format(arg='foo', method=method)} or {stmt2.format(arg='bar', method=method)}" + for stmt1, stmt2 in zip(STMTS, STMTS) + for method in MTDS +] + +WARN = [ + f"{stmt1} or {stmt2}" + for method in MTDS + for arg in ARGS + for stmt1, stmt2 in combinations_with_replacement( + [stmt.format(arg=arg, method=method) for stmt in STMTS], 2 + ) +] + + +EXAMPLES = [ex(code=code, errors=[PIE810(lineno=1, col_offset=0)]) for code in WARN] + [ + ex(code=code, errors=[]) + for code in OK_SINGLE + OK_AND + OK_DIFF_MTDS + OK_DIFF_ARGS +] + + +@pytest.mark.parametrize("code,errors", EXAMPLES) +def test_examples(code: str, errors: list[Error]) -> None: + expr = ast.parse(code) + assert to_errors(Flake8PieCheck(expr, filename="foo.py").run()) == errors