pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.3k stars 1.14k forks source link

Warn if module dunders are below imports #5471

Open harshil21 opened 2 years ago

harshil21 commented 2 years ago

Current problem

The PEP-8 guidelines state that module level dunders are supposed to be above imports. This is something pylint does not warn about if the code does not follow it.

Desired solution

Pylint should raise a warning if the above is not followed.

Additional context

Similar to #965, but also add a warning for it

clavedeluna commented 2 years ago

I can probably work on this. Throwing some ideas out there. I was thinking this could be added to the wrong-import-order check, For example, here are some test cases:

This is correct according to PEP8

from __future__ import barry_as_FLUFL

__all__ = ["barry_as_FLUFL"]
__version__ = '0.1'
__author__ = 'Cardinal Biggles'

import os
import sys

This would be incorrect:


from __future__ import barry_as_FLUFL
import os   #[wrong-import-order]
import sys #[wrong-import-order]

__all__ = ["barry_as_FLUFL"]
__version__ = '0.1'
__author__ = 'Cardinal Biggles'

Also this is incorrect


import os   #[wrong-import-order]
import sys #[wrong-import-order]
from __future__ import barry_as_FLUFL #[wrong-import-order]

__all__ = ["barry_as_FLUFL"]
__version__ = '0.1'
__author__ = 'Cardinal Biggles'

But I could see the argument for adding a new check that would raise on the module level dunder names instead of on the imports. Thoughts?

Pierre-Sassoulas commented 2 years ago

Disclaimer : I'm probably not going to use this check unless isort fix it automatically.

But I think it would make sense to raise both message:

import os   #[wrong-import-order]
import sys #[wrong-import-order]
from __future__ import barry_as_FLUFL #[wrong-import-order]

__all__ = ["barry_as_FLUFL"]   # [wrong-dunder-order]
__version__ = '0.1'   # [wrong-dunder-order]
__author__ = 'Cardinal Biggles'   # [wrong-dunder-order]

isort is doing the wrong import order check right now. Maybe the thing to do is to propose or implement this in isort first ?

clavedeluna commented 2 years ago

Yeah, I can see isort moving the imports but it won't move the dunders around. Proposing in isort makes sense. But I don't see a time dependency on that, pylint can implement that and so can isort, independently. Unless there's some PyCQA process I don't know about?

clavedeluna commented 2 years ago

Opened https://github.com/PyCQA/isort/issues/1980

Pierre-Sassoulas commented 2 years ago

I don't see a time dependency on that, pylint can implement that and so can isort, independently. Unless there's some PyCQA process I don't know about?

No, it's just that if we raise a message actual people are going to spend time fixing it manually. If we know that this could be fixed by isort if we wait a little then we're going to save days to months of time modifying code for something that ultimately does not matter that much, and it's bad etiquette. (So cats will be petted longer in our name; which is nice)

clavedeluna commented 1 year ago

I think it's reasonable to close this as Won't Do and hope it's done in isort https://github.com/PyCQA/isort/issues/1980

DanielNoord commented 1 year ago

I'm voting to reopen as long as the maintainers of isort have not responded to your issue. We aren't even sure that they accept the proposal.

Pierre-Sassoulas commented 1 year ago

Blocked by a decision on https://github.com/PyCQA/isort/issues/1980.

Pierre-Sassoulas commented 1 year ago

The message could be named misplaced-module-dunder. Since the last message ruff appeared and would also autofix this now. This needs some investigation to check if ruff does it and / or creating the equivalent of https://github.com/PyCQA/isort/issues/1980 in ruff's repository.