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

WPS440/441 on multiple loops #1416

Open
Dreamsorcerer opened this issue May 24, 2020 · 16 comments
Open

WPS440/441 on multiple loops #1416

Dreamsorcerer opened this issue May 24, 2020 · 16 comments
Labels
bug Something isn't working help wanted Extra attention is needed level:starter Good for newcomers

Comments

@Dreamsorcerer
Copy link
Contributor

With code like:

        for widget in self._widgets:
            await widget.update()
        ...
        for widget in self._widgets:
            await widget.set_something(state)

I get WPS440 and WPS441 due to reusing the widget variable name in the second loop.

I think that WPS441 shouldn't apply, as we are defining a new variable, as opposed to reusing the previous value from the last loop.

Then I think also, that WPS440 shouldn't apply, as the WPS441 stops you from reusing the first variable anyway, so shadowing a variable you're not supposed to be using is not an issue. i.e. If WPS441 is going to make you treat them as block-scoped variables, then shadowing them shouldn't be taken into account as they shouldn't be treated as being in the enclosing scope.

@Dreamsorcerer Dreamsorcerer added the bug Something isn't working label May 24, 2020
@anatoly-scherbakov
Copy link

Another example of the same issue with except blocks.

        try:
            sqs_response = self.client.send_message(
                QueueUrl=self.url,
                MessageBody=message_body,
            )

        except self.client.exceptions.QueueDoesNotExist as err:
            raise SQSQueueDoesNotExist(queue=self) from err

        except self.client.exceptions.ClientError as err:
            if self._error_code_is(err, 'InvalidParameterValue'):
                raise MessageTooLarge(
                    max_supported_size=MAX_MESSAGE_SIZE,
                    message_body=message_body,
                )

cause 440 and 441:

  38:9     WPS440 Found block variables overlap: err
  except self.client.exceptions.ClientError as err:
  ^

  39:36    WPS441 Found control variable used after block: err
  if self._error_code_is(err, 'InvalidParameterValue'):

@huangyunzen
Copy link
Contributor

Hello, @Elton-Lin and I would like to work on fixing this. Thanks!

@sobolevn sobolevn added help wanted Extra attention is needed level:starter Good for newcomers labels Nov 10, 2020
@sobolevn sobolevn added this to the Version 0.15 aka Python3.9 milestone Nov 10, 2020
@sobolevn
Copy link
Member

Awesome! Thanks!

@huangyunzen
Copy link
Contributor

Another example of the same issue with except blocks.

        try:
            sqs_response = self.client.send_message(
                QueueUrl=self.url,
                MessageBody=message_body,
            )

        except self.client.exceptions.QueueDoesNotExist as err:
            raise SQSQueueDoesNotExist(queue=self) from err

        except self.client.exceptions.ClientError as err:
            if self._error_code_is(err, 'InvalidParameterValue'):
                raise MessageTooLarge(
                    max_supported_size=MAX_MESSAGE_SIZE,
                    message_body=message_body,
                )

cause 440 and 441:

  38:9     WPS440 Found block variables overlap: err
  except self.client.exceptions.ClientError as err:
  ^

  39:36    WPS441 Found control variable used after block: err
  if self._error_code_is(err, 'InvalidParameterValue'):

We believe that the issue with the except blocks is fixed in this issue:
#1115

@huangyunzen
Copy link
Contributor

@sobolevn Should we work on modifying both rules WPS 440/441 so that they don't trigger on multiple for loops?

@sobolevn
Copy link
Member

sobolevn commented Dec 5, 2020

Yes, @huangyunzen, please!

@huangyunzen
Copy link
Contributor

WPS441 also applies to "with blocks" in addition to the for blocks mentioned in the issue - should we fix them too?

@sobolevn
Copy link
Member

sobolevn commented Dec 9, 2020

Yes, please 👍

@huangyunzen
Copy link
Contributor

@sobolevn Is it possible to separate this issue into two issues? We are done with fixing WPS441, but WPS440 might take some more time and we were wondering if we can submit a pull request now for only 441.

@sobolevn
Copy link
Member

I can always accept a PR with a partial fix! 👍

@sobolevn
Copy link
Member

sobolevn commented Feb 6, 2021

Done!

@sobolevn sobolevn closed this as completed Feb 6, 2021
@fpuga
Copy link

fpuga commented Jun 24, 2021

Can this ticket be reopened?

I'm still getting a WPS440 in code like the shown in the first comment with version 0.15.3

for widget in self._widgets:
    await widget.update()
...
for widget in self._widgets:
    await widget.set_something(state)

@sobolevn
Copy link
Member

Reproduction:

async def some():
    for widget in self._widgets:
        await widget.update()

    for widget in self._widgets:
        await widget.set_something(state)

Снимок экрана 2021-06-24 в 19 14 09

@strmwalker
Copy link

Any updates?

@Xoriun
Copy link

Xoriun commented Jan 17, 2024

If the first loop is a proper loop and the second loop if a comprehension, WPS441 is raised in the comprehension variable:

   for widget in self._widgets:
      await widget.update()
   ...
   return [widget.get_value() for widget in self._widgets]
           ^^^^^^

@Xoriun
Copy link

Xoriun commented Mar 25, 2024

cond = False
index = 4

if cond:
    for index in range(465):
        print(index)
else:
    print(index)

This raises an WPS440 error in the last row, even though if execution reached that point, the loop would not have been executed and index would still be 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed level:starter Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants