Skip to content
New issue

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

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

Already on GitHub? # to your account

Invalid pointless-string-statement for docstrings for Python 3.12 type-aliases (PEP 695) #9268

Closed
Stausssi opened this issue Nov 28, 2023 · 12 comments · Fixed by #9269
Closed
Assignees
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code python 3.12
Milestone

Comments

@Stausssi
Copy link

Stausssi commented Nov 28, 2023

Bug description

Adding a multiline docstring to a Python 3.12 type-alias (PEP 695) causing pylint to faild with a pointless-string-statement.

Example code:

type _GeneratorCallable = Callable[..., list[str]]
"""
Multiline
docstring
for a new type-alias
introduced in Python3.12
"""

Adding a : before the string also doesn't help:

type _GeneratorCallable = Callable[..., list[str]]
""":
Multiline
docstring with :
for a new type-alias
introduced in Python3.12
"""

Configuration

No response

Command used

pylint src

Pylint output

************* Module src.generator.generator
src/generator/generator.py:8:0: W0105: String statement has no effect (pointless-string-statement)

Expected behavior

Pylint should not raise a warning here

Pylint version

pylint 3.0.2
astroid 3.0.1
Python 3.12.0 (main, Nov  2 2023, 13:03:57) [Clang 15.0.0 (clang-1500.0.40.1)]

OS / Environment

macOS Sonoma, iTerm2

Additional dependencies

No response

@Stausssi Stausssi added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 28, 2023
@jacobtylerwalls jacobtylerwalls self-assigned this Nov 28, 2023
@jacobtylerwalls jacobtylerwalls added False Positive 🦟 A message is emitted but nothing is wrong with the code python 3.12 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 28, 2023
@jacobtylerwalls jacobtylerwalls added this to the 3.0.3 milestone Nov 28, 2023
@DanielNoord
Copy link
Collaborator

I can't actually check as I don't have an working interpreter, but are theses set to __doc__? The PEP doesn't mention it.
We exclude Assign and AnnAssign because of this rejected PEP, but for this case I don't see that?

@jacobtylerwalls
Copy link
Member

Ah. Here's this, if helpful:

>>> n = ast.parse("""
... type MyTuple = tuple[str, str]
... "my docstring"
... """)
>>> n.body[0].__doc__
'TypeAlias(expr name, type_param* type_params, expr value)'

We exclude Assign and AnnAssign because of this rejected PEP, but for this case I don't see that?

Interesting, I would have thought we just were being friendly to (someone's) common practice (somewhere), not observing a rejected PEP.

@DanielNoord
Copy link
Collaborator

I think the original rationale was that PEP.

I think we might actually not want to do this? As this makes it clearer that the statement is indeed pointless and does not do what you think it does?

@jacobtylerwalls
Copy link
Member

Yeah, I find that convincing.

@jacobtylerwalls jacobtylerwalls added New parser Requires a new AST parser (upstream) Needs decision 🔒 Needs a decision before implemention or rejection and removed New parser Requires a new AST parser (upstream) labels Nov 28, 2023
@Stausssi
Copy link
Author

Good to know that this type of docstring is actually not the way to go. Been using it for a while and all my tools picked it up fine so far.

@jacobtylerwalls
Copy link
Member

I'm interested to know which tools support it. VS Code doesn't seem to provide the docstring on hover the way it does for attributes. Are there other tools that do?

@Stausssi
Copy link
Author

I figured VS Code not showing any kind of hover information is just an "early adopter" problem. Was referring to that type of docstring for attributes in my earlier comment.
In fact, I can't seem to get any type of docstring format to work for the new type alias. Seems like some sort of oversight in the PEP?

@jacobtylerwalls
Copy link
Member

Maybe worth asking on discuss.python.org?

@Stausssi
Copy link
Author

Link to discussion

Posting this here to keep in sync

@Stausssi
Copy link
Author

Stausssi commented Dec 6, 2023

Feedback from python.org is that the new type aliases are to be treated the same as regular assignments regarding docstrings. In this case I suppose it means also excluding it like Assign and AnnAssign.

Support in pyright / pylance was added for this type of docstring: microsoft/pyright#6647

@jacobtylerwalls
Copy link
Member

Thanks, that's really helpful!

@jacobtylerwalls jacobtylerwalls removed the Needs decision 🔒 Needs a decision before implemention or rejection label Dec 6, 2023
@Stausssi
Copy link
Author

Stausssi commented Dec 8, 2023

Thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code python 3.12
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants