Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bogus N805 with positional-only parameters (PEP 570) #122

Closed
scottj97 opened this issue Nov 1, 2019 · 3 comments · Fixed by #123
Closed

Bogus N805 with positional-only parameters (PEP 570) #122

scottj97 opened this issue Nov 1, 2019 · 3 comments · Fixed by #123
Assignees
@jparise
Labels
bug

Comments

@scottj97
Copy link

scottj97 commented Nov 1, 2019

Running Python 3.8.0:

$ python3 -m venv venv
$ venv/bin/pip3 install flake8 pep8_naming

Please provide the exact, unmodified output of flake8 --bug-report

{
  "dependencies": [
    {
      "dependency": "entrypoints",
      "version": "0.3"
    }
  ],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.8.0",
    "system": "Linux"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "naming",
      "version": "0.8.2"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.5.0"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "2.1.1"
    }
  ],
  "version": "3.7.9"
}

Please describe the problem or feature

Trying to use Python 3.8's new positional-only parameters (PEP 570), I'm seeing a bogus warning from pep8_naming. It seems the AST has added a new args.posonlyargs which pep8-naming's get_arg_name_tuples() is not aware of.

If this is a bug report, please explain with examples (and example code) what you expected to happen and what actually happened.

The complete run.py:

#!python3


class Foobar:
    def runme(self, qrs, /, xyz=None):
        pass

When I run flake8 run.py I get the following:

run.py:5:27: E225 missing whitespace around operator
run.py:5:30: N805 first argument of a method should be named 'self'

The E225 is a known issue in pycodestyle. The N805 should not be there. The argument is named 'self', so the check should pass, but pep8_naming sees the argument named 'xyz' instead.
@jparise jparise added the bug label Nov 3, 2019
@jparise jparise self-assigned this Nov 3, 2019
jparise added a commit to jparise/pep8-naming that referenced this issue Nov 3, 2019
@jparise
Recognize Python 3.8's args.posonlyargs group
19cc445 
`args.posonlyargs` was added to Python 3.8's AST to represent
position-only arguments. These comes first in the total ordering of a
function's argument groups, so we need to consider them inside of
get_arg_name_tuples().

This is particularly relevant to N805, which checks the name of the
first argument for methods.

Fixes PyCQA#122
@jparise jparise mentioned this issue Nov 3, 2019
Merged
@5j9 5j9 closed this as completed in #123 Nov 4, 2019
5j9 pushed a commit that referenced this issue Nov 4, 2019
@jparise @5j9
Recognize Python 3.8's args.posonlyargs group
b978465 
`args.posonlyargs` was added to Python 3.8's AST to represent
position-only arguments. These comes first in the total ordering of a
function's argument groups, so we need to consider them inside of
get_arg_name_tuples().

This is particularly relevant to N805, which checks the name of the
first argument for methods.

Fixes #122
@jparise
Copy link
Member

jparise commented Nov 4, 2019

Thanks for the thorough bug report, @scottj97! A fix for this is now in master (b978465). Would you mind verifying that it fixes what you're seeing?

@scottj97
Copy link
Author

scottj97 commented Nov 4, 2019

Thanks for the thorough bug report, @scottj97! A fix for this is now in master (b978465). Would you mind verifying that it fixes what you're seeing?

Confirmed, bogus N805 warning disappears.

@scottj97 scottj97 mentioned this issue Nov 4, 2019
Closed
@jparise
Copy link
Member

jparise commented Nov 5, 2019

Confirmed, bogus N805 warning disappears.

Wonderful! I'll plan on putting together a new package release this week that will include this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jparise jparise

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Recognize Python 3.8's args.posonlyargs group jparise/pep8-naming
2 participants
@jparise @scottj97