Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

B018 doesn't trigger for useless expressions involving multiple variables #452

Open
swierh opened this issue Jan 16, 2024 · 3 comments
Open

Comments

@swierh
Copy link

swierh commented Jan 16, 2024

I recently ran into the following bug and was surprised it wasn't flagged by B018:

def some_funcion(a, b, c, d):
    result = a * b
    +c * d

    return result

that should have been

def some_funcion(a, b, c, d):
    result = (
        a * b
        + c * d
    )

    return result

It looks like B018 doesn't trigger for expressions containing multiple variables.

Here's some examples:

a * b     # doesn't trigger B018
+a * b    # doesn't trigger B018
+1 * 2    # doesn't trigger B018
-1 * 2    # doesn't trigger B018
1 * 2 + 3 # doesn't trigger B018
a + 1     # doesn't trigger B018
a         # triggers B018
(a, b)    # triggers B018
1         # triggers B018
+1        # triggers B018
-1        # triggers B018

Versions I'm using:

python version 3.10.13
flake8 version 7.0.0
flake8-bugbear version 23.12.2
ruff version 0.1.13

@cooperlees
Copy link
Collaborator

Agree this would be nice to add to B018. Thanks for reporting.

@JelleZijlstra
Copy link
Collaborator

"multiple variables" isn't a great description of what's going on here. If you look at the code for B018, it triggers if a statement consists only of an expression that is a set/list/tuple/dict literal or a non-str constant. (The original post claims that B018 triggers for +1 and -1, but I don't think it does.)

We could expand the rule to some other kinds of nodes:

  • UnaryOp, like +1, not x, ~3. I don't know of any code patterns that would trigger false positives.
  • BinOp, like 1 + 1 or 2 << 3. This is what the OP is asking for. This could have false positives with Airflow code, where you do dag << task to add a task to a dag.
  • Compare, like 1 < 2. Can't think of any problems with this one.
  • BoolOp, like a and b. People could be using this for conditional execution.
  • GeneratorExp, like (x for x in y). That is definitely useless.
  • JoinedStr, which is an f-string. Having an f-string in a line by itself doesn't have any use.

So we could definitely expand B018 to a few more kinds of AST nodes, but there's some room for false positives.

@JelleZijlstra
Copy link
Collaborator

I remembered that internally in our codebase we have a similar lint rule. It triggers on these types:

ast.Attribute, ast.Lambda, ast.Dict, ast.Set, ast.Compare, ast.Num, ast.Subscript, ast.List, ast.Tuple, ast.Name

So that's missing a few that I mentioned above. We also flag ast.Call nodes if the called function is one of sorted, reversed, map, and filter. If you write sorted(x) on a line by itself, you probably meant x.sort().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants