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.

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

False positive missing-param-doc when escaping underscore in docstring #7827

Closed
kasium opened this issue Nov 23, 2022 · 5 comments · Fixed by #7878
Closed

False positive missing-param-doc when escaping underscore in docstring #7827

kasium opened this issue Nov 23, 2022 · 5 comments · Fixed by #7878
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@kasium
Copy link
Contributor

kasium commented Nov 23, 2022

Bug description

In this code I need to escape the underscore, because else it's not valid RST.

def get_value(something: int, raise_: bool = False) -> bool:
    """Return the maximum length of a column.

    Args:
        something: the something
        raise\\_: the other

    Returns:
        something
    """
    return something and raise_

Configuration

[MAIN]
load-plugins = ["pylint.extensions.docparams"]

Command used

pylint foo.py

Pylint output

************* Module foo
foo.py:4:0: W9015: "raise_" missing in parameter documentation (missing-param-doc)

Expected behavior

Escaped underscore is understood

Pylint version

pylint 2.15.6
astroid 2.12.12
Python 3.7.2 (default, Jun  2 2022, 07:23:31)
@kasium kasium added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 23, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 23, 2022
@Pierre-Sassoulas Pierre-Sassoulas changed the title W9015 when escaping underscore False positive missing-param-doc when escaping underscore in docstring Nov 23, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Help wanted 🙏 Outside help would be appreciated, good for new contributors Good first issue Friendly and approachable by new contributors labels Nov 23, 2022
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Nov 23, 2022

Thank you for opening the issue. It seems like we need to "normalize" the variable name recovered from the docstring. It should be doable without too much pylint knowledge.

@clavedeluna
Copy link
Contributor

It seems like to fix this we would either have to:

  1. Update this regex https://github.com/PyCQA/pylint/blob/main/pylint/extensions/_check_docs_utils.py#L472 which seems daunting honestly!
  2. Do something a bit hacky here before the regex is used, such as entry.replace("\\", "")

@Pierre-Sassoulas thoughts on what would be accepted?

@Pierre-Sassoulas
Copy link
Member

I was thinking of "normalizing" the string with a hack initially. If it's detected via regex and it requires a regex change then it might (😄) not be such a good first issue after all...

@clavedeluna
Copy link
Contributor

I'll PR the hacky normalization and see what a variety of maintainers have to say. If it's rejected then we can close it and remove the "good first issue" label 😄

@kasium
Copy link
Contributor Author

kasium commented Dec 4, 2022

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants