From 4564f6aaf288c36cdaa410f90423a84d80868df0 Mon Sep 17 00:00:00 2001 From: "Rogalski, Lukasz" Date: Sat, 25 Aug 2018 01:38:08 +0200 Subject: [PATCH] Fixme should be triggered only in comments Closes #2321 --- pylint/checkers/misc.py | 103 +++++++++++---------------- pylint/test/unittest_checker_misc.py | 88 +++++++++++------------ 2 files changed, 81 insertions(+), 110 deletions(-) diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py index b0f52ca8b3..aa1feafafd 100644 --- a/pylint/checkers/misc.py +++ b/pylint/checkers/misc.py @@ -20,9 +20,9 @@ # pylint: disable=W0511 -import re +import tokenize -from pylint.interfaces import IRawChecker +from pylint.interfaces import IRawChecker, ITokenChecker from pylint.checkers import BaseChecker from pylint.utils import OPTION_RGX, MessagesHandlerMixIn @@ -69,7 +69,7 @@ class EncodingChecker(BaseChecker): * encoding issues. """ - __implements__ = IRawChecker + __implements__ = (IRawChecker, ITokenChecker) # configuration section name name = "miscellaneous" @@ -103,53 +103,6 @@ class EncodingChecker(BaseChecker): ), ) - def _check_note(self, notes, lineno, line, module_last_lineno): - """ - Add the message 'fixme' in case a note is found in the line. - - :param notes: regular expression object matching any notes - (XXX, TODO, FIXME) behind a '#' - :type notes: re.pattern object - :param lineno: line number - :type lineno: int - :param line: line to be checked - :type line: str - :param module_last_lineno: last line number of the module as parsed by astroid - (may be different from real last line number in case - commented lines exist at the end of the module) - :type module_last_lineno: int - """ - match = notes.search(line) - if not match: - return - # In case the module ends with commented lines, the astroid parser - # don't take into account those lines, then: - # - the line number of those lines is greater than the - # module last line number (module.tolineno) - # - astroid module object can't inform pylint - # of disabled messages in those extra lines. - if lineno > module_last_lineno: - disable_option_match = OPTION_RGX.search(line) - if disable_option_match: - try: - _, value = disable_option_match.group(1).split("=", 1) - values = [_val.strip().upper() for _val in value.split(",")] - if set(values) & set(self.config.notes): - return - except ValueError: - self.add_message( - "bad-inline-option", - args=disable_option_match.group(1).strip(), - line=line, - ) - return - self.add_message( - "fixme", - args=line[match.start(1) :].rstrip(), - line=lineno, - col_offset=match.start(1), - ) - def _check_encoding(self, lineno, line, file_encoding): try: return line.decode(file_encoding) @@ -167,15 +120,7 @@ def _check_encoding(self, lineno, line, file_encoding): ) def process_module(self, module): - """inspect the source file to find encoding problem or fixmes like - notes - """ - if self.config.notes: - notes = re.compile( - r"#\s*(%s)\b" % "|".join(map(re.escape, self.config.notes)), re.I - ) - else: - notes = None + """inspect the source file to find encoding problem""" if module.file_encoding: encoding = module.file_encoding else: @@ -183,9 +128,43 @@ def process_module(self, module): with module.stream() as stream: for lineno, line in enumerate(stream): - line = self._check_encoding(lineno + 1, line, encoding) - if line is not None and notes: - self._check_note(notes, lineno + 1, line, module.tolineno) + self._check_encoding(lineno + 1, line, encoding) + + def process_tokens(self, tokens): + """inspect the source to find fixme problems""" + if not self.config.notes: + return + comments = [ + token_info for token_info in tokens if token_info.type == tokenize.COMMENT + ] + for comment in comments: + comment_text = comment.string[1:].lstrip() # trim '#' and whitespaces + + # handle pylint disable clauses + disable_option_match = OPTION_RGX.search(comment_text) + if disable_option_match: + try: + _, value = disable_option_match.group(1).split("=", 1) + values = [_val.strip().upper() for _val in value.split(",")] + if set(values) & set(self.config.notes): + return + except ValueError: + self.add_message( + "bad-inline-option", + args=disable_option_match.group(1).strip(), + line=comment.string, + ) + return + + # emit warnings if necessary + for note in self.config.notes: + if comment_text.lower().startswith(note.lower()): + self.add_message( + "fixme", + args=comment_text, + line=comment.start[0], + col_offset=comment.string.lower().index(note.lower()), + ) def register(linter): diff --git a/pylint/test/unittest_checker_misc.py b/pylint/test/unittest_checker_misc.py index 5143698a07..fd189fdef3 100644 --- a/pylint/test/unittest_checker_misc.py +++ b/pylint/test/unittest_checker_misc.py @@ -13,85 +13,77 @@ """Tests for the misc checker.""" from pylint.checkers import misc -from pylint.testutils import ( - CheckerTestCase, - Message, - set_config, - _create_file_backed_module, -) +from pylint.testutils import CheckerTestCase, Message, set_config, _tokenize_str class TestFixme(CheckerTestCase): CHECKER_CLASS = misc.EncodingChecker def test_fixme_with_message(self): - with _create_file_backed_module( - """a = 1 + code = """a = 1 # FIXME message """ - ) as module: - with self.assertAddsMessages( - Message(msg_id="fixme", line=2, args="FIXME message") - ): - self.checker.process_module(module) + with self.assertAddsMessages( + Message(msg_id="fixme", line=2, args="FIXME message") + ): + self.checker.process_tokens(_tokenize_str(code)) def test_todo_without_message(self): - with _create_file_backed_module( - """a = 1 + code = """a = 1 # TODO """ - ) as module: - with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="TODO")): - self.checker.process_module(module) + with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="TODO")): + self.checker.process_tokens(_tokenize_str(code)) def test_xxx_without_space(self): - with _create_file_backed_module( - """a = 1 + code = """a = 1 #XXX """ - ) as module: - with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="XXX")): - self.checker.process_module(module) + with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="XXX")): + self.checker.process_tokens(_tokenize_str(code)) def test_xxx_middle(self): - with _create_file_backed_module( - """a = 1 + # THIS TEST STARTED TO FAIL, WHY? + code = """a = 1 # midle XXX """ - ) as module: - with self.assertNoMessages(): - self.checker.process_module(module) + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) def test_without_space_fixme(self): - with _create_file_backed_module( - """a = 1 + code = """a = 1 #FIXME """ - ) as module: - with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="FIXME")): - self.checker.process_module(module) + with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="FIXME")): + self.checker.process_tokens(_tokenize_str(code)) @set_config(notes=[]) def test_absent_codetag(self): - with _create_file_backed_module( - """a = 1 - # FIXME - # TODO - # XXX + code = """a = 1 + # FIXME # FIXME + # TODO # TODO + # XXX # XXX """ - ) as module: - with self.assertNoMessages(): - self.checker.process_module(module) + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) @set_config(notes=["CODETAG"]) def test_other_present_codetag(self): - with _create_file_backed_module( - """a = 1 + code = """a = 1 # CODETAG # FIXME """ - ) as module: - with self.assertAddsMessages( - Message(msg_id="fixme", line=2, args="CODETAG") - ): - self.checker.process_module(module) + with self.assertAddsMessages(Message(msg_id="fixme", line=2, args="CODETAG")): + self.checker.process_tokens(_tokenize_str(code)) + + def test_issue_2321_should_not_trigger(self): + code = 'print("# TODO this should not trigger a fixme")' + with self.assertNoMessages(): + self.checker.process_tokens(_tokenize_str(code)) + + def test_issue_2321_should_trigger(self): + code = "# TODO this should not trigger a fixme" + with self.assertAddsMessages( + Message(msg_id="fixme", line=1, args="TODO this should not trigger a fixme") + ): + self.checker.process_tokens(_tokenize_str(code))