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

Report unassigned expressions #55

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented Dec 5, 2015

Resolves lp:1523001

@asmeurer
Copy link
Contributor

asmeurer commented Feb 8, 2016

I am -0.5. There are legitimate use-cases for calling == etc. without assigning it. Worse, if someone tries to assign it to work around this, they will hit an unused variable warning.

I really think there needs to be a separate project, perhaps depending on pyflakes, for these "probably wrong, but not technically wrong" errors. They don't really fit pep8 (they aren't in the PEP, and aren't really style issues), but they don't follow pyflakes' "no false positives" rule.

@sigmavirus24
Copy link
Member

I tend to agree with @asmeurer here. This can be developed as a flake8 plugin easily. Otherwise, I don't think it makes sense for here or pep8.

@ambv
Copy link
Member

ambv commented Mar 14, 2016

This is a flake8 plugin I would use! But it should be on an opt-in basis because it presents a design problem, not a clear-cut error. As such, it doesn't belong to pyflakes (as discussed at length in #59).

As I understand the problem, it trips up people coming from Ruby, expecting the last expression in a function to be returned from the block. Other usages are harder to judge. There's many examples of code using list comprehensions as single-line for-loops. It's questionable style but it's not technically incorrect.

@@ -1262,7 +1265,7 @@ def test_assignInListComprehension(self):
"""
self.flakes('''
def f():
[None for i in range(10)]
return [None for i in range(10)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if list comprehensions might be unassigned when people use them to map a function with side effects to a sequence. For example:

[frobnicate(x) for x in frobnitzen]

Personally I use map() or a for loop in these instances, but I have seen the above as well.

@bitglue
Copy link
Member

bitglue commented Mar 15, 2016

Does this have any tests asserting that the new functionality works?

What's the reason for the special doctest cases in the tests?

I also wonder if in some instances an apparently useless expression is being used to invoke a side effect of some object with overridden operators. Some people think < and > look like arrows, for example, and so override these operators to mean something like "put into". I'd like to see this change run against some existing code to see if this will be a problem.

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

Successfully merging this pull request may close these issues.

5 participants