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

Too broad suppression of unused-argument warnings #249

Closed
psrb opened this issue Oct 27, 2019 · 1 comment
Closed

Too broad suppression of unused-argument warnings #249

psrb opened this issue Oct 27, 2019 · 1 comment

Comments

@psrb
Copy link
Contributor

psrb commented Oct 27, 2019 •
edited
Loading

pylint-django currently suppresses the unused-argument warning when a function/method contains an argument called request. The reason to ignore these warnings for an argument named request was discussed in #155 and implemented with commit e4c8fc7.

Unfortunately the warning is also suppressed for all other function arguments. Therefore every other unused argument of the function will not be reported. Unused arguments can be an indicator for bugs and these warnings should only be explicitly silenced.

Example

def user_detail(request, user_id):
    # nothing is done with user_id
    return JsonResponse({'username': 'steve'})

Pylint should issue a warning that user_id is unused. But because the function has an argument named request no warning is issued.
psrb added a commit to psrb/pylint-django that referenced this issue Oct 27, 2019
@psrb
Ignore unused-argument warning for request arguments (Fixes pylint-de…
29d3f48 
…v#249)

The previous implementation (result of pylint-dev#155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit extends the `ignored-argument-names` regex of Pylint to also
ignore arguments named `request`. This way unused-argument warnings will
still be issued for every other unused argument of the function/method.
@psrb psrb mentioned this issue Oct 27, 2019
Merged
@fechnert
Copy link

We've encountered the same issue recently. Pylint was not able to detect that further arguments were unused.

It would be awesome to see the linked PR merged 👍

psrb added a commit to psrb/pylint-django that referenced this issue Oct 30, 2019
@psrb
Ignore unused-argument warning for request arguments (Fixes pylint-de…
2281d1f 
…v#249)

The previous implementation (result of pylint-dev#155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit extends the `ignored-argument-names` regex of Pylint to also
ignore arguments named `request`. This way unused-argument warnings will
still be issued for every other unused argument of the function/method.
psrb added a commit to psrb/pylint-django that referenced this issue Oct 30, 2019
@psrb
Ignore unused-argument warning for request arguments (Fixes pylint-de…
c817213 
…v#249)

The previous implementation (result of pylint-dev#155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit extends the `ignored-argument-names` regex of Pylint to also
ignore arguments named `request`. This way unused-argument warnings will
still be issued for every other unused argument of the function/method.
atodorov pushed a commit that referenced this issue Oct 31, 2019
@psrb @atodorov
Ignore unused-argument warning for request arguments (Fixes #249)
b54f3c5 
The previous implementation (result of #155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit extends the `ignored-argument-names` regex of Pylint to also
ignore arguments named `request`. This way unused-argument warnings will
still be issued for every other unused argument of the function/method.
psrb added a commit to psrb/pylint-django that referenced this issue Nov 2, 2019
@psrb
Monkey patch VariablesChecker._is_name_ignored (Fixes pylint-dev#249)
5e8ae71
psrb added a commit to psrb/pylint-django that referenced this issue Nov 3, 2019
@psrb
Ignore unused-argument warning for request arguments (Fixes pylint-de…
a7cb1e1 
…v#249)

The previous implementation (result of pylint-dev#155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit monkey patches `VariablesChecker._is_ignored_named` to
specifically suppress the generation of unused-argument warnings for
arguments named `request`.
This way unused-argument warnings will still be issued for every other
unused argument of the function/method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@psrb @fechnert