flake8 icon indicating copy to clipboard operation
flake8 copied to clipboard

per-file-ignores in config file should be treated relative to it

Open asottile opened this issue 4 years ago • 24 comments

In GitLab by @blueyed on Mar 7, 2019, 07:10

With tests/conftest.py:E402 in setup.cfg, flake8 run in tests will still complain about the error.

I think relative paths in config files should be treated relative to the config file's location.

Using flake8 3.7.7 (mccabe: 0.6.1, pycodestyle: 2.5.0, pyflakes: 2.1.1) CPython 3.7.2 on Linux.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Mar 7, 2019, 08:38

Odd, this does seem to be treated differently than exclude= though they appear to use the same code to check:

https://gitlab.com/pycqa/flake8/blob/88caf5ac484f5c09aedc02167c59c66ff0af0068/src/flake8/style_guide.py#L473-491

https://gitlab.com/pycqa/flake8/blob/88caf5ac484f5c09aedc02167c59c66ff0af0068/src/flake8/checker.py#L173-195

I set up the following in /tmp/x:

t/   # cwd is inside `t/`
    t.py
setup.cfg

The difference appears to be here where the "filename" (actually the per-file-ignore pattern) is being normalized:

https://gitlab.com/pycqa/flake8/blob/88caf5ac484f5c09aedc02167c59c66ff0af0068/src/flake8/style_guide.py#L450

Looking at a log, the value for exclude gets normalized as if it is run from the directory of setup.cfg

flake8.options.config     MainProcess     82 DEBUG    Option "exclude" returned value: 't/t.py'
flake8.options.config     MainProcess     82 DEBUG    't/t.py' has been normalized to ['/tmp/x/t/t.py'] for option "exclude"

Looking at the log for per-file-ignores

flake8.style_guide        MainProcess     98 DEBUG    <StyleGuide [/tmp/x/t/t/t.py]> does not match "/tmp/x/t/t.py"

The fix here is to normalize the paths relative to the config file they're defined in, though I'm not quite sure where to thread that through, will take more of a sit down

Thanks for the report :tada:

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @blueyed on Mar 18, 2019, 02:39

Any more findings by now? (closing old tabs)

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Mar 18, 2019, 07:53

nope, havnen't done anything with this since triaging -- if you want to take a stab at it by all means :)

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Mar 21, 2019, 08:24

mentioned in merge request !311

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @ericvw on May 20, 2019, 20:37

mentioned in merge request !321

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @ericvw on May 20, 2019, 20:43

I created !321, which is an initial minimal stab to get this working.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @pjacock on Aug 8, 2019, 03:47

I wonder if this is part of a larger issue with the path normalisation? i.e. new issue #562.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Aug 8, 2019, 08:18

#562 is unrelated, per-file-ignores is not parsed / normalized at all in the same way as normal options which is what needs fixing

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @ericvw on Aug 29, 2019, 10:11

mentioned in merge request !337

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @ericvw on Aug 31, 2019, 06:38

mentioned in merge request !351

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @gbd.lin on Oct 9, 2019, 05:44

What is the current status of this issue? I'm really looking forward to using some features that will be introduced in 3.8.0, especially --extend-exclude but that release depends on fixing this issue currently.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Oct 9, 2019, 06:00

in process, please don't bump issues like this, use the thumbs up

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @pjacock on Jan 17, 2020, 24:34

#562 has been closed as a duplicate, the same symptoms but wider in scope affecting any path configuration values and sources of config files. I included some examples using a tiny plugin adding a path value.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @felker on Feb 25, 2020, 09:07

We recently stumbled upon this in the Emacs flycheck project:

  • https://github.com/flycheck/flycheck/pull/1651
  • https://github.com/flycheck/flycheck/issues/1644

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Apr 27, 2020, 13:01

this is likely to miss 3.8.0, but we will try and address this soon (the work to fix this is large, complicated, and in progress)

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @lewoudar on May 31, 2020, 13:03

Hi everyone, I'm not sure but I think I have an issue related to this one. I have a file starting like this

from gevent import monkey

monkey.patch_all()  # noqa: E402
from .files import read_mp, write_mp

Under flake8 3.7.9 when I run flake8 <source> I had no issue

But under flake8 3.8.2, No I have ...\__init__.py:4:1: E402 module level import not at top of file

My environment

  • Windows 10
  • python 3.8

How I install flake8: poetry add flake8 which in fact run pip install <dependency> --no-deps for every flake8 dependency before installing the package itself.

That sounds like a regression :/

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on May 31, 2020, 13:06

hello @lewoudar,

that's unrelated to this issue, you can read more about it in #638

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @lewoudar on May 31, 2020, 13:16

Aww thanks for the reference, I will check that

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @tmewett on Nov 3, 2020, 08:59

Is it necessary to involve the location of the config file? With the full current working directory and the relative path of what is being scanned by flake8, is it possible to construct a full valid path for any file being scanned. This can then be matched against the configured path patterns.

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @asottile on Nov 3, 2020, 09:09

yes, the expectation of flake8 users is that paths in config are relative to it -- this is true for all options currently except per-file-ignores

the config is not always in the current working directory

asottile avatar Apr 03 '21 06:04 asottile

In GitLab by @tmewett on Nov 3, 2020, 09:52

That is certainly the expectation of current flake8 users, but just fyi it contrasts with other tools which have file pattern config files. Anyway, I raised a separate issue #693 for this as it's not completely related.

asottile avatar Apr 03 '21 06:04 asottile

Spent a good hour stuck on this today. Got confused because my config file only has file name options in as it stands and the mix between relative and absolute paths threw me off

per-file-ignores =
    tests/*:E501
filename =
    ./src/*.py,
    ./tests/*.py```

kaybee99 avatar Jun 01 '22 17:06 kaybee99

Any news on that? Are you planning to apply this change?

I am having issues with Neovim editor and nvim-lint plugin, because the plugin runs flake8 in the path where you are working not from the project's root (where I have the setup.cfg with the per-file-ignores config)

Guillem96 avatar Jul 12 '22 06:07 Guillem96

@Guillem96 OSS advice: github provides a thumbs up so you can participate without sending an email to everyone subscribed. it's also easy to check the status of an issue: is it open? yes. if you have doubts on the freshness of the issue it's very easy to just try it

asottile avatar Jul 12 '22 12:07 asottile