From ccd41e09d5f01d1e89283d69702d2c91a9f8ff0f Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sat, 22 Jun 2024 03:40:24 -0400 Subject: [PATCH 01/15] Support fixme's in docstrings --- pylint/checkers/misc.py | 56 ++++++++--- tests/checkers/unittest_misc.py | 125 +++++++++++++++++++++++++ tests/functional/f/fixme.py | 4 +- tests/functional/f/fixme_docstring.py | 40 ++++++++ tests/functional/f/fixme_docstring.rc | 7 ++ tests/functional/f/fixme_docstring.txt | 10 ++ 6 files changed, 227 insertions(+), 15 deletions(-) create mode 100644 tests/functional/f/fixme_docstring.py create mode 100644 tests/functional/f/fixme_docstring.rc create mode 100644 tests/functional/f/fixme_docstring.txt diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index 78c21d0c5e..aadc5583ea 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -90,6 +90,15 @@ class EncodingChecker(BaseTokenChecker, BaseRawFileChecker): "default": "", }, ), + ( + "check-fixme-in-docstring", + { + "type": "yn", + "metavar": "", + "default": False, + "help": "Whether or not to search for fixme's in docstrings." + } + ) ) def open(self) -> None: @@ -97,11 +106,17 @@ def open(self) -> None: notes = "|".join(re.escape(note) for note in self.linter.config.notes) if self.linter.config.notes_rgx: - regex_string = rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + comment_regex = rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + self._comment_fixme_pattern = re.compile(comment_regex, re.I) + if self.linter.config.check_fixme_in_docstring: + docstring_regex = rf"\"\"\"\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) else: - regex_string = rf"#\s*({notes})(?=(:|\s|\Z))" - - self._fixme_pattern = re.compile(regex_string, re.I) + comment_regex = rf"#\s*({notes})(?=(:|\s|\Z))" + self._comment_fixme_pattern = re.compile(comment_regex, re.I) + if self.linter.config.check_fixme_in_docstring: + docstring_regex = rf"\"\"\"\s*({notes})(?=(:|\s|\Z))" + self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) def _check_encoding( self, lineno: int, line: bytes, file_encoding: str @@ -133,16 +148,29 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: if not self.linter.config.notes: return for token_info in tokens: - if token_info.type != tokenize.COMMENT: - continue - comment_text = token_info.string[1:].lstrip() # trim '#' and white-spaces - if self._fixme_pattern.search("#" + comment_text.lower()): - self.add_message( - "fixme", - col_offset=token_info.start[1] + 1, - args=comment_text, - line=token_info.start[0], - ) + if token_info.type == tokenize.COMMENT: + comment_text = token_info.string[1:].lstrip() # trim '#' and white-spaces + if self._comment_fixme_pattern.search("#" + comment_text.lower()): + self.add_message( + "fixme", + col_offset=token_info.start[1] + 1, + args=comment_text, + line=token_info.start[0], + ) + elif self.linter.config.check_fixme_in_docstring and self._is_docstring_comment(token_info): + docstring_lines = token_info.string.split("\n") + for line_no, line in enumerate(docstring_lines): + comment_text = line.removeprefix('"""').lstrip().removesuffix('"""') # trim '""""' and whitespace + if self._docstring_fixme_pattern.search('"""' + comment_text.lower()): + self.add_message( + "fixme", + col_offset=token_info.start[1] + 1, + args=comment_text, + line=token_info.start[0] + line_no, + ) + + def _is_docstring_comment(self, token_info: tokenize.TokenInfo) -> bool: + return token_info.type == tokenize.STRING and token_info.line.lstrip().startswith('"""') def register(linter: PyLinter) -> None: diff --git a/tests/checkers/unittest_misc.py b/tests/checkers/unittest_misc.py index 2f292b81f1..0918a2c182 100644 --- a/tests/checkers/unittest_misc.py +++ b/tests/checkers/unittest_misc.py @@ -116,3 +116,128 @@ def test_dont_trigger_on_todoist(self) -> None: """ with self.assertNoMessages(): self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_message(self) -> None: + code = """ + \"\"\"FIXME message\"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) + + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_nl_message(self) -> None: + code = """ + \"\"\" + FIXME message + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="FIXME message", col_offset=9) + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_nl_message_multi(self) -> None: + code = """ + \"\"\" + FIXME this + TODO: that + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="FIXME this", col_offset=9), + MessageTest(msg_id="fixme", line=4, args="TODO: that", col_offset=9) + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_comment(self) -> None: + code = """ + # XXX message1 + \"\"\" + FIXME message2 + TODO message3 + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=2, args="XXX message1", col_offset=9), + MessageTest(msg_id="fixme", line=4, args="FIXME message2", col_offset=9), + MessageTest(msg_id="fixme", line=5, args="TODO message3", col_offset=9) + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_comment_prefix(self) -> None: + code = """ + # \"\"\" XXX should not trigger + \"\"\" # XXX should not trigger \"\"\" + """ + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_todo_middle_nl(self) -> None: + code = """ + \"\"\" + something FIXME message + \"\"\" + """ + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_todo_middle(self) -> None: + code = """ + \"\"\"something FIXME message + \"\"\" + """ + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config(check_fixme_in_docstring=True) + def test_docstring_todo_mult(self) -> None: + code = """ + \"\"\" + FIXME this TODO that + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="FIXME this TODO that", col_offset=9), + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config( + check_fixme_in_docstring=True, + notes=["CODETAG"] + ) + def test_docstring_custom_note(self) -> None: + code = """ + \"\"\" + CODETAG implement this + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="CODETAG implement this", col_offset=9), + ): + self.checker.process_tokens(_tokenize_str(code)) + + @set_config( + check_fixme_in_docstring=True, + notes_rgx="FIX.*" + ) + def test_docstring_custom_rgx(self) -> None: + code = """ + \"\"\" + FIXME implement this + FIXTHIS also implement this + \"\"\" + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="FIXME implement this", col_offset=9), + MessageTest(msg_id="fixme", line=4, args="FIXTHIS also implement this", col_offset=9), + ): + self.checker.process_tokens(_tokenize_str(code)) diff --git a/tests/functional/f/fixme.py b/tests/functional/f/fixme.py index e3d420f8ef..cdb3e304af 100644 --- a/tests/functional/f/fixme.py +++ b/tests/functional/f/fixme.py @@ -1,5 +1,5 @@ """Tests for fixme and its disabling and enabling.""" -# pylint: disable=missing-function-docstring, unused-variable +# pylint: disable=missing-function-docstring, unused-variable, pointless-string-statement # +1: [fixme] # FIXME: beep @@ -35,6 +35,8 @@ def function(): # pylint: disable-next=fixme # FIXME: Don't raise when the message is disabled +"""TODO: Don't raise when docstring fixmes are disabled""" + # This line needs to be at the end of the file to make sure it doesn't end with a comment # Pragma's compare against the 'lineno' attribute of the respective nodes which # would stop too soon otherwise. diff --git a/tests/functional/f/fixme_docstring.py b/tests/functional/f/fixme_docstring.py new file mode 100644 index 0000000000..8a5b62af8f --- /dev/null +++ b/tests/functional/f/fixme_docstring.py @@ -0,0 +1,40 @@ +"""Tests for fixme in docstrings""" +# pylint: disable=missing-function-docstring, pointless-string-statement + +# +1: [fixme] +"""TODO resolve this""" + +""" +FIXME don't forget this # [fixme] +XXX also remember this # [fixme] +??? but not this + + TODO yes this # [fixme] + FIXME: and this line, but treat it as one FIXME TODO # [fixme] +# TODO not this, however, even though it looks like a comment +also not if there's stuff in front TODO + XXX spaces are okay though # [fixme] +""" + +# +1: [fixme] +# FIXME should still work + +# +1: [fixme] +# TODO """ should work + +# """ TODO should not work +"""# TODO neither should this""" + +"""TODOist API should not result in a message""" + +# +2: [fixme] +""" +TO make something DO: look a regex +""" + +# pylint: disable-next=fixme +"""TODO won't work anymore""" + +# +2: [fixme] +def function(): + """./TODO implement this""" diff --git a/tests/functional/f/fixme_docstring.rc b/tests/functional/f/fixme_docstring.rc new file mode 100644 index 0000000000..de8a8941f9 --- /dev/null +++ b/tests/functional/f/fixme_docstring.rc @@ -0,0 +1,7 @@ +[MISCELLANEOUS] +# List of note tags to take in consideration, separated by a comma. +notes=XXX,TODO,./TODO +# Regular expression of note tags to take in consideration. +notes-rgx=FIXME(?!.*ISSUE-\d+)|TO.*DO +# enable checking for fixme's in docstrings +check-fixme-in-docstring=yes diff --git a/tests/functional/f/fixme_docstring.txt b/tests/functional/f/fixme_docstring.txt new file mode 100644 index 0000000000..b0ff92e86f --- /dev/null +++ b/tests/functional/f/fixme_docstring.txt @@ -0,0 +1,10 @@ +fixme:5:1:None:None::TODO resolve this:UNDEFINED +fixme:8:1:None:None::FIXME don't forget this # [fixme]:UNDEFINED +fixme:9:1:None:None::XXX also remember this # [fixme]:UNDEFINED +fixme:12:1:None:None::TODO yes this # [fixme]:UNDEFINED +fixme:13:1:None:None::"FIXME: and this line, but treat it as one FIXME TODO # [fixme]":UNDEFINED +fixme:16:1:None:None::XXX spaces are okay though # [fixme]:UNDEFINED +fixme:20:1:None:None::FIXME should still work:UNDEFINED +fixme:23:1:None:None::"TODO """""" should work":UNDEFINED +fixme:32:1:None:None::"TO make something DO: look a regex":UNDEFINED +fixme:40:5:None:None::./TODO implement this:UNDEFINED From 90df10b596984cf2683a0f5d3c89cedd58f68d49 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sat, 22 Jun 2024 04:00:59 -0400 Subject: [PATCH 02/15] Add changelog --- doc/whatsnew/fragments/9255.feature | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/whatsnew/fragments/9255.feature diff --git a/doc/whatsnew/fragments/9255.feature b/doc/whatsnew/fragments/9255.feature new file mode 100644 index 0000000000..d01726288e --- /dev/null +++ b/doc/whatsnew/fragments/9255.feature @@ -0,0 +1,3 @@ +Add ability for `fixme` check to also search through docstrings. + +Closes #9255 From 4b4ea5c5037b13a9c2bcb8069c53b2c67a920432 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 30 Jun 2024 16:04:37 -0400 Subject: [PATCH 03/15] Make python 3.8 compatible --- pylint/checkers/misc.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index aadc5583ea..b8eccf152a 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -160,12 +160,16 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: elif self.linter.config.check_fixme_in_docstring and self._is_docstring_comment(token_info): docstring_lines = token_info.string.split("\n") for line_no, line in enumerate(docstring_lines): - comment_text = line.removeprefix('"""').lstrip().removesuffix('"""') # trim '""""' and whitespace - if self._docstring_fixme_pattern.search('"""' + comment_text.lower()): + if line.startswith('"""'): + line = line[3:] + line = line.lstrip() + if line.endswith('"""'): + line = line[:-3] + if self._docstring_fixme_pattern.search('"""' + line.lower()): self.add_message( "fixme", col_offset=token_info.start[1] + 1, - args=comment_text, + args=line, line=token_info.start[0] + line_no, ) From b08d311f2b6f3dc71e72fa741eb54a14ce76f8d9 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 30 Jun 2024 16:21:55 -0400 Subject: [PATCH 04/15] Support single quote docstrings --- pylint/checkers/misc.py | 18 ++++++++++++------ tests/checkers/unittest_misc.py | 23 +++++++++++++++++++++++ tests/functional/f/fixme_docstring.py | 11 +++++++++++ tests/functional/f/fixme_docstring.txt | 4 ++++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index b8eccf152a..ff580c2fbf 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -109,13 +109,13 @@ def open(self) -> None: comment_regex = rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" self._comment_fixme_pattern = re.compile(comment_regex, re.I) if self.linter.config.check_fixme_in_docstring: - docstring_regex = rf"\"\"\"\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + docstring_regex = rf"((\"\"\")|(\'\'\'))\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) else: comment_regex = rf"#\s*({notes})(?=(:|\s|\Z))" self._comment_fixme_pattern = re.compile(comment_regex, re.I) if self.linter.config.check_fixme_in_docstring: - docstring_regex = rf"\"\"\"\s*({notes})(?=(:|\s|\Z))" + docstring_regex = rf"((\"\"\")|(\'\'\'))\s*({notes})(?=(:|\s|\Z))" self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) def _check_encoding( @@ -160,12 +160,15 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: elif self.linter.config.check_fixme_in_docstring and self._is_docstring_comment(token_info): docstring_lines = token_info.string.split("\n") for line_no, line in enumerate(docstring_lines): - if line.startswith('"""'): + # trim '"""' at beginning or end and whitespace + if line.startswith('"""') or line.startswith("'''"): line = line[3:] line = line.lstrip() - if line.endswith('"""'): + if line.endswith('"""') or line.endswith("'''"): line = line[:-3] - if self._docstring_fixme_pattern.search('"""' + line.lower()): + if self._docstring_fixme_pattern.search( + '"""' + line.lower() + ) or self._docstring_fixme_pattern.search("'''" + line.lower()): self.add_message( "fixme", col_offset=token_info.start[1] + 1, @@ -174,7 +177,10 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: ) def _is_docstring_comment(self, token_info: tokenize.TokenInfo) -> bool: - return token_info.type == tokenize.STRING and token_info.line.lstrip().startswith('"""') + return token_info.type == tokenize.STRING and ( + token_info.line.lstrip().startswith('"""') + or token_info.line.lstrip().startswith("'''") + ) def register(linter: PyLinter) -> None: diff --git a/tests/checkers/unittest_misc.py b/tests/checkers/unittest_misc.py index 0918a2c182..60eaec3bdf 100644 --- a/tests/checkers/unittest_misc.py +++ b/tests/checkers/unittest_misc.py @@ -128,6 +128,17 @@ def test_docstring_with_message(self) -> None: ): self.checker.process_tokens(_tokenize_str(code)) + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_message_single_quote(self) -> None: + code = """ + \'\'\'FIXME message\'\'\' + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) + + ): + self.checker.process_tokens(_tokenize_str(code)) + @set_config(check_fixme_in_docstring=True) def test_docstring_with_nl_message(self) -> None: code = """ @@ -140,6 +151,18 @@ def test_docstring_with_nl_message(self) -> None: ): self.checker.process_tokens(_tokenize_str(code)) + @set_config(check_fixme_in_docstring=True) + def test_docstring_with_nl_message_single_quote(self) -> None: + code = """ + \'\'\' + FIXME message + \'\'\' + """ + with self.assertAddsMessages( + MessageTest(msg_id="fixme", line=3, args="FIXME message", col_offset=9) + ): + self.checker.process_tokens(_tokenize_str(code)) + @set_config(check_fixme_in_docstring=True) def test_docstring_with_nl_message_multi(self) -> None: code = """ diff --git a/tests/functional/f/fixme_docstring.py b/tests/functional/f/fixme_docstring.py index 8a5b62af8f..52fa42cb75 100644 --- a/tests/functional/f/fixme_docstring.py +++ b/tests/functional/f/fixme_docstring.py @@ -38,3 +38,14 @@ # +2: [fixme] def function(): """./TODO implement this""" + + +''' + XXX single quotes should be no different # [fixme] +''' +def function2(): + '''./TODO implement this''' # [fixme] + ''' + ./TODO some more work # [fixme] + FIXME as well as this # [fixme] + ''' diff --git a/tests/functional/f/fixme_docstring.txt b/tests/functional/f/fixme_docstring.txt index b0ff92e86f..52d63a5bcc 100644 --- a/tests/functional/f/fixme_docstring.txt +++ b/tests/functional/f/fixme_docstring.txt @@ -8,3 +8,7 @@ fixme:20:1:None:None::FIXME should still work:UNDEFINED fixme:23:1:None:None::"TODO """""" should work":UNDEFINED fixme:32:1:None:None::"TO make something DO: look a regex":UNDEFINED fixme:40:5:None:None::./TODO implement this:UNDEFINED +fixme:44:1:None:None::XXX single quotes should be no different # [fixme]:UNDEFINED +fixme:47:5:None:None::./TODO implement this:UNDEFINED +fixme:49:5:None:None::./TODO some more work # [fixme]:UNDEFINED +fixme:50:5:None:None::FIXME as well as this # [fixme]:UNDEFINED From 5d16e6e7c085c5a4b2aa9b678082d378d10aaadb Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 30 Jun 2024 16:49:11 -0400 Subject: [PATCH 05/15] Fix formatting and linting --- pylint/checkers/misc.py | 26 ++++++++++++++--------- tests/checkers/unittest_misc.py | 37 +++++++++++++++++---------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index ff580c2fbf..037ec07b79 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -96,9 +96,9 @@ class EncodingChecker(BaseTokenChecker, BaseRawFileChecker): "type": "yn", "metavar": "", "default": False, - "help": "Whether or not to search for fixme's in docstrings." - } - ) + "help": "Whether or not to search for fixme's in docstrings.", + }, + ), ) def open(self) -> None: @@ -106,7 +106,9 @@ def open(self) -> None: notes = "|".join(re.escape(note) for note in self.linter.config.notes) if self.linter.config.notes_rgx: - comment_regex = rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + comment_regex = ( + rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" + ) self._comment_fixme_pattern = re.compile(comment_regex, re.I) if self.linter.config.check_fixme_in_docstring: docstring_regex = rf"((\"\"\")|(\'\'\'))\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" @@ -149,7 +151,9 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: return for token_info in tokens: if token_info.type == tokenize.COMMENT: - comment_text = token_info.string[1:].lstrip() # trim '#' and white-spaces + comment_text = token_info.string[ + 1: + ].lstrip() # trim '#' and white-spaces if self._comment_fixme_pattern.search("#" + comment_text.lower()): self.add_message( "fixme", @@ -157,14 +161,17 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: args=comment_text, line=token_info.start[0], ) - elif self.linter.config.check_fixme_in_docstring and self._is_docstring_comment(token_info): + elif ( + self.linter.config.check_fixme_in_docstring + and self._is_docstring_comment(token_info) + ): docstring_lines = token_info.string.split("\n") for line_no, line in enumerate(docstring_lines): # trim '"""' at beginning or end and whitespace - if line.startswith('"""') or line.startswith("'''"): + if line.startswith(('"""', "'''")): line = line[3:] line = line.lstrip() - if line.endswith('"""') or line.endswith("'''"): + if line.endswith(('"""', "'''")): line = line[:-3] if self._docstring_fixme_pattern.search( '"""' + line.lower() @@ -178,8 +185,7 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: def _is_docstring_comment(self, token_info: tokenize.TokenInfo) -> bool: return token_info.type == tokenize.STRING and ( - token_info.line.lstrip().startswith('"""') - or token_info.line.lstrip().startswith("'''") + token_info.line.lstrip().startswith(('"""', "'''")) ) diff --git a/tests/checkers/unittest_misc.py b/tests/checkers/unittest_misc.py index 60eaec3bdf..102d96b514 100644 --- a/tests/checkers/unittest_misc.py +++ b/tests/checkers/unittest_misc.py @@ -8,6 +8,7 @@ from pylint.testutils import CheckerTestCase, MessageTest, _tokenize_str, set_config +# pylint: disable=too-many-public-methods class TestFixme(CheckerTestCase): CHECKER_CLASS = misc.EncodingChecker @@ -124,7 +125,6 @@ def test_docstring_with_message(self) -> None: """ with self.assertAddsMessages( MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) - ): self.checker.process_tokens(_tokenize_str(code)) @@ -135,7 +135,6 @@ def test_docstring_with_message_single_quote(self) -> None: """ with self.assertAddsMessages( MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) - ): self.checker.process_tokens(_tokenize_str(code)) @@ -173,7 +172,7 @@ def test_docstring_with_nl_message_multi(self) -> None: """ with self.assertAddsMessages( MessageTest(msg_id="fixme", line=3, args="FIXME this", col_offset=9), - MessageTest(msg_id="fixme", line=4, args="TODO: that", col_offset=9) + MessageTest(msg_id="fixme", line=4, args="TODO: that", col_offset=9), ): self.checker.process_tokens(_tokenize_str(code)) @@ -189,7 +188,7 @@ def test_docstring_with_comment(self) -> None: with self.assertAddsMessages( MessageTest(msg_id="fixme", line=2, args="XXX message1", col_offset=9), MessageTest(msg_id="fixme", line=4, args="FIXME message2", col_offset=9), - MessageTest(msg_id="fixme", line=5, args="TODO message3", col_offset=9) + MessageTest(msg_id="fixme", line=5, args="TODO message3", col_offset=9), ): self.checker.process_tokens(_tokenize_str(code)) @@ -229,14 +228,13 @@ def test_docstring_todo_mult(self) -> None: \"\"\" """ with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="FIXME this TODO that", col_offset=9), + MessageTest( + msg_id="fixme", line=3, args="FIXME this TODO that", col_offset=9 + ), ): self.checker.process_tokens(_tokenize_str(code)) - - @set_config( - check_fixme_in_docstring=True, - notes=["CODETAG"] - ) + + @set_config(check_fixme_in_docstring=True, notes=["CODETAG"]) def test_docstring_custom_note(self) -> None: code = """ \"\"\" @@ -244,14 +242,13 @@ def test_docstring_custom_note(self) -> None: \"\"\" """ with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="CODETAG implement this", col_offset=9), + MessageTest( + msg_id="fixme", line=3, args="CODETAG implement this", col_offset=9 + ), ): self.checker.process_tokens(_tokenize_str(code)) - - @set_config( - check_fixme_in_docstring=True, - notes_rgx="FIX.*" - ) + + @set_config(check_fixme_in_docstring=True, notes_rgx="FIX.*") def test_docstring_custom_rgx(self) -> None: code = """ \"\"\" @@ -260,7 +257,11 @@ def test_docstring_custom_rgx(self) -> None: \"\"\" """ with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="FIXME implement this", col_offset=9), - MessageTest(msg_id="fixme", line=4, args="FIXTHIS also implement this", col_offset=9), + MessageTest( + msg_id="fixme", line=3, args="FIXME implement this", col_offset=9 + ), + MessageTest( + msg_id="fixme", line=4, args="FIXTHIS also implement this", col_offset=9 + ), ): self.checker.process_tokens(_tokenize_str(code)) From ec19e0ac9366dc07dfdc9d1d05a68f1fd861d4fe Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 30 Jun 2024 17:03:06 -0400 Subject: [PATCH 06/15] Update docs --- doc/user_guide/configuration/all-options.rst | 9 +++++++++ pylint/checkers/misc.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/user_guide/configuration/all-options.rst b/doc/user_guide/configuration/all-options.rst index eb7d72f2a3..0f728330eb 100644 --- a/doc/user_guide/configuration/all-options.rst +++ b/doc/user_guide/configuration/all-options.rst @@ -1151,6 +1151,13 @@ Standard Checkers ``Miscellaneous`` **Checker** ----------------------------- +--check-fixme-in-docstring +"""""""""""""""""""""""""" +*Whether or not to search for fixme's in docstrings.* + +**Default:** ``False`` + + --notes """"""" *List of note tags to take in consideration, separated by a comma.* @@ -1176,6 +1183,8 @@ Standard Checkers .. code-block:: toml [tool.pylint.miscellaneous] + check-fixme-in-docstring = false + notes = ["FIXME", "XXX", "TODO"] notes-rgx = "" diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index 037ec07b79..14488340c6 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -51,7 +51,7 @@ def process_module(self, node: nodes.Module) -> None: class EncodingChecker(BaseTokenChecker, BaseRawFileChecker): - """BaseChecker for encoding issues. + """BaseChecker for encoding issues and fixme notes. Checks for: * warning notes in the code like FIXME, XXX From 002d8043866c006dd5ca5a8dbbd55944d0d711cd Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 30 Jun 2024 18:00:31 -0400 Subject: [PATCH 07/15] Fix pypy38 failure --- tests/functional/f/fixme_docstring.py | 6 +++--- tests/functional/f/fixme_docstring.txt | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/f/fixme_docstring.py b/tests/functional/f/fixme_docstring.py index 52fa42cb75..59071eca94 100644 --- a/tests/functional/f/fixme_docstring.py +++ b/tests/functional/f/fixme_docstring.py @@ -44,8 +44,8 @@ def function(): XXX single quotes should be no different # [fixme] ''' def function2(): - '''./TODO implement this''' # [fixme] ''' - ./TODO some more work # [fixme] - FIXME as well as this # [fixme] + ./TODO implement this # [fixme] + FIXME and this # [fixme] ''' + '''FIXME one more for good measure''' # [fixme] diff --git a/tests/functional/f/fixme_docstring.txt b/tests/functional/f/fixme_docstring.txt index 52d63a5bcc..ce2e2149db 100644 --- a/tests/functional/f/fixme_docstring.txt +++ b/tests/functional/f/fixme_docstring.txt @@ -9,6 +9,6 @@ fixme:23:1:None:None::"TODO """""" should work":UNDEFINED fixme:32:1:None:None::"TO make something DO: look a regex":UNDEFINED fixme:40:5:None:None::./TODO implement this:UNDEFINED fixme:44:1:None:None::XXX single quotes should be no different # [fixme]:UNDEFINED -fixme:47:5:None:None::./TODO implement this:UNDEFINED -fixme:49:5:None:None::./TODO some more work # [fixme]:UNDEFINED -fixme:50:5:None:None::FIXME as well as this # [fixme]:UNDEFINED +fixme:48:5:None:None::./TODO implement this # [fixme]:UNDEFINED +fixme:49:5:None:None::FIXME and this # [fixme]:UNDEFINED +fixme:51:5:None:None::FIXME one more for good measure:UNDEFINED From 198fd93ca02ba16b9619d2a42d218ad09b5cfa6d Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 7 Jul 2024 22:40:44 -0400 Subject: [PATCH 08/15] Refactor to use regex capture groups instead of string processing to get the fixme message --- pylint/checkers/misc.py | 84 ++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index 14488340c6..0f21d898a0 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -106,19 +106,21 @@ def open(self) -> None: notes = "|".join(re.escape(note) for note in self.linter.config.notes) if self.linter.config.notes_rgx: - comment_regex = ( - rf"#\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" - ) - self._comment_fixme_pattern = re.compile(comment_regex, re.I) - if self.linter.config.check_fixme_in_docstring: - docstring_regex = rf"((\"\"\")|(\'\'\'))\s*({notes}|{self.linter.config.notes_rgx})(?=(:|\s|\Z))" - self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) - else: - comment_regex = rf"#\s*({notes})(?=(:|\s|\Z))" - self._comment_fixme_pattern = re.compile(comment_regex, re.I) - if self.linter.config.check_fixme_in_docstring: - docstring_regex = rf"((\"\"\")|(\'\'\'))\s*({notes})(?=(:|\s|\Z))" - self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) + notes += f"|{self.linter.config.notes_rgx}" + + comment_regex = rf"#\s*(?P({notes})(?=(:|\s|\Z)).*$)" + self._comment_fixme_pattern = re.compile(comment_regex, re.I) + + # single line docstring like '''this''' or """this""" + docstring_regex = rf"((\"\"\")|(\'\'\'))\s*(?P({notes})(?=(:|\s|\Z)).*?)((\"\"\")|(\'\'\'))" + self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) + + # multiline docstrings which will be split into newlines + # so we do not need to look for quotes/doublequotes + multiline_docstring_regex = rf"^\s*(?P({notes})(?=(:|\s|\Z)).*$)" + self._multiline_docstring_fixme_pattern = re.compile( + multiline_docstring_regex, re.I + ) def _check_encoding( self, lineno: int, line: bytes, file_encoding: str @@ -151,41 +153,37 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: return for token_info in tokens: if token_info.type == tokenize.COMMENT: - comment_text = token_info.string[ - 1: - ].lstrip() # trim '#' and white-spaces - if self._comment_fixme_pattern.search("#" + comment_text.lower()): + if match := self._comment_fixme_pattern.match(token_info.string): self.add_message( "fixme", col_offset=token_info.start[1] + 1, - args=comment_text, + args=match.group("msg"), line=token_info.start[0], ) - elif ( - self.linter.config.check_fixme_in_docstring - and self._is_docstring_comment(token_info) - ): - docstring_lines = token_info.string.split("\n") - for line_no, line in enumerate(docstring_lines): - # trim '"""' at beginning or end and whitespace - if line.startswith(('"""', "'''")): - line = line[3:] - line = line.lstrip() - if line.endswith(('"""', "'''")): - line = line[:-3] - if self._docstring_fixme_pattern.search( - '"""' + line.lower() - ) or self._docstring_fixme_pattern.search("'''" + line.lower()): - self.add_message( - "fixme", - col_offset=token_info.start[1] + 1, - args=line, - line=token_info.start[0] + line_no, - ) - - def _is_docstring_comment(self, token_info: tokenize.TokenInfo) -> bool: - return token_info.type == tokenize.STRING and ( - token_info.line.lstrip().startswith(('"""', "'''")) + elif self.linter.config.check_fixme_in_docstring: + if self._is_multiline_docstring(token_info): + docstring_lines = token_info.string.split("\n") + for line_no, line in enumerate(docstring_lines): + if match := self._multiline_docstring_fixme_pattern.match(line): + self.add_message( + "fixme", + col_offset=token_info.start[1] + 1, + args=match.group("msg"), + line=token_info.start[0] + line_no, + ) + elif match := self._docstring_fixme_pattern.match(token_info.string): + self.add_message( + "fixme", + col_offset=token_info.start[1] + 1, + args=match.group("msg"), + line=token_info.start[0], + ) + + def _is_multiline_docstring(self, token_info: tokenize.TokenInfo) -> bool: + return ( + token_info.type == tokenize.STRING + and (token_info.line.lstrip().startswith(('"""', "'''"))) + and "\n" in token_info.line.rstrip() ) From 767881377cbaf9182a9e5b80f3bc70b4b580328c Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 7 Jul 2024 22:45:39 -0400 Subject: [PATCH 09/15] Revert unit tests since functional tests cover all cases --- tests/checkers/unittest_misc.py | 149 -------------------------------- 1 file changed, 149 deletions(-) diff --git a/tests/checkers/unittest_misc.py b/tests/checkers/unittest_misc.py index 102d96b514..2f292b81f1 100644 --- a/tests/checkers/unittest_misc.py +++ b/tests/checkers/unittest_misc.py @@ -8,7 +8,6 @@ from pylint.testutils import CheckerTestCase, MessageTest, _tokenize_str, set_config -# pylint: disable=too-many-public-methods class TestFixme(CheckerTestCase): CHECKER_CLASS = misc.EncodingChecker @@ -117,151 +116,3 @@ def test_dont_trigger_on_todoist(self) -> None: """ with self.assertNoMessages(): self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_message(self) -> None: - code = """ - \"\"\"FIXME message\"\"\" - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_message_single_quote(self) -> None: - code = """ - \'\'\'FIXME message\'\'\' - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=2, args="FIXME message", col_offset=9) - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_nl_message(self) -> None: - code = """ - \"\"\" - FIXME message - \"\"\" - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="FIXME message", col_offset=9) - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_nl_message_single_quote(self) -> None: - code = """ - \'\'\' - FIXME message - \'\'\' - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="FIXME message", col_offset=9) - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_nl_message_multi(self) -> None: - code = """ - \"\"\" - FIXME this - TODO: that - \"\"\" - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=3, args="FIXME this", col_offset=9), - MessageTest(msg_id="fixme", line=4, args="TODO: that", col_offset=9), - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_comment(self) -> None: - code = """ - # XXX message1 - \"\"\" - FIXME message2 - TODO message3 - \"\"\" - """ - with self.assertAddsMessages( - MessageTest(msg_id="fixme", line=2, args="XXX message1", col_offset=9), - MessageTest(msg_id="fixme", line=4, args="FIXME message2", col_offset=9), - MessageTest(msg_id="fixme", line=5, args="TODO message3", col_offset=9), - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_with_comment_prefix(self) -> None: - code = """ - # \"\"\" XXX should not trigger - \"\"\" # XXX should not trigger \"\"\" - """ - with self.assertNoMessages(): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_todo_middle_nl(self) -> None: - code = """ - \"\"\" - something FIXME message - \"\"\" - """ - with self.assertNoMessages(): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_todo_middle(self) -> None: - code = """ - \"\"\"something FIXME message - \"\"\" - """ - with self.assertNoMessages(): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True) - def test_docstring_todo_mult(self) -> None: - code = """ - \"\"\" - FIXME this TODO that - \"\"\" - """ - with self.assertAddsMessages( - MessageTest( - msg_id="fixme", line=3, args="FIXME this TODO that", col_offset=9 - ), - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True, notes=["CODETAG"]) - def test_docstring_custom_note(self) -> None: - code = """ - \"\"\" - CODETAG implement this - \"\"\" - """ - with self.assertAddsMessages( - MessageTest( - msg_id="fixme", line=3, args="CODETAG implement this", col_offset=9 - ), - ): - self.checker.process_tokens(_tokenize_str(code)) - - @set_config(check_fixme_in_docstring=True, notes_rgx="FIX.*") - def test_docstring_custom_rgx(self) -> None: - code = """ - \"\"\" - FIXME implement this - FIXTHIS also implement this - \"\"\" - """ - with self.assertAddsMessages( - MessageTest( - msg_id="fixme", line=3, args="FIXME implement this", col_offset=9 - ), - MessageTest( - msg_id="fixme", line=4, args="FIXTHIS also implement this", col_offset=9 - ), - ): - self.checker.process_tokens(_tokenize_str(code)) From 1ce441c7960421ddd36fb71ea7c346e5652da978 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 7 Jul 2024 23:37:41 -0400 Subject: [PATCH 10/15] Tweak regex to account for comments starting with multiple pound signs --- pylint/checkers/misc.py | 2 +- tests/functional/f/fixme.py | 4 +++- tests/functional/f/fixme.txt | 17 +++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index 0f21d898a0..dda169ebdb 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -108,7 +108,7 @@ def open(self) -> None: if self.linter.config.notes_rgx: notes += f"|{self.linter.config.notes_rgx}" - comment_regex = rf"#\s*(?P({notes})(?=(:|\s|\Z)).*$)" + comment_regex = rf"(#\s*)+(?P({notes})(?=(:|\s|\Z)).*$)" self._comment_fixme_pattern = re.compile(comment_regex, re.I) # single line docstring like '''this''' or """this""" diff --git a/tests/functional/f/fixme.py b/tests/functional/f/fixme.py index cdb3e304af..ece77a6e3f 100644 --- a/tests/functional/f/fixme.py +++ b/tests/functional/f/fixme.py @@ -3,7 +3,9 @@ # +1: [fixme] # FIXME: beep - +# +1: [fixme] +# # TODO: don't forget indented ones should trigger +# but not TODO: this def function(): variable = "FIXME: Ignore me!" diff --git a/tests/functional/f/fixme.txt b/tests/functional/f/fixme.txt index 53b6680202..9a599ff1b1 100644 --- a/tests/functional/f/fixme.txt +++ b/tests/functional/f/fixme.txt @@ -1,9 +1,10 @@ fixme:5:1:None:None::"FIXME: beep":UNDEFINED -fixme:11:20:None:None::"FIXME: Valid test":UNDEFINED -fixme:14:5:None:None::"TODO: Do something with the variables":UNDEFINED -fixme:16:18:None:None::"XXX: Fix this later":UNDEFINED -fixme:18:5:None:None::"FIXME: no space after hash":UNDEFINED -fixme:20:5:None:None::"todo: no space after hash":UNDEFINED -fixme:23:2:None:None::"FIXME: this is broken":UNDEFINED -fixme:25:5:None:None::"./TODO: find with notes":UNDEFINED -fixme:27:5:None:None::"TO make something DO: find with regex":UNDEFINED +fixme:7:1:None:None::"TODO: don't forget indented ones should trigger":UNDEFINED +fixme:13:20:None:None::"FIXME: Valid test":UNDEFINED +fixme:16:5:None:None::"TODO: Do something with the variables":UNDEFINED +fixme:18:18:None:None::"XXX: Fix this later":UNDEFINED +fixme:20:5:None:None::"FIXME: no space after hash":UNDEFINED +fixme:22:5:None:None::"todo: no space after hash":UNDEFINED +fixme:25:2:None:None::"FIXME: this is broken":UNDEFINED +fixme:27:5:None:None::"./TODO: find with notes":UNDEFINED +fixme:29:5:None:None::"TO make something DO: find with regex":UNDEFINED From d408f09afdce51fefdf5bebf8ca59b373ada9a5e Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 7 Jul 2024 23:38:36 -0400 Subject: [PATCH 11/15] Fix spelling --- pylint/checkers/misc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index dda169ebdb..fe0c226c74 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -116,7 +116,7 @@ def open(self) -> None: self._docstring_fixme_pattern = re.compile(docstring_regex, re.I) # multiline docstrings which will be split into newlines - # so we do not need to look for quotes/doublequotes + # so we do not need to look for quotes/double-quotes multiline_docstring_regex = rf"^\s*(?P({notes})(?=(:|\s|\Z)).*$)" self._multiline_docstring_fixme_pattern = re.compile( multiline_docstring_regex, re.I From 40d5ed89d60a5f35c9a597b938f8a2eb8c30a6ab Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Mon, 8 Jul 2024 00:34:45 -0400 Subject: [PATCH 12/15] Revert how comment fixmes extract a message --- pylint/checkers/misc.py | 9 ++++++--- tests/functional/f/fixme.py | 1 - tests/functional/f/fixme.txt | 18 +++++++++--------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index fe0c226c74..d0b5c4d6c4 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -108,7 +108,7 @@ def open(self) -> None: if self.linter.config.notes_rgx: notes += f"|{self.linter.config.notes_rgx}" - comment_regex = rf"(#\s*)+(?P({notes})(?=(:|\s|\Z)).*$)" + comment_regex = rf"#\s*({notes})(?=(:|\s|\Z))" self._comment_fixme_pattern = re.compile(comment_regex, re.I) # single line docstring like '''this''' or """this""" @@ -153,11 +153,14 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: return for token_info in tokens: if token_info.type == tokenize.COMMENT: - if match := self._comment_fixme_pattern.match(token_info.string): + comment_text = token_info.string[ + 1: + ].lstrip() # trim '#' and white-spaces + if self._comment_fixme_pattern.search("#" + comment_text.lower()): self.add_message( "fixme", col_offset=token_info.start[1] + 1, - args=match.group("msg"), + args=comment_text, line=token_info.start[0], ) elif self.linter.config.check_fixme_in_docstring: diff --git a/tests/functional/f/fixme.py b/tests/functional/f/fixme.py index ece77a6e3f..517176acce 100644 --- a/tests/functional/f/fixme.py +++ b/tests/functional/f/fixme.py @@ -5,7 +5,6 @@ # FIXME: beep # +1: [fixme] # # TODO: don't forget indented ones should trigger -# but not TODO: this def function(): variable = "FIXME: Ignore me!" diff --git a/tests/functional/f/fixme.txt b/tests/functional/f/fixme.txt index 9a599ff1b1..f1c51c8d4d 100644 --- a/tests/functional/f/fixme.txt +++ b/tests/functional/f/fixme.txt @@ -1,10 +1,10 @@ fixme:5:1:None:None::"FIXME: beep":UNDEFINED -fixme:7:1:None:None::"TODO: don't forget indented ones should trigger":UNDEFINED -fixme:13:20:None:None::"FIXME: Valid test":UNDEFINED -fixme:16:5:None:None::"TODO: Do something with the variables":UNDEFINED -fixme:18:18:None:None::"XXX: Fix this later":UNDEFINED -fixme:20:5:None:None::"FIXME: no space after hash":UNDEFINED -fixme:22:5:None:None::"todo: no space after hash":UNDEFINED -fixme:25:2:None:None::"FIXME: this is broken":UNDEFINED -fixme:27:5:None:None::"./TODO: find with notes":UNDEFINED -fixme:29:5:None:None::"TO make something DO: find with regex":UNDEFINED +fixme:7:1:None:None::"# TODO: don't forget indented ones should trigger":UNDEFINED +fixme:12:20:None:None::"FIXME: Valid test":UNDEFINED +fixme:15:5:None:None::"TODO: Do something with the variables":UNDEFINED +fixme:17:18:None:None::"XXX: Fix this later":UNDEFINED +fixme:19:5:None:None::"FIXME: no space after hash":UNDEFINED +fixme:21:5:None:None::"todo: no space after hash":UNDEFINED +fixme:24:2:None:None::"FIXME: this is broken":UNDEFINED +fixme:26:5:None:None::"./TODO: find with notes":UNDEFINED +fixme:28:5:None:None::"TO make something DO: find with regex":UNDEFINED From eea95d23a71ae38441ce17a753b7f3a5a4b3d705 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 14 Jul 2024 00:16:25 -0400 Subject: [PATCH 13/15] Change fixme logic to allow only spaces between hash and fixme keyword --- pylint/checkers/misc.py | 9 +++------ tests/functional/f/fixme.py | 8 +++++++- tests/functional/f/fixme.txt | 20 +++++++++--------- tests/functional/f/fixme_docstring.py | 23 ++++++++++++--------- tests/functional/f/fixme_docstring.txt | 28 ++++++++++++++------------ 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index d0b5c4d6c4..ea2d9e324f 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -108,7 +108,7 @@ def open(self) -> None: if self.linter.config.notes_rgx: notes += f"|{self.linter.config.notes_rgx}" - comment_regex = rf"#\s*({notes})(?=(:|\s|\Z))" + comment_regex = rf"#\s*(?P({notes})(?=(:|\s|\Z)).*?$)" self._comment_fixme_pattern = re.compile(comment_regex, re.I) # single line docstring like '''this''' or """this""" @@ -153,14 +153,11 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: return for token_info in tokens: if token_info.type == tokenize.COMMENT: - comment_text = token_info.string[ - 1: - ].lstrip() # trim '#' and white-spaces - if self._comment_fixme_pattern.search("#" + comment_text.lower()): + if match := self._comment_fixme_pattern.match(token_info.string): self.add_message( "fixme", col_offset=token_info.start[1] + 1, - args=comment_text, + args=match.group("msg"), line=token_info.start[0], ) elif self.linter.config.check_fixme_in_docstring: diff --git a/tests/functional/f/fixme.py b/tests/functional/f/fixme.py index 517176acce..7ff749d94c 100644 --- a/tests/functional/f/fixme.py +++ b/tests/functional/f/fixme.py @@ -4,7 +4,13 @@ # +1: [fixme] # FIXME: beep # +1: [fixme] -# # TODO: don't forget indented ones should trigger + # TODO: don't forget indented ones should trigger +# +1: [fixme] +# TODO: that precedes another TODO: is treated as one and the message starts after the first +# +1: [fixme] +# TODO: large indentations after hash are okay + +# but things cannot precede the TODO: do this def function(): variable = "FIXME: Ignore me!" diff --git a/tests/functional/f/fixme.txt b/tests/functional/f/fixme.txt index f1c51c8d4d..e70b4d7461 100644 --- a/tests/functional/f/fixme.txt +++ b/tests/functional/f/fixme.txt @@ -1,10 +1,12 @@ fixme:5:1:None:None::"FIXME: beep":UNDEFINED -fixme:7:1:None:None::"# TODO: don't forget indented ones should trigger":UNDEFINED -fixme:12:20:None:None::"FIXME: Valid test":UNDEFINED -fixme:15:5:None:None::"TODO: Do something with the variables":UNDEFINED -fixme:17:18:None:None::"XXX: Fix this later":UNDEFINED -fixme:19:5:None:None::"FIXME: no space after hash":UNDEFINED -fixme:21:5:None:None::"todo: no space after hash":UNDEFINED -fixme:24:2:None:None::"FIXME: this is broken":UNDEFINED -fixme:26:5:None:None::"./TODO: find with notes":UNDEFINED -fixme:28:5:None:None::"TO make something DO: find with regex":UNDEFINED +fixme:7:5:None:None::"TODO: don't forget indented ones should trigger":UNDEFINED +fixme:9:1:None:None::"TODO: that precedes another TODO: is treated as one and the message starts after the first":UNDEFINED +fixme:11:1:None:None::"TODO: large indentations after hash are okay":UNDEFINED +fixme:18:20:None:None::"FIXME: Valid test":UNDEFINED +fixme:21:5:None:None::"TODO: Do something with the variables":UNDEFINED +fixme:23:18:None:None::"XXX: Fix this later":UNDEFINED +fixme:25:5:None:None::"FIXME: no space after hash":UNDEFINED +fixme:27:5:None:None::"todo: no space after hash":UNDEFINED +fixme:30:2:None:None::"FIXME: this is broken":UNDEFINED +fixme:32:5:None:None::"./TODO: find with notes":UNDEFINED +fixme:34:5:None:None::"TO make something DO: find with regex":UNDEFINED diff --git a/tests/functional/f/fixme_docstring.py b/tests/functional/f/fixme_docstring.py index 59071eca94..918ec00b07 100644 --- a/tests/functional/f/fixme_docstring.py +++ b/tests/functional/f/fixme_docstring.py @@ -3,17 +3,22 @@ # +1: [fixme] """TODO resolve this""" +# +1: [fixme] +""" TODO: indentations are permitted """ +# +1: [fixme] +''' TODO: indentations are permitted ''' +# +1: [fixme] +""" TODO: indentations are permitted""" + +""" preceding text TODO: is not permitted""" """ FIXME don't forget this # [fixme] XXX also remember this # [fixme] -??? but not this - - TODO yes this # [fixme] - FIXME: and this line, but treat it as one FIXME TODO # [fixme] -# TODO not this, however, even though it looks like a comment -also not if there's stuff in front TODO - XXX spaces are okay though # [fixme] +FIXME: and this line, but treat it as one FIXME TODO # [fixme] +text cannot precede the TODO: it must be at the start + XXX indentations are okay # [fixme] +??? the codetag must be recognized """ # +1: [fixme] @@ -22,8 +27,8 @@ # +1: [fixme] # TODO """ should work -# """ TODO should not work -"""# TODO neither should this""" +# """ TODO will not work +"""# TODO will not work""" """TODOist API should not result in a message""" diff --git a/tests/functional/f/fixme_docstring.txt b/tests/functional/f/fixme_docstring.txt index ce2e2149db..b0b19a9ebb 100644 --- a/tests/functional/f/fixme_docstring.txt +++ b/tests/functional/f/fixme_docstring.txt @@ -1,14 +1,16 @@ fixme:5:1:None:None::TODO resolve this:UNDEFINED -fixme:8:1:None:None::FIXME don't forget this # [fixme]:UNDEFINED -fixme:9:1:None:None::XXX also remember this # [fixme]:UNDEFINED -fixme:12:1:None:None::TODO yes this # [fixme]:UNDEFINED -fixme:13:1:None:None::"FIXME: and this line, but treat it as one FIXME TODO # [fixme]":UNDEFINED -fixme:16:1:None:None::XXX spaces are okay though # [fixme]:UNDEFINED -fixme:20:1:None:None::FIXME should still work:UNDEFINED -fixme:23:1:None:None::"TODO """""" should work":UNDEFINED -fixme:32:1:None:None::"TO make something DO: look a regex":UNDEFINED -fixme:40:5:None:None::./TODO implement this:UNDEFINED -fixme:44:1:None:None::XXX single quotes should be no different # [fixme]:UNDEFINED -fixme:48:5:None:None::./TODO implement this # [fixme]:UNDEFINED -fixme:49:5:None:None::FIXME and this # [fixme]:UNDEFINED -fixme:51:5:None:None::FIXME one more for good measure:UNDEFINED +fixme:7:1:None:None::"TODO: indentations are permitted ":UNDEFINED +fixme:9:1:None:None::"TODO: indentations are permitted ":UNDEFINED +fixme:11:1:None:None::"TODO: indentations are permitted":UNDEFINED +fixme:16:1:None:None::FIXME don't forget this # [fixme]:UNDEFINED +fixme:17:1:None:None::XXX also remember this # [fixme]:UNDEFINED +fixme:18:1:None:None::"FIXME: and this line, but treat it as one FIXME TODO # [fixme]":UNDEFINED +fixme:20:1:None:None::XXX indentations are okay # [fixme]:UNDEFINED +fixme:25:1:None:None::FIXME should still work:UNDEFINED +fixme:28:1:None:None::"TODO """""" should work":UNDEFINED +fixme:37:1:None:None::"TO make something DO: look a regex":UNDEFINED +fixme:45:5:None:None::./TODO implement this:UNDEFINED +fixme:49:1:None:None::XXX single quotes should be no different # [fixme]:UNDEFINED +fixme:53:5:None:None::./TODO implement this # [fixme]:UNDEFINED +fixme:54:5:None:None::FIXME and this # [fixme]:UNDEFINED +fixme:56:5:None:None::FIXME one more for good measure:UNDEFINED From cec41a25f1277c384cb67617ca8a57f5c345e4e6 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Thu, 25 Jul 2024 00:55:45 -0400 Subject: [PATCH 14/15] Improve wording --- doc/whatsnew/fragments/9255.feature | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/9255.feature b/doc/whatsnew/fragments/9255.feature index d01726288e..bc6b1033da 100644 --- a/doc/whatsnew/fragments/9255.feature +++ b/doc/whatsnew/fragments/9255.feature @@ -1,3 +1,4 @@ -Add ability for `fixme` check to also search through docstrings. +The `fixme` check can now search through docstrings as well as comments, by using +``check-fixme-in-docstring = true`` in the ``[tool.pylint.miscellaneous]`` section. Closes #9255 From d06c5622a9b9ffa6f977807c827b2c1eb027f551 Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Thu, 25 Jul 2024 01:24:26 -0400 Subject: [PATCH 15/15] Fix logic to avoid checking regex for every token --- pylint/checkers/misc.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index ea2d9e324f..1fd6c3eeca 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -160,8 +160,10 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: args=match.group("msg"), line=token_info.start[0], ) - elif self.linter.config.check_fixme_in_docstring: - if self._is_multiline_docstring(token_info): + elif self.linter.config.check_fixme_in_docstring and self._is_docstring( + token_info + ): + if "\n" in token_info.line.rstrip(): # multi-line docstring docstring_lines = token_info.string.split("\n") for line_no, line in enumerate(docstring_lines): if match := self._multiline_docstring_fixme_pattern.match(line): @@ -179,11 +181,9 @@ def process_tokens(self, tokens: list[tokenize.TokenInfo]) -> None: line=token_info.start[0], ) - def _is_multiline_docstring(self, token_info: tokenize.TokenInfo) -> bool: - return ( - token_info.type == tokenize.STRING - and (token_info.line.lstrip().startswith(('"""', "'''"))) - and "\n" in token_info.line.rstrip() + def _is_docstring(self, token_info: tokenize.TokenInfo) -> bool: + return token_info.type == tokenize.STRING and ( + token_info.line.lstrip().startswith(('"""', "'''")) )