From 386974f0ddd57f7cbdaa6e58c13a82bd90ee364d Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Wed, 5 Oct 2022 15:19:41 -0700 Subject: [PATCH] Cleanup the "in tuple" check: * Remove the "performance" note from the --explain docs since the performance is the same in both cases, and instead say that it is for style/consistency. * Add support for parsing `not in` expressions. * Update message to use the "replace x with y" format. --- refurb/checks/iterable/in_tuple.py | 27 ++++++++++++++++----------- test/data/err_109.py | 3 +++ test/data/err_109.txt | 13 +++++++------ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/refurb/checks/iterable/in_tuple.py b/refurb/checks/iterable/in_tuple.py index c127ac4..314099d 100644 --- a/refurb/checks/iterable/in_tuple.py +++ b/refurb/checks/iterable/in_tuple.py @@ -8,8 +8,8 @@ @dataclass class ErrorInfo(Error): """ - Since tuples cannot change value over time, it is more performant to use - them in `for` loops, generators, etc.: + Since tuple, list, and set literals can be used with the `in` operator, it + is best to pick one and stick with it. Bad: @@ -31,23 +31,28 @@ class ErrorInfo(Error): """ code = 109 - msg: str = "Use `in (x, y, z)` instead of `in [x, y, z]`" + + +def error_msg(oper: str) -> str: + return f"Replace `{oper} [x, y, z]` with `{oper} (x, y, z)`" def check( node: ComparisonExpr | ForStmt | GeneratorExpr, errors: list[Error] ) -> None: match node: - case ( - ComparisonExpr( - operators=["in"], - operands=[_, ListExpr() as expr], - ) - | ForStmt(expr=ListExpr() as expr) + case ComparisonExpr( + operators=["in" | "not in" as oper], + operands=[_, ListExpr() as expr], ): - errors.append(ErrorInfo(expr.line, expr.column)) + errors.append(ErrorInfo(expr.line, expr.column, error_msg(oper))) + + case ForStmt(expr=ListExpr() as expr): + errors.append(ErrorInfo(expr.line, expr.column, error_msg("in"))) case GeneratorExpr(): for expr in node.sequences: # type: ignore if isinstance(expr, ListExpr): - errors.append(ErrorInfo(expr.line, expr.column)) + errors.append( + ErrorInfo(expr.line, expr.column, error_msg("in")) + ) diff --git a/test/data/err_109.py b/test/data/err_109.py index 5d66504..33078c3 100644 --- a/test/data/err_109.py +++ b/test/data/err_109.py @@ -15,6 +15,9 @@ if 1 in [1, 2, 3]: pass +if 1 not in [1, 2, 3]: + pass + # these will not diff --git a/test/data/err_109.txt b/test/data/err_109.txt index d5a4932..b9279c2 100644 --- a/test/data/err_109.txt +++ b/test/data/err_109.txt @@ -1,6 +1,7 @@ -test/data/err_109.py:3:10 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` -test/data/err_109.py:6:13 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` -test/data/err_109.py:8:13 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` -test/data/err_109.py:11:22 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` -test/data/err_109.py:12:14 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` -test/data/err_109.py:15:9 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]` +test/data/err_109.py:3:10 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:6:13 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:8:13 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:11:22 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:12:14 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:15:9 [FURB109]: Replace `in [x, y, z]` with `in (x, y, z)` +test/data/err_109.py:18:13 [FURB109]: Replace `not in [x, y, z]` with `not in (x, y, z)`