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

issue9255 - Detect FIXME words in docstring #9281

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/9255.new_check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added a ``fixme-in-docstring`` message to check TODO, FIXME inside doc strings

Closes #9255
18 changes: 17 additions & 1 deletion pylint/checkers/base/docstring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ class DocStringChecker(_BasicChecker):
"docstring.",
{"old_names": [("C0111", "missing-docstring")]},
),
"C0150": (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect this message to be in the same checker than fixme. Maybe even the same message.

"%s",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this message a little more descriptive? Something like:

Suggested change
"%s",
"Disallowed word in docstring: '%s'",

I know the original fixme message doesn't have this but I don't think that's nice πŸ˜„

"fixme-in-docstring",
"Used when a warning note as FIXME or XXX is detected in docstring.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo should be a rational (why it's bad) and not when it's raised. (I know the current descriptions are a bad example of that.)

),
}
options = (
(
Expand Down Expand Up @@ -156,7 +161,7 @@ def _check_docstring(
report_missing: bool = True,
confidence: interfaces.Confidence = interfaces.HIGH,
) -> None:
"""Check if the node has a non-empty docstring."""
"""Check if the node has a non-empty docstring and check for fixme words."""
docstring = node.doc_node.value if node.doc_node else None
if docstring is None:
docstring = _infer_dunder_doc_attribute(node)
Expand Down Expand Up @@ -206,3 +211,14 @@ def _check_docstring(
self.add_message(
"empty-docstring", node=node, args=(node_type,), confidence=confidence
)
else:
fixme_words = self.linter.config.notes
for word in fixme_words:
Comment on lines +215 to +216
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fixme_words = self.linter.config.notes
for word in fixme_words:
for word in self.linter.config.notes:

if word in docstring:
self.add_message(
"fixme-in-docstring",
node=node,
args=(word,),
confidence=confidence,
)
break
1 change: 1 addition & 0 deletions pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ disable=
format,
# We anticipate #3512 where it will become optional
fixme,
fixme-in-docstring,
consider-using-assignment-expr,


Expand Down
27 changes: 27 additions & 0 deletions tests/functional/d/docstrings_fixme.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# pylint: disable=unnecessary-pass, consider-using-f-string

"""
Test fixme in docstrings.
"""
Comment on lines +1 to +5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you switch the comment and the docstring around?


def func1(): # [fixme-in-docstring]
"""
TODO: Implement
"""

# fixme-in-docstring
def func2(): # [fixme-in-docstring]
"""
FIXME: Implement
"""

# fixme-in-docstring
def func3(): # [fixme-in-docstring]
"""
XXX: Implement
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test where the FIXME is not the first word of the line? And also one where it is not surrounded by spaces?

"""

def func4():
"""
This is regular docstring
"""
3 changes: 3 additions & 0 deletions tests/functional/d/docstrings_fixme.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fixme-in-docstring:7:0:7:9:func1:TODO:HIGH
fixme-in-docstring:13:0:13:9:func2:FIXME:HIGH
fixme-in-docstring:19:0:19:9:func3:XXX:HIGH