MPP-79: Add Python code linter #162
Comments
|
Also randomly tried bandit, which didn't give a lot of feedback: bandit -r *.py emails phones privaterelay
[main] INFO profile include tests: None
[main] INFO profile exclude tests: None
[main] INFO cli include tests: None
[main] INFO cli exclude tests: None
[main] INFO running on Python 3.7.4
[node_visitor] INFO Unable to find qualified name for module: manage.py
Run started:2020-04-14 16:40:05.061205
Test results:
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
Severity: Low Confidence: High
Location: emails/tests/model_tests.py:18
More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
17 relay_address = RelayAddress.make_relay_address(self.user)
18 assert relay_address.user == self.user
19
--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
Severity: Low Confidence: High
Location: emails/tests/model_tests.py:25
More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
24 relay_addresses.append(RelayAddress.make_relay_address(self.user))
25 assert len(relay_addresses) == len(set(relay_addresses))
26
--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
Severity: Low Confidence: High
Location: emails/tests/model_tests.py:32
More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
31 deleted_count = DeletedAddress.objects.filter(address_hash=address_hash).count()
32 assert deleted_count == 1
33
--------------------------------------------------
Code scanned:
Total lines of code: 1135
Total lines skipped (#nosec): 0
Run metrics:
Total issues (by severity):
Undefined: 0.0
Low: 3.0
Medium: 0.0
High: 0.0
Total issues (by confidence):
Undefined: 0.0
Low: 0.0
Medium: 0.0
High: 3.0
Files skipped (0): |
|
@jrbenny35 offered the following advice in Slack:
|
|
Flake8 output: # Ignore "E501 line too long" errors.
flake8 *.py emails phones privaterelay --extend-ignore=E501
emails/apps.py:8:9: F401 'emails.signals' imported but unused
emails/views.py:9:1: F401 'decouple.config' imported but unused
emails/views.py:18:1: F401 'django.shortcuts.render' imported but unused
emails/views.py:18:1: F401 'django.shortcuts.get_object_or_404' imported but unused
emails/views.py:22:1: F401 '.models.DeletedAddress' imported but unused
emails/migrations/0004_auto_20190612_2047.py:2:1: F401 'uuid' imported but unused
emails/tests/model_tests.py:38:9: F841 local variable 'deleted_address' is assigned to but never used
phones/tests.py:1:1: F401 'django.test.TestCase' imported but unused
phones/views.py:11:1: F401 'django.shortcuts.render' imported but unused
phones/views.py:58:5: F841 local variable 'db_session' is assigned to but never used
phones/views.py:68:5: F841 local variable 'pretty_from' is assigned to but never used
phones/views.py:117:5: F841 local variable 'new_participant' is assigned to but never used
phones/views.py:125:5: F841 local variable 'message' is assigned to but never used
privaterelay/settings.py:150:69: E231 missing whitespace after ','
privaterelay/settings.py:188:78: E231 missing whitespace after ','
privaterelay/settings.py:189:79: E231 missing whitespace after ','
privaterelay/settings.py:190:80: E231 missing whitespace after ','
privaterelay/settings.py:255:70: E231 missing whitespace after ','
privaterelay/settings.py:256:60: E231 missing whitespace after ','
privaterelay/urls.py:16:1: F401 'decouple.config' imported but unused
privaterelay/views.py:38:5: F841 local variable 'c' is assigned to but never used |
|
Sorry, I'm basically just using this issue as a notepad for my learnings at this point. Feel free to unsubscribe to my TED talk here... @hackebrot also suggested the following:
|
|
The first tests are using |
|
Also, digging in a bit more, I think https://github.com/pyupio/safety would be nice to add as well. It seems to be the Python verison of npm audit. Looks like we might need to update django. https://github.com/vintasoftware/python-linters-and-code-analysis |
|
I'm still playing w/ flake8-black, but not sure how valuable it is as a flake8 plugin vs standalone tool since flake8 only seems to give me vague output like:
Here was my local .flake8 config file (but I honestly don't know enough about Python to know if thats the best approach, or if you want it some awkward tox file or add a precommit hook, etc): [flake8]
exclude = */migrations/*,venv,.git
extend-ignore = E501
max-complexity = 10 |
|
I'm working on black + flake8 + mypy:
I think we can do this rapidly at the end of this sprint / beginning of next, so I've also moved MPP-79 to the current sprint. |
|
1 of 5 linters integrated:
I think I'll also want a (optional) pre-commit hook, so devs can see linting exceptions before committing and submitting PRs. The |
|
Many Python projects are switching to |
Ref: #160
OK, I had a bit more time last night to randomly look at Python libraries, and maybe we want some combination of Black (#160) and Pylint (which I'm assuming is roughly similar to Prettier+ESLint respectively).
I managed to pip install pylint in a virtualenv and then run this command:
Where I had to do some fiddling with the .pylintrc config file to exclude a bunch of messages I wasn't ready to hear from yet (about 98% of that list was automatically generated by pylint, I only added a few gems at the top of the list):
The text was updated successfully, but these errors were encountered: