Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Commit

Permalink
Introduce single-starts-ends-with (#121)
Browse files Browse the repository at this point in the history
Closes #102.

Also fixes an instance of a PIE801 warning in the codebase.
  • Loading branch information
AlpAribal authored Sep 20, 2022
1 parent 60a8dbe commit aaa54ce
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 5 deletions.
26 changes: 26 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions flake8_pie/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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}>"

Expand Down
7 changes: 2 additions & 5 deletions flake8_pie/pie798_no_unnecessary_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
53 changes: 53 additions & 0 deletions flake8_pie/pie810_single_starts_ends_with.py
Original file line number Diff line number Diff line change
@@ -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."
),
)
66 changes: 66 additions & 0 deletions flake8_pie/tests/test_pie810_single_starts_ends_with.py
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit aaa54ce

Please sign in to comment.