From d3b0b46758006f9667885ce1e5eedfc94e1d49d0 Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Fri, 19 Jan 2024 15:44:16 -0800 Subject: [PATCH] Add more cases to FURB111: FURB111 now detects instances of `lambda: x`, where `x` is the default literal value for a builtin type. For example, the default value of `bool()` is `False`, `int()` is `0`, and so on. Similar to how `lambda: []` should be written as `list`, instances of `lambda: 0` (and other literal values) should be written as `int`. Pyupgrade can already detect these new cases, but for whatever reason, it only detects them in `defaultdict()` calls and nowhere else. With that in mind I decided to implement this anyways, but without the `defaultdict()` restriction. Pyupgrade also doesn't check `lambda: b""`, though this is probably not very common. --- docs/checks.md | 22 +++++- pyproject.toml | 2 + refurb/checks/readability/use_func_name.py | 82 +++++++++++++++------- test/data/err_111.py | 17 +++++ test/data/err_111.txt | 6 ++ 5 files changed, 100 insertions(+), 29 deletions(-) diff --git a/docs/checks.md b/docs/checks.md index 9bb11e6..9947271 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -240,10 +240,9 @@ Note: if `x` depends on side-effects, then this check should be ignored. ## FURB111: `use-func-name` -Categories: `readability` +Categories: `performance` `readability` -Don't use a lambda if it is just forwarding its arguments to a -function verbatim: +Don't use a lambda if its only forwarding its arguments to a function. Bad: @@ -261,6 +260,23 @@ predicate = bool some_func(print) ``` +In addition, don't use lambdas when you want a default value for a literal +type: + +Bad: + +```python +counter = defaultdict(lambda: 0) +multimap = defaultdict(lambda: []) +``` + +Good: + +```python +counter = defaultdict(int) +multimap = defaultdict(list) +``` + ## FURB112: `use-literal` Categories: `pythonic` `readability` diff --git a/pyproject.toml b/pyproject.toml index 17917a4..43cf862 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,6 +60,7 @@ omit = [ exclude_lines = [ "pragma: no cover", "if TYPE_CHECKING:", + "assert False", ] skip_covered = true skip_empty = true @@ -120,6 +121,7 @@ target-version = "py310" [tool.ruff.per-file-ignores] "test/*" = ["ANN201", "ARG001", "E501", "TCH001", "TCH002"] +"refurb/*" = ["PT"] "refurb/main.py" = ["E501"] "refurb/visitor/traverser.py" = ["ALL"] "test/e2e/gbk.py" = ["FURB105"] diff --git a/refurb/checks/readability/use_func_name.py b/refurb/checks/readability/use_func_name.py index b40ba03..3a4075f 100644 --- a/refurb/checks/readability/use_func_name.py +++ b/refurb/checks/readability/use_func_name.py @@ -4,14 +4,19 @@ ArgKind, Argument, Block, + BytesExpr, CallExpr, + ComplexExpr, DictExpr, Expression, + FloatExpr, + IntExpr, LambdaExpr, ListExpr, NameExpr, RefExpr, ReturnStmt, + StrExpr, TupleExpr, ) @@ -22,8 +27,7 @@ @dataclass class ErrorInfo(Error): """ - Don't use a lambda if it is just forwarding its arguments to a - function verbatim: + Don't use a lambda if its only forwarding its arguments to a function. Bad: @@ -40,11 +44,28 @@ class ErrorInfo(Error): some_func(print) ``` + + In addition, don't use lambdas when you want a default value for a literal + type: + + Bad: + + ``` + counter = defaultdict(lambda: 0) + multimap = defaultdict(lambda: []) + ``` + + Good: + + ``` + counter = defaultdict(int) + multimap = defaultdict(list) + ``` """ name = "use-func-name" code = 111 - categories = ("readability",) + categories = ("performance", "readability") def get_lambda_arg_names(args: list[Argument]) -> list[str]: @@ -60,50 +81,59 @@ def check(node: LambdaExpr, errors: list[Error]) -> None: case LambdaExpr( arguments=lambda_args, body=Block( - body=[ - ReturnStmt(expr=CallExpr(callee=RefExpr() as ref) as func), - ] + body=[ReturnStmt(expr=CallExpr(callee=RefExpr() as ref) as func)], ), ) if ( get_lambda_arg_names(lambda_args) == get_func_arg_names(func.args) and all(kind == ArgKind.ARG_POS for kind in func.arg_kinds) ): func_name = stringify(ref) - arg_names = get_lambda_arg_names(lambda_args) - arg_names = ", ".join(arg_names) if arg_names else "" - _lambda = f"lambda {arg_names}" if arg_names else "lambda" + msg = f"Replace `{stringify(node)}` with `{func_name}`" - errors.append( - ErrorInfo.from_node( - node, - f"Replace `{_lambda}: {func_name}({arg_names})` with `{func_name}`", # noqa: E501 - ) - ) + errors.append(ErrorInfo.from_node(node, msg)) case LambdaExpr( arguments=[], body=Block( body=[ ReturnStmt( - expr=ListExpr(items=[]) | DictExpr(items=[]) | TupleExpr(items=[]) as expr, + expr=( + ListExpr(items=[]) + | DictExpr(items=[]) + | TupleExpr(items=[]) + | IntExpr(value=0) + | FloatExpr(value=0.0) + | ComplexExpr(value=0j) + | NameExpr(fullname="builtins.False") + | StrExpr(value="") + | BytesExpr(value="") + ) as expr, ) ], ), ): if isinstance(expr, ListExpr): - old = "[]" new = "list" elif isinstance(expr, DictExpr): - old = "{}" new = "dict" - else: - old = "()" + elif isinstance(expr, TupleExpr): new = "tuple" + elif isinstance(expr, IntExpr): + new = "int" + elif isinstance(expr, FloatExpr): + new = "float" + elif isinstance(expr, ComplexExpr): + new = "complex" + elif isinstance(expr, NameExpr): + new = "bool" + elif isinstance(expr, StrExpr): + new = "str" + elif isinstance(expr, BytesExpr): + new = "bytes" + else: + assert False, "unreachable" # noqa: B011 + + msg = f"Replace `{stringify(node)}` with `{new}`" - errors.append( - ErrorInfo.from_node( - node, - f"Replace `lambda: {old}` with `{new}`", - ) - ) + errors.append(ErrorInfo.from_node(node, msg)) diff --git a/test/data/err_111.py b/test/data/err_111.py index b5f239d..9ff4b1d 100644 --- a/test/data/err_111.py +++ b/test/data/err_111.py @@ -16,6 +16,13 @@ def f(x, y): lambda x: mod.cast(x) +_ = lambda: 0 +_ = lambda: 0.0 +_ = lambda: 0j +_ = lambda: False +_ = lambda: "" +_ = lambda: b"" + # these will not @@ -31,3 +38,13 @@ def f(x, y): lambda: [1, 2, 3] lambda: {"k": "v"} lambda: (1, 2, 3) + +lambda: None +lambda: 1 +lambda: 1.2 +lambda: True +lambda: "abc" +lambda: b"abc" +lambda: 1j +lambda: set() # noqa: FURB111 +lambda: {"x"} diff --git a/test/data/err_111.txt b/test/data/err_111.txt index 393efa4..118b18e 100644 --- a/test/data/err_111.txt +++ b/test/data/err_111.txt @@ -5,3 +5,9 @@ test/data/err_111.py:13:1 [FURB111]: Replace `lambda: []` with `list` test/data/err_111.py:14:1 [FURB111]: Replace `lambda: {}` with `dict` test/data/err_111.py:15:1 [FURB111]: Replace `lambda: ()` with `tuple` test/data/err_111.py:17:1 [FURB111]: Replace `lambda x: mod.cast(x)` with `mod.cast` +test/data/err_111.py:19:5 [FURB111]: Replace `lambda: 0` with `int` +test/data/err_111.py:20:5 [FURB111]: Replace `lambda: 0.0` with `float` +test/data/err_111.py:21:5 [FURB111]: Replace `lambda: 0j` with `complex` +test/data/err_111.py:22:5 [FURB111]: Replace `lambda: False` with `bool` +test/data/err_111.py:23:5 [FURB111]: Replace `lambda: ""` with `str` +test/data/err_111.py:24:5 [FURB111]: Replace `lambda: b""` with `bytes`