sbdchd / flake8-pie Goto Github PK
View Code? Open in Web Editor NEW:pizza: A flake8 extension that implements misc. lints
License: BSD 2-Clause "Simplified" License
:pizza: A flake8 extension that implements misc. lints
License: BSD 2-Clause "Simplified" License
# err
with psycopg2.connect(dsn=dsn) as conn:
with conn.cursor() as cursor:
cursor.execute("SELECT 1")
# ok
with psycopg2.connect(dsn=dsn) as conn, conn.cursor() as cursor:
cursor.execute("SELECT 1")
# err
async with asyncpg.create_pool(user="postgres", command_timeout=60) as pool:
async with pool.acquire() as con:
await con.execute(
"""
CREATE TABLE names (
id serial PRIMARY KEY,
name VARCHAR (255) NOT NULL)
"""
)
await con.fetch("SELECT 1")
# ok
async with asyncpg.create_pool(
user="postgres", command_timeout=60
) as pool, pool.acquire() as con:
await con.execute(
"""
CREATE TABLE names (
id serial PRIMARY KEY,
name VARCHAR (255) NOT NULL)
"""
)
await con.fetch("SELECT 1")
FooTable.objects.update(**{**bar})
# err
class Foo(Base):
def __init__(self, *args: Any, **kwargs: Any) -> None:
super(Base, self).__init__(*args, **kwargs)
# ok
class Foo(Base):
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
# ok
class Foo(Base, Buzz):
def __init__(self, *args: Any, **kwargs: Any) -> None:
super(Base, self).__init__(*args, **kwargs)
# error
def main() -> bool:
...
if foo > 10:
return True
return False
# ok
def main() -> bool:
...
return foo > 10
if not is_authorized():
return 401
else:
log_activity()
if not is_authorized:
return 401
else if missing:
return 404
else:
log_activity()
if not is_authorized():
return 401
log_activity()
if not is_authorized():
status = 401
else if foobar():
status = 404
log_activity()
def bar() -> list[Foo]:
foos = []
foo = Foo()
foos.append(foo)
return foos
# should instead be
def bar() -> list[Foo]:
return [Foo()]
or more generally:
# ok
foo = Foo()
foos = [foo]
s: Deque[str] = deque(["start"])
foos = []
if buzz:
foos.append(Foo())
# error
foos = []
foo = Foo()
foos.append(foo)
foos = []
foos.append(Foo())
foos = []
foos.append(Foo())
foos.append(Foo())
foos = [Foo()]
foos.append(Foo())
s: Deque[str] = deque()
s.append("start")
can call it something like: prefer-literal
# error
foo.__setitem__("bar" , bar)
# ok
foo["bar"] = bar
buzz(foo.__setitem__)
flake8-bugbear
already uses the B
code for its entry_points[flake8.extension]
, just like flake8-pie
currently does. This makes it impossible to auto-load both flake8-bugbear
and flake8-pie
at the same time. See issue 534 for flake8 over at GitLab for more details.
Per the discussion in the comments at that flake8 issue, since flake8-pie
is much newer, perhaps it should switch to using one of the newly-allowed three-character error code categories... PIE
, perhaps? From the bottom of the docs page here:
Please Note: Your entry point does not need to be exactly 4 characters as of Flake8 3.0. Consider using an entry point with 3 letters followed by 3 numbers (i.e.
ABC123
).
Similar to #75.
# err
foo = len([x for x in bar])
# ok
foo = len(x for x in bar)
# err
try:
assert len(foo) == bar
except AssertionError:
...
try:
assert "@" == bar
except AssertionError:
...
# ok
if len(foo) == bar:
...
try:
assert len(foo) == bar
except (AssertionError, SomeOtherError):
...
try:
assert len(foo) == bar
except AssertionError:
...
raise
Hi!
I see that in https://github.com/sbdchd/flake8-pie/pull/56/files#diff-6dbbed115d61c90646772020134721500e433fe8f7359e4119340fa3772b07ccR7, typing_extensions
is used, however it seems that this is a 3rd party package available on pip and not part of the python stdlib. When I install flake8-pie 0.8, flake8 fails to run with:
File "/Users/xxxxxxx/env/lib/python3.9/site-packages/flake8_pie/__init__.py", line 7, in <module>
from flake8_pie.base import Body, Error, Flake8Error
File "/Users/xxxxxxx/env/lib/python3.9/site-packages/flake8_pie/base.py", line 6, in <module>
from typing_extensions import Protocol
ModuleNotFoundError: No module named 'typing_extensions'
During handling of the above exception, another exception occurred:
Instead can be rewritten as len(set(all_errors))
this might be too infrequent to bother with a rule
# error
foo.startswith("foo") or foo.startswith("bar")
# ok
foo.startswith(("foo", "bar"))
# error
foo.endswith("foo") or foo.endswith("bar")
# ok
foo.endswith(("foo", "bar"))
# err, can be `for foo in bar`
for _idx, foo in enumerate(bar):
...
# err, can be `for value in foo.values()`
for _key, value in foo.items():
...
# err, can be `for key in foo.keys()` or `for key in foo.keys()`
for key, _value in foo.items():
...
This would work better with type checking since .items()
and keys()
can be overloaded, but probably still work syntacticly
I have this warning for code
@celery_app.task()
def recognize_eng_call():
if i leave it like it is, then will be used
def _task_from_fun(self, fun, name=None, base=None, bind=False, **options):
if not self.finalized and not self.autofinalize:
raise RuntimeError('Contract breach: app not finalized')
name = name or self.gen_task_name(fun.__name__, fun.__module__)
and hence in flower i will have
glue.tasks.recognition.recognize_eng_call
IMHO it is good enough like it is and not worth to be rewritten to
@celery_app.task(name='recognize_eng_call')
def recognize_eng_call():
I think this rule could be pretty general but a basic example:
# err
for id in list({x.bar for x in foo}): ...
# ok
for id in {x.bar for x in foo}: ...
for id in list({x.bar for x in foo})[:5]: ...
Flake8 infers the error codes received from the plugins via splitting with " "
, and error messages in
flake8-pie follow the format "[CODE]: [MSG]"
. As a result, when flake8 parses the error codes, codes from flake8-pie incorrectly contain a semicolon at the end.
Wrongly formatted error codes can lead to unexpected behavior when flake8-pie is used with other linters/formatters. For example, currently, yesqa strips all noqas (e.g. # noqa: PIE803
) with a flake8-pie error code since the expected error code (PIE803
) does not match the actual error code (PIE803:
).
Minimal example:
Running
flake8 --no-show-source --format="%(code)s" - <<< 'logger.info("Login error for %s" % user)'
gets you the output
F821
PIE803:
F821
where the error code with a trailing semicolon can be seen.
Suggested fix:
Use only whitespace between the error code and error text in error messages. I would be happy to contribute this PR.
# err
def test_foo() -> None:
try:
bar()
except BarError:
raise pytest.fail("some bad error")
# ok
def test_foo() -> None:
bar()
Accordimg to https://pypi.org/project/flake8-pie/ latest version is 0.15.0 however here there is no git tag for that version.
I'm trying to package your module as rpm packag. So I'm using typical in such case build, install and test cycle used on building package from non-root account:
May I ask for help because few units are failing:
+ PYTHONPATH=/home/tkloczko/rpmbuild/BUILDROOT/python-flake8-pie-0.14.0-2.fc35.x86_64/usr/lib64/python3.8/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/python-flake8-pie-0.14.0-2.fc35.x86_64/usr/lib/python3.8/site-packages
+ /usr/bin/pytest -ra
=========================================================================== test session starts ============================================================================
platform linux -- Python 3.8.11, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/tkloczko/rpmbuild/BUILD/flake8-pie-0.14.0
plugins: forked-1.3.0, shutil-1.7.0, virtualenv-1.7.0, expect-1.1.0, flake8-1.0.7, timeout-1.4.2, betamax-0.8.1, freezegun-0.4.2, case-1.5.3, aspectlib-1.5.2, toolbox-0.5, rerunfailures-9.1.1, requests-mock-1.9.3, cov-2.12.1, pyfakefs-4.5.0, flaky-3.7.0, benchmark-3.4.1, xdist-2.3.0, pylama-7.7.1, datadir-1.3.1, regressions-2.2.0, cases-3.6.3, xprocess-0.18.1, black-0.3.12, checkdocs-2.7.1, anyio-3.3.0, Faker-8.11.0, asyncio-0.15.1, trio-0.7.0, httpbin-1.0.0, subtests-0.5.0, isort-2.0.0, hypothesis-6.14.6, mock-3.6.1, profiling-1.7.0
collected 231 items
. . [ 0%]
flake8_pie/test_utils.py . [ 0%]
flake8_pie/tests/test_pie781_assign_and_return.py ........ [ 4%]
flake8_pie/tests/test_pie783_celery_explicit_names.py ......... [ 8%]
flake8_pie/tests/test_pie784_celery_crontab_args.py .............. [ 14%]
flake8_pie/tests/test_pie785_celery_require_tasks_expire.py ..... [ 16%]
flake8_pie/tests/test_pie786_precise_exception_handler.py ................... [ 24%]
flake8_pie/tests/test_pie787_no_len_condition.py ........ [ 28%]
flake8_pie/tests/test_pie788_no_bool_condition.py ........ [ 31%]
flake8_pie/tests/test_pie789_prefer_isinstance_type_compare.py .......... [ 36%]
flake8_pie/tests/test_pie790_no_unnecessary_pass.py .................... [ 44%]
flake8_pie/tests/test_pie791_no_pointless_statements.py ............ [ 50%]
flake8_pie/tests/test_pie792_no_inherit_object.py ..... [ 52%]
flake8_pie/tests/test_pie793_prefer_dataclass.py ................. [ 59%]
flake8_pie/tests/test_pie794_dupe_class_field_definitions.py .... [ 61%]
flake8_pie/tests/test_pie795_prefer_stdlib_enums.py ........ [ 64%]
flake8_pie/tests/test_pie796_prefer_unique_enums.py .......... [ 69%]
flake8_pie/tests/test_pie797_no_unnecessary_if_expr.py ..... [ 71%]
flake8_pie/tests/test_pie798_no_unnecessary_class.py .......... [ 75%]
flake8_pie/tests/test_pie799_prefer_col_init.py ................. [ 83%]
flake8_pie/tests/test_pie800_no_unnecessary_spread.py .... [ 84%]
flake8_pie/tests/test_pie801_prefer_simple_return.py ... [ 86%]
flake8_pie/tests/test_pie802_prefer_simple_any_all.py .FF. [ 87%]
flake8_pie/tests/test_pie803_prefer_logging_interpolation.py ......... [ 91%]
flake8_pie/tests/test_pie804_no_unnecessary_dict_kwargs.py ...... [ 94%]
flake8_pie/tests/test_pie805_prefer_literal.py .... [ 96%]
flake8_pie/tests/test_pie806_no_assert_except.py ... [ 97%]
flake8_pie/tests/test_pie807_prefer_list_builtin.py .... [ 99%]
flake8_pie/tests/test_pie808_prefer_simple_range.py .. [100%]
================================================================================= FAILURES =================================================================================
_____________________________________________________ test_prefer_simple_any_all[\nany([x.id for x in bar])\n-errors1] _____________________________________________________
code = '\nany([x.id for x in bar])\n', errors = [Error(lineno=2, col_offset=5, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.')]
@pytest.mark.parametrize("code,errors", EXAMPLES)
def test_prefer_simple_any_all(code: str, errors: list[Error]) -> None:
expr = ast.parse(code)
> assert to_errors(Flake8PieCheck(expr, filename="foo.py").run()) == errors
E AssertionError: assert [Error(lineno...prehension.')] == [Error(lineno...prehension.')]
E At index 0 diff: Error(lineno=2, col_offset=4, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.') != Error(lineno=2, col_offset=5, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.')
E Use -v to get the full diff
flake8_pie/tests/test_pie802_prefer_simple_any_all.py:47: AssertionError
_____________________________________________________ test_prefer_simple_any_all[\nall([x.id for x in bar])\n-errors2] _____________________________________________________
code = '\nall([x.id for x in bar])\n', errors = [Error(lineno=2, col_offset=5, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.')]
@pytest.mark.parametrize("code,errors", EXAMPLES)
def test_prefer_simple_any_all(code: str, errors: list[Error]) -> None:
expr = ast.parse(code)
> assert to_errors(Flake8PieCheck(expr, filename="foo.py").run()) == errors
E AssertionError: assert [Error(lineno...prehension.')] == [Error(lineno...prehension.')]
E At index 0 diff: Error(lineno=2, col_offset=4, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.') != Error(lineno=2, col_offset=5, message='PIE802: prefer-simple-any-all: remove unnecessary comprehension.')
E Use -v to get the full diff
flake8_pie/tests/test_pie802_prefer_simple_any_all.py:47: AssertionError
========================================================================= short test summary info ==========================================================================
FAILED flake8_pie/tests/test_pie802_prefer_simple_any_all.py::test_prefer_simple_any_all[\nany([x.id for x in bar])\n-errors1] - AssertionError: assert [Error(lineno...p...
FAILED flake8_pie/tests/test_pie802_prefer_simple_any_all.py::test_prefer_simple_any_all[\nall([x.id for x in bar])\n-errors2] - AssertionError: assert [Error(lineno...p...
====================================================================== 2 failed, 228 passed in 14.51s ======================================================================
pytest-xprocess reminder::Be sure to terminate the started process by running 'pytest --xkill' if you have not explicitly done so in your fixture with 'xprocess.getinfo(<process_name>).terminate()'.
probably have one for ends with as well, or maybe they can the same rule?
# err
if re.search(r"^@.*$", foo):
...
# ok
if foo.startswith("@"):
...
# err
@dataclass
class Foo:
foo: List[str] = field(default_factory=lambda: [])
# ok
@dataclass
class Foo:
foo: List[str] = field(default_factory=list)
Probably also relevant for ORM models like:
class FooTable(BaseTable):
bar = fields.ListField(default=lambda: [])
Can call it something like, prefer-list-constructor
or maybe prefer-list-builtin
# err
Foo(**{"foo": "bar"})
# ok
Foo(foo="bar")
Foo(**{"foo-bar": "bar"})
Foo(**bar)
Essentially, if we have a literal w/ a spread at the call site, we can remove both the spread and top level literal.
# err
all((x.id for x in bar))
any([x.id for x in bar])
# ok
all(x.id for x in bar)
any(x.id for x in bar)
any({x.id for x in bar})
I thought https://github.com/adamchainz/flake8-comprehensions would check for this, but sadly it doesn't
Like technically it changes the meaning a bit, but I think it's okay
maybe there's an edge case when someone overloads this? not sure
class MyClass:
name: str
def update(self, name: str) -> None:
self.name = name
def some_func(self, count: int) -> str:
return f"foo:{count}"
Should be
def some_func(count: int) -> str:
return f"foo:{count}"
class MyClass:
name: str
def update(self, name: str) -> None:
self.name = name
# err
foo = len([x for x in bar if x.buzz > bar])
foo = len([n for n in buzz if n.bar is not True])
foo = len([x for x, bar in results if bar])
# ok
foo = sum(x.buzz > bar for x in bar)
foo = sum(n.bar is not True for n in buzz)
foo = sum(bool(bar) for _, bar in results)
# error
logger.info("Login error for %s" % user)
logger.info("Login error for %s, %s" % (user_id, name))
logger.info("Login error for {}".format(user))
logger.info("Login error for {}, {}".format(user_id, name))
logger.info(f"Login error for {user}")
logger.info(f"Login error for {user_id}, {name}")
# allowed
logger.info("Login error for %s", user)
logger.info("Login error for %s, %s", user_id, name)
{c for c in " /.0123456789"}
can be
set(" /.0123456789")
aka when checking the truthiness of the value, or
is shorter
# err
foo if foo else bar
foo.buzz if foo.buzz else bar
# ok
foo.buzz if cond else bar
# essentially anytime the test in the if expression doesn't match the value being returned in the truthy case, we'd allow it
foo or bar
Since you have "lambda: []
is equivalent to the builtin list
", I suggest other rules with similar refactoring strategies:
lambda: {}
is equivalent to the builtin dict
lambda arg: f(arg)
is equivalent to f
lambda obj: obj.attr
is equivalent to operator.attrgetter('attr')
lambda items: items[0]
is equivalent to operator.itemgetter(0)
lambda obj: obj.name()
is equivalent to operator.methodcaller('name')
The descriptions are still not generic enough, and maybe should not be the ones to be really used.
Some of these rules can be rather complex, in terms of all the possible variations.
# err
not foo == bar
# ok
foo != bar
# bad
def foo():
with db.transaction() as transaction:
with http.Client() as http:
pass
# ok
def foo():
with db.transaction() as transaction, http.Client() as http:
pass
# okay
def foo():
async with db.transaction() as transaction:
with http.Client() as http:
pass
# bad
def foo():
async with db.transaction() as transaction:
async with http.Client() as http:
pass
# ok
def foo():
async with db.transaction() as transaction, http.Client() as http:
pass
# err
range(0, 10)
# ok
range(10)
range(0, 10, x)
# err
"foo".encode()
# ok
b"foo"
# error
[Item.objects.create(item) for item in items]
# error
[Item.objects.create(item) for item in [bar for bar in buzz]]
# error
(Item.objects.create(item) for item in items)
# ok
Item.objects.insert(items)
Item.objects.create(item)
# err
not [user for user in users if user.name is not null]
# ok
not len(user for user in users if user.name is not null)
I didn't initially see the flake8 validation codes in your README as they were part of section titles. You also don't list the actual error messages which could help with Google searches etc (and would have likely caught the typos fixed in #12 earlier).
How about a simple table at the start of the lints section?
f
sigil.I'd also document the meaning behind the number 781 (if there is one) and abbreviation PIE (which puzzles me).
P.S. Would you be happy with "prefix" rather than "sigil" in PIE782?
Basically, for things like DB clients, prefer using the with
API instead of manually calling close
# err
conn = psycopg2.connect(dsn=dsn)
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.close()
conn.close()
# ok
with psycopg2.connect(dsn=dsn) as conn, conn.cursor() as cursor:
cursor.execute("SELECT 1")
I think this requires type information since we need to check that the type has a close
and has an __exit__
, we may also need to manually specify the supported types for the lint since not all __exit__
s will call close
# err
redis.set(redis_key, data)
redis.expire(redis_key, timedelta(days=1))
# ok
redis.set(redis_key, data, ex=timedelta(days=1))
Note: I think we'd want to ignore the name of the client, and look for methods .set
and .expire
that are next to each other
Error from installing via pip under Python 2.7:
$ python -m pip install flake8-pie
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
Collecting flake8-pie
Downloading https://files.pythonhosted.org/packages/f0/ea/ee42a4eafe323282fb59c700f9c70e94054e6e4e0ac0dc59dfd37aad7195/flake8-pie-0.2.1.tar.gz
ERROR: Complete output from command python setup.py egg_info:
ERROR: Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/private/var/folders/x7/ygl010ps57g2s5q_l8y_jyjdmzsyc0/T/pip-install-RKc929/flake8-pie/setup.py", line 6
def get_version(fname: str) -> Optional[str]:
^
SyntaxError: invalid syntax
----------------------------------------
ERROR: Command "python setup.py egg_info" failed with error code 1 in /private/var/folders/x7/ygl010ps57g2s5q_l8y_jyjdmzsyc0/T/pip-install-RKc929/flake8-pie/
At this point tweaks to work under Python 2 are not the best use of time, but could you document this in your README.md
please?
I take it from the metadata in setup.py
that you need Python 3.6 onwards?
Hi, I represent https://github.com/wemake-services/wemake-python-styleguide project.
Awesome tool! We have the same rules:
f
strings: https://wemake-python-styleguide.readthedocs.io/en/latest/pages/violations/consistency.html#wemake_python_styleguide.violations.consistency.FormattedStringViolationAnd since our plugins are so similar I would be happy to list your plugin in our docs. Maybe you could do the same?
Original issue: wemake-services/wemake-python-styleguide#471 (comment)
# err
@app.task(name="foo")
def foo() -> None:
...
# err
@shared_task(name="foo")
def foo() -> None:
...
# ok
@app.task(name="foo", ignore_result=True)
def foo() -> None:
...
# ok
@shared_task(name="foo", ignore_result=True)
def foo() -> None:
...
This might not be wise since AsyncResult
can't be used to check the status if ignore_result
is set to True
https://docs.celeryproject.org/en/stable/userguide/tasks.html#Task.ignore_result
If a class has no state and only classmethods and/or static methods, it doesn't need to be a class. The methods can be normal functions.
Here's a complete nonsense example that covers the use case.
from typing import NamedTuple
class UserManager:
"""
some class
"""
class User(NamedTuple):
name: str
@classmethod
def update_user(cls, user: User) -> None:
...
@staticmethod
def sync_users() -> None:
...
This should be:
from typing import NamedTuple
class User(NamedTuple):
name: str
def update_user(user: User) -> None:
...
def sync_users() -> None:
...
# error
{**foo, **{"bar": True }}
# ok
{**foo, "bar": True }
Table.objects.filter(inst=inst, **{f"foo__{bar}__exists": True})
Tested on flake8-pie-0.4.2
test.py
v = 3.14159
print(f"{v:0.2f}")
$ python3 test.py
3.14
$ flake8 --select PIE782 test.py
test.py:2:7: PIE782: Unnecessary f-string. You can safely remove the `f` prefix.
But the f
is not safe to remove.
# incorrect
logging.info("Login error for %s, %s", (user_id, name))
# correct
logging.info("Login error for %s, %s", user_id, name)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.