search

smarkets / flake8-strict

Flake8 plugin that checks Python code against a set of opinionated style rules
MIT License
10 stars 8 forks source link

Fix elements inside class not being linted correctly. #37

Closed JaimeLennox closed 6 years ago

JaimeLennox commented 6 years ago

This was due to a change that modifies the children of a classdef node in order to lint arguments to a class. The node is now copied instead to avoid modifying its children.

This could also potentially affect decorators and imports, which also have their children modified, so these have been copied in the same way. However, as decorator nodes are separate from funcdef nodes, and children of importfrom nodes shouldn't have to be linted in this way, it is unlikely they would be an issue.

Fixes: #36

mxr commented 6 years ago

Thanks for fixing! I tried this branch out on a larger repo and it causes flake8 to run much longer, up to 10x times as long on some large files. Is it possible to use a shallow copy, to deep copy only part of the objects, or use another technique instead?

mxr commented 6 years ago

I pulled the branch and did an %s/deepcopy/copy/g, and all the unit tests still passed. The runtime was much more reasonable with that change, too

mxr commented 6 years ago

@JaimeLennox @pgjones here's my change to use copy instead of deepcopy - would it be possible to integrate that into this PR? https://github.com/smarkets/flake8-strict/compare/classdef-fix...mxr:patch-1?expand=1

JaimeLennox commented 6 years ago

@mxr Sorry for the delay. You're right, I'll add your change to this PR. Thanks!

mxr commented 6 years ago

Thank you!

pgjones commented 6 years ago

This has sadly broken master (although I had thought it passed as a pull request).

JaimeLennox commented 6 years ago

It seems to have just had trouble cloning the repository. I've retried and it passes now.