Wemake-python-styleguide: Warn about possibly infinite `while`

[orsinium]

Rule request

Check that while True block has raise or return.

UPD: and break too.

Thesis

It's OK to not have them for CLI entry point, but in other cases, it can be an error. So, let's ask users explicitly noqa such cases to be sure that they really know what they're doing.

Reasoning

[sobolevn]

break is also an option. I like this one!

[ragohin]

Hi, I've been looking to contribute to this project and would like to try and implement this rule if nobody is on it!

[sobolevn]

Sure, I would be happy to answer your questions! Thanks!

[ragohin]

Hi there! After perusing through the source code and the tutorial documentation, I was thinking that this rule would best fit in wemake_python_styleguide\visitors\ast\loops.py. Does this seem like a good place to start?

[sobolevn]

Yes, it does! Do you want to take this one?

Feel free to ask for help! 👍

[ragohin]

Sorry about the delayed responses on this issue! I'll work on this implementation tomorrow. Thanks!

[ragohin]

I was able to successfully call "make test" without any errors when I first cloned the repo, but after implemented a few changes, I go the following error complaining that "no shebang is present:"

I thought that this may have been caused by the changed I introduced to the source code, but after recloning the repo, the same error messages persist.

I could not find much specific help on this issue online and was wondering if you have run into this? I am running on Ubuntu 18.04 by the way. Thanks!

[sobolevn]

This warning means that you file has executable rights, but does not have a sebang #!/... in its source code. And since these files should be executable at all, I guess the problem is inside your machine.

Check file rights with ls -alh, they should not have +x for all categories.

You can also send me git status output, it might help.

[ragohin]

Ah yes, chmod did the trick to fix the file rights. Thanks!

[ragohin]

I'm a bit confused by the error "WPS214 Found too many methods: 9" error that is printed out when flake8 runs in wemake_python_styleguide\visitors\ast\loops.py. It seems like you've talked about this in a previous issue https://github.com/wemake-services/wemake-python-styleguide/pull/678.

So, do I have to reduce the number of methods that I have added to the WrongLoopVisitor class? I added two functions to this class, one for parsing a node to check if it violates the rule and a helper function. However, this error was still thrown when I did not have a helper function (the number of methods was 8).

[ragohin]

I created a new pull request just in case you need access to see the specific code I am referring to. Thanks!

[ragohin]

It seem like these functions in WrongLoopVisitor class are cluttering things up and need to be reused or moved.

However, I cannot figure out how to change/move them without breaking other things in the code.

[sobolevn]

Move them to logic/

[ragohin]

Great thanks!

I moved those functions to logic and everything seems to be working, but there is an issue right now with running pytest. The rule, as defined, raises the violation in any "while True:" loop that does not contain a break, return, or raise, but there are instances of these loops in some of the other test files like the one below (wemake-python-styleguide/tests/test_visitors/test_ast/test_loops/test_loops/test_lambda_in_loop.py).

It seems like there are two options:
1) add a break to these tests
2) change the way the infinite while loop rule is defined so that these instances don't fail

I'm leaning towards (1) because it seems like the while loops in these test cases would be incorrect because they don't necessarily exit. Does this seem like the best approach?

[ragohin]

Also, what exactly would an integration test to add to tests/fixtures/noqa.py look like? I'm a bit confused on what I should add here. Like I probably shouldn't just add "while True:" because it might hang forever.

[ragohin]

I think I figured it out. I removed those lines from test_lambda_in_loop.py because they are actual violations of the infinite loop rule. All of the requirements have been satisfied except for the adding of an integration test in noqa.py because I could not figure out how to include an infinite loop without pytest failing when running this file. I'll submit another pull request right now, but please let me know if it is possible to add an integration test for this. Thanks!

[pandaseal]

@ragohin may I ask how you fixed the EXE002 error exactly? I have just forked the project and get this error the first time i try to run make test.

I run Ubuntu 16.04 but we get the same error in Ubuntu 18.04.

[sobolevn]

@pandaseal this is related to your folder structure. Run ls -alh and then fix file permissions if you have any. That should solve it.

[pandaseal]

@sobolevn should I do it manually for every file that fails? I do not really know how to go about all of it... Any tips?

[sobolevn]

It really dependends on your problem and your OS. I don't think that I can help without any further information. You can also just ignore this issue for now. It works on CI correctly.

[pandaseal]

@sobolevn We fixed it. Thanks for your help.

If anyone else has this problem, this was the solution:

• System: Ubuntu 16.04 WSL on Windows 10
• Problem: Windows overwrote the permissions of the WSL when the repo was located in the Windows partition (/mnt), changing the permissions did not work. Here is why changing permissions wouldn't work.
• Solution: Clone the repo _directly onto the home_ (ie. do cd and then clone the repo) of the WSL.

[sobolevn]

@pandaseal you can submit this to CONTRIBUTING.md

[pandaseal]

@sobolevn should I create an issue where we can discuss this further? For example where the information should be added in CONTRIBUTING.md and on which level it should be described? (I mean, at this point is not really relevant for _this_ issue)