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

Forbid to use continue in finally #1082

Closed
sobolevn opened this issue Dec 19, 2019 · 12 comments · Fixed by #1179
Closed

Forbid to use continue in finally #1082

sobolevn opened this issue Dec 19, 2019 · 12 comments · Fixed by #1179
Assignees
Labels
help wanted Extra attention is needed level:starter Good for newcomers pr-merged rule request Adding a new rule

Comments

@sobolevn
Copy link
Member

Rule request

Thesis

This code is now allowed:

for some in range(10):
    try:
        func(some)
    finally:
        continue

Why would you do that?
Putting any control statements in finally is a terrible practice.
We should not allow it.

It was added in python3.8: https://www.python.org/downloads/release/python-381/

@sobolevn sobolevn added help wanted Extra attention is needed level:starter Good for newcomers rule request Adding a new rule labels Dec 19, 2019
@sobolevn sobolevn added this to the Version 0.14 aka python3.8 milestone Dec 19, 2019
@Kvm99
Copy link
Contributor

Kvm99 commented Feb 15, 2020

Hi, could I take the issue?

@sobolevn
Copy link
Member Author

Sure! Feel free to ask for help.

@Kvm99
Copy link
Contributor

Kvm99 commented Feb 18, 2020

Hi! I am trying to run and write tests and have an error:
ModuleNotFoundError: No module named 'plugins'
I could not find any information about such module in the internet and have no idea how to deal with the problem.
Could you help me, please?

@sobolevn
Copy link
Member Author

Sure! It looks like a problem with your environment.

Can you please post how exactly you run your tests?
You can also take a look at how we run our tests here: https://github.com/wemake-services/wemake-python-styleguide/blob/master/.travis.yml#L19-L20

@Kvm99
Copy link
Contributor

Kvm99 commented Feb 19, 2020

I tried to run tests from the project root by make test and pytest. Each time there was the same error: ModuleNotFoundError: No module named 'plugins'
Then I changed plugins.violation to test.plugins.violation in conftest.py and it helped.

@sobolevn
Copy link
Member Author

Two questions:

  1. How does your installation process looks like? Because it is proven to work on CI and locally
  2. What is your OS?

@Kvm99
Copy link
Contributor

Kvm99 commented Feb 19, 2020

I was going the following way:

pyenv update
pyenv local 3.6.7
pip3 install poetry
poetry install
poetry shell
make test

OS: Ubuntu 18.04

@sobolevn
Copy link
Member Author

Looks strange. This should do the trick.

Tow comments, however:

  1. Use should use 3.8.0, because this feature is only possible there
  2. You can try to activate env instead of using poetry shell. Try source $(poetry env info -p)/bin/activate

@sobolevn
Copy link
Member Author

@Kvm99 can you please also test break keyword in finally? Is it allowed? If so, we need to forbid it as well.

@sobolevn
Copy link
Member Author

Please, also specify that this rule is related to TryExceptMultipleReturnPathViolation

@Kvm99
Copy link
Contributor

Kvm99 commented Feb 25, 2020

@Kvm99 can you please also test break keyword in finally? Is it allowed? If so, we need to forbid it as well.

I checked. break in finally works with python 3.6, 3.7, 3.8.
Should not we add a new issue for such case? I will also do it.

@sobolevn
Copy link
Member Author

We can combine them into one: continue and break

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed level:starter Good for newcomers pr-merged rule request Adding a new rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants