Pylint: Add scanning for assignment expressions (PEP 572)

[gappleto97]

Many developers have expressed opposition to this PEP, including a desire to ban this syntax from their projects. If there was a flag to scan for this syntax and register it as an error, this would facilitate them being able to do so.

I would prefer it if that were a default option that they had to explicitly disable,

In addition, scanning for this at a top level should be enabled by default. The following, for instance, should always be an error regardless of the flag:

a = 3  # no error
b := 4  # error, doesn't need to be at top level
c = d := 5  # error, can be done better with chained assignment

[PCManticore]

I'm pretty sure I don't want to support this syntax in pylint either :( But it's kind of early now, as it's not implemented yet, so there's no AST yet to play with it.

[gappleto97]

There actually is a CPython fork that implements it right now, though I imagine it is subject to change. Mostly I just wanted to get ahead of the game so that there would be support for such a measure by the time we get a beta.

[13steinj]

To have to have developers explicitly disable a part of the language syntax is a tad ridiculous.

[PCManticore]

I'm pretty sure that's the purpose of every linter out there, to disable patterns in your code that will result in unmaintainable, slow or hard to comprehend code. This walrus operator is just that, a confusing syntax that's going to be abused to no avail, so I don't find disabling it ridiculous at all, as pylint is already opinionated with some of its messages.

[13steinj]

That's not what I mean.

If the maintainer of some code wants to ban the expression, sure, fine.

But it shouldn't be banned by default. It is a language feature, and therefore if the maintainer wants to ban it, they should have to opt in to banning it.

Pylint checks for general errors in Python code and naming standards by PEP8 (the latter being irrelevant here)-- using assignment expressions at the top level isn't an error, it is valid Python.

Using assignment expressions in chaining is not an error, it is valid Python code, and in fact the PEP specifies how the assignment expression equivalent of normal chaining (however, the mix of chaining how @gappleto97 showed is in fact an error because it isn't parenthesized).

If the linter chooses to default to behavior that is unwanted by the maintainer of the linter but allowed by the language, it becomes a slippery slope for other new Python features that are added in the future.

Furthermore, I believe (may be incorrect), but this would be completely out of scope for PyLint to begin with. Given the fact that the documentation states it checks for errors and naming style, not for general behavior, it really shouldn't be here at all (besides checking for all the cases as defined in the PEP where := is not allowed / must be parenthisized / no annotations on :=). This request moreso fits a custom pycodestyle/flake8 optional plugin, rather than PyLint or any other linter.

[PCManticore]

By this argument, we shouldn't warn about the use exec or eval, as they are fine language constructs that might be correctly by the author, or we shouldn't suggest any refactoring message at all, as they might not be actual errors in the codebase, just a different way of writing the code.

The truth is that pylint moved away from being just a linter for general errors and naming standards quite some time ago, and nowadays it checks for all sorts of patterns that might result in unmaintainable, hard to reason or complicated code, as well as checking for general errors and some type checking errors. One thing that never changes with it though is the ability to disable any check you might disagree with from the get go. And if is out of the scope of pylint, that's for the maintainers and committers to the project to decide where they want to take the future of the tool. If this is unclear from the documentation, it's something that we can address.

Regarding this check itself, if it proves that the community absolutely love this feature and want this check disabled by default in the linter, we can address that as well. But as things stand out today, it was a feature added against the opposition of many CPython core developers, which shows just how dividing this feature is.

[AWhetter]

I agree with everything that @PCManticore has said. Perhaps it's a matter of getting used to it, but I think that the operator makes code less readable enough that warning about its usage by default is a good idea. There will of course be the option to disable the check.

[13steinj]

By this argument, we shouldn't warn about the use exec or eval, as they are fine language constructs that might be correctly by the author, or we shouldn't suggest any refactoring message at all, as they might not be actual errors in the codebase, just a different way of writing the code.

But does PyLint call this an error, or simply a warning?

And if is out of the scope of pylint, that's for the maintainers and committers to the project to decide where they want to take the future of the tool. If this is unclear from the documentation, it's something that we can address.

Absolutely-- then the documentation needs to be addressed. Because as it stands,

Pylint is a tool that checks for errors in Python code, tries to enforce a coding standard and looks for code smells.

And this request, from my reading, wants this to be an error. But it isn't an error. Bad standard. Sure. Smelly, sure.

There are five different categories of check PyLint checks against according to the docs:

• (C) convention, for programming standard violation
• (R) refactor, for bad code smell
• (W) warning, for python specific problems
• (E) error, for much probably bugs in the code
• (F) fatal, if an error occurred which prevented pylint from doing further processing.

The use of := is not a fatal issue, nor a "probable bug", because it has both top level and condition expression uses that are normal.

Perfectly understandable as per the documentation if this is say, by default, and treated as bad convention / a warning / even a refactor needed, depending on the case.

But to call it by default as an error is strange.

Nowhere in the docs does it say that PyLint gets to have an opinion on what is and what is not an error. An opinion on the first three categories make sense, but on errors, it does not.

[PCManticore]

I think I get what you're saying now. It most likely won't be an error, that is won't be part of the Error category, which is reserved for type checking and syntax errors, so no need to worry about that. I think the disconnect came from the original comment of the issue, which seems to suggest that this will be an error by default. Again this is not the case, most likely it's going to be a warning or a convention message.

[13steinj]

I think the disconnect came from the original comment of the issue, which seems to suggest that this will be an error by default.

Yeah, that was what threw a spanner in my mental gears. If it's a warning / convention message, I'm perfectly fine with that-- it makes sense.

[gappleto97]

I do still think that throwing an assignment expression into chained assignment should be a straight up error. My apologies for not being as nuanced with the rest.

[13steinj]

@gappleto97, it really depends on what you mean. Because

a = b := c

is an error as defined by the PEP. But,

a = (b := c)

is not , nor is

x = a := (b := (c := d))

Furthermore they have a different meaning then regular assignment chaining-- in assignment chaining the right most literal either gets unpacked or mirrored while traversing left, while assignment expressions go inner to outer, right to left, no unpacking.

Edit: confused my directions.

[13steinj]

Commenting to correct myself, because at some point more recently a change to the PEP was introduced that disallows assignment expressions at the top level without parenthesis-- so

a := 5

Would now be an error, and so is

x = a := (b := (c := d))

Because the assignment expression of a is not parenthesized, and leaving them unparenthesized would be an error. Making them parenthesized works, but is of course ugly.

More examples of what is and isn't invalid beyond the obvious "allowed in control flow and comprehensions" can be seen here

[Pk13055]

What about something like this:

if (var_x := foo(args, **kwargs)) > 0:
    # some processing here with `var_x`
    pass

[13steinj]

@Pk13055 that's fully allowed by the language. Again, I wouldn't want that to be listed as an error, because it simply is not. Refactor or warning, sure, but that depends on the relevance of the issue. It seems like people aren't even complaining about := anymore (this issue is 2 years old), similar to f-strings.