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

Introduce single-starts-ends-with #121

Merged
merged 5 commits into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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] = {}
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
seen.setdefault(t, set())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use a defaultdict(set) to avoid the set default stuff but doesn’t matter — just a little cuter

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