From 6a312b2270a725080ea4387a8c2164fd6a69d289 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 6 Jul 2019 17:37:34 +0200 Subject: [PATCH 1/4] [pylint.message] Rename MessagesStore to MessageDefinitionStore --- pylint/lint.py | 4 ++-- pylint/message/__init__.py | 9 +++++++-- .../{message_store.py => message_definition_store.py} | 2 +- tests/message/unittest_message.py | 4 ++-- ...age_store.py => unittest_message_definition_store.py} | 4 ++-- 5 files changed, 14 insertions(+), 9 deletions(-) rename pylint/message/{message_store.py => message_definition_store.py} (99%) rename tests/message/{unittest_message_store.py => unittest_message_definition_store.py} (97%) diff --git a/pylint/lint.py b/pylint/lint.py index 275904d4b5..4066f8ef00 100644 --- a/pylint/lint.py +++ b/pylint/lint.py @@ -78,7 +78,7 @@ from pylint import checkers, config, exceptions, interfaces, reporters from pylint.__pkginfo__ import version from pylint.constants import MAIN_CHECKER_NAME, MSG_TYPES, OPTION_RGX -from pylint.message import Message, MessagesHandlerMixIn, MessagesStore +from pylint.message import Message, MessageDefinitionStore, MessagesHandlerMixIn from pylint.reporters.ureports import nodes as report_nodes from pylint.utils import ASTWalker, FileState, utils @@ -593,7 +593,7 @@ def __init__(self, options=(), reporter=None, option_groups=(), pylintrc=None): # some stuff has to be done before ancestors initialization... # # messages store / checkers / reporter / astroid manager - self.msgs_store = MessagesStore() + self.msgs_store = MessageDefinitionStore() self.reporter = None self._reporter_name = None self._reporters = {} diff --git a/pylint/message/__init__.py b/pylint/message/__init__.py index 4852dcfc10..d552a61a29 100644 --- a/pylint/message/__init__.py +++ b/pylint/message/__init__.py @@ -41,7 +41,12 @@ from pylint.message.message import Message from pylint.message.message_definition import MessageDefinition +from pylint.message.message_definition_store import MessageDefinitionStore from pylint.message.message_handler_mix_in import MessagesHandlerMixIn -from pylint.message.message_store import MessagesStore -__all__ = ["Message", "MessageDefinition", "MessagesHandlerMixIn", "MessagesStore"] +__all__ = [ + "Message", + "MessageDefinition", + "MessageDefinitionStore", + "MessagesHandlerMixIn", +] diff --git a/pylint/message/message_store.py b/pylint/message/message_definition_store.py similarity index 99% rename from pylint/message/message_store.py rename to pylint/message/message_definition_store.py index 8667d66945..8447a471fb 100644 --- a/pylint/message/message_store.py +++ b/pylint/message/message_definition_store.py @@ -10,7 +10,7 @@ from pylint.exceptions import InvalidMessageError, UnknownMessageError -class MessagesStore: +class MessageDefinitionStore: """The messages store knows information about every possible message but has no particular state during analysis. diff --git a/tests/message/unittest_message.py b/tests/message/unittest_message.py index 7420ca273f..3bc1578b5c 100644 --- a/tests/message/unittest_message.py +++ b/tests/message/unittest_message.py @@ -7,12 +7,12 @@ from pylint.checkers import BaseChecker from pylint.exceptions import InvalidMessageError -from pylint.message import MessageDefinition, MessagesStore +from pylint.message import MessageDefinition, MessageDefinitionStore @pytest.fixture def store(): - return MessagesStore() + return MessageDefinitionStore() @pytest.mark.parametrize( diff --git a/tests/message/unittest_message_store.py b/tests/message/unittest_message_definition_store.py similarity index 97% rename from tests/message/unittest_message_store.py rename to tests/message/unittest_message_definition_store.py index 0ad0ff7c6f..9133e488e8 100644 --- a/tests/message/unittest_message_store.py +++ b/tests/message/unittest_message_definition_store.py @@ -10,12 +10,12 @@ from pylint.checkers import BaseChecker from pylint.exceptions import InvalidMessageError, UnknownMessageError -from pylint.message import MessageDefinition, MessagesStore +from pylint.message import MessageDefinition, MessageDefinitionStore @pytest.fixture def store(): - store = MessagesStore() + store = MessageDefinitionStore() class Checker(BaseChecker): name = "achecker" From d741c543842f67f852c96a1afddfc67403ecf6fc Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Tue, 16 Jul 2019 23:59:08 +0200 Subject: [PATCH 2/4] [checkers.utils] Incompatible return value type (expected List[Any]) --- pylint/checkers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 5ce03de0a1..fd38bfe9ab 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -896,7 +896,7 @@ def get_exception_handlers( return [ handler for handler in context.handlers if error_of_type(handler, exception) ] - return None + return [] def is_node_inside_try_except(node: astroid.Raise) -> bool: From 9e2707c7b6f0e565317bb0e2d964c919568b1fce Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 23 Mar 2019 10:25:54 +0100 Subject: [PATCH 3/4] [pylint.message] Create a MessageId class Permit to have msgid and symbol together. --- pylint/message/__init__.py | 2 ++ pylint/message/message_definition.py | 28 +++++++-------- pylint/message/message_id.py | 31 ++++++++++++++++ tests/message/unittest_message.py | 13 ------- tests/message/unittest_message_id.py | 53 ++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 28 deletions(-) create mode 100644 pylint/message/message_id.py create mode 100644 tests/message/unittest_message_id.py diff --git a/pylint/message/__init__.py b/pylint/message/__init__.py index d552a61a29..20e00902f7 100644 --- a/pylint/message/__init__.py +++ b/pylint/message/__init__.py @@ -43,10 +43,12 @@ from pylint.message.message_definition import MessageDefinition from pylint.message.message_definition_store import MessageDefinitionStore from pylint.message.message_handler_mix_in import MessagesHandlerMixIn +from pylint.message.message_id import MessageId __all__ = [ "Message", "MessageDefinition", "MessageDefinitionStore", "MessagesHandlerMixIn", + "MessageId", ] diff --git a/pylint/message/message_definition.py b/pylint/message/message_definition.py index 873cdc3b81..f82c71597e 100644 --- a/pylint/message/message_definition.py +++ b/pylint/message/message_definition.py @@ -5,8 +5,7 @@ import sys -from pylint.constants import MSG_TYPES -from pylint.exceptions import InvalidMessageError +from pylint.message.message_id import MessageId from pylint.utils import normalize_text @@ -24,19 +23,23 @@ def __init__( old_names=None, ): self.checker = checker - if len(msgid) != 5: - raise InvalidMessageError("Invalid message id %r" % msgid) - if not msgid[0] in MSG_TYPES: - raise InvalidMessageError("Bad message type %s in %r" % (msgid[0], msgid)) - self.msgid = msgid + MessageId.check_msgid(msgid) + self.message_id = MessageId(msgid, symbol) self.msg = msg self.description = description - self.symbol = symbol self.scope = scope self.minversion = minversion self.maxversion = maxversion self.old_names = old_names or [] + @property + def symbol(self): + return self.message_id.symbol + + @property + def msgid(self): + return self.message_id.msgid + def __repr__(self): return "MessageDefinition:%s (%s)" % (self.symbol, self.msgid) @@ -57,10 +60,6 @@ def format_help(self, checkerref=False): if checkerref: desc += " This message belongs to the %s checker." % self.checker.name title = self.msg - if self.symbol: - msgid = "%s (%s)" % (self.symbol, self.msgid) - else: - msgid = self.msgid if self.minversion or self.maxversion: restr = [] if self.minversion: @@ -75,6 +74,5 @@ def format_help(self, checkerref=False): desc = normalize_text(" ".join(desc.split()), indent=" ") if title != "%s": title = title.splitlines()[0] - - return ":%s: *%s*\n%s" % (msgid, title.rstrip(" "), desc) - return ":%s:\n%s" % (msgid, desc) + return ":%s: *%s*\n%s" % (self.message_id, title.rstrip(" "), desc) + return ":%s:\n%s" % (self.message_id, desc) diff --git a/pylint/message/message_id.py b/pylint/message/message_id.py new file mode 100644 index 0000000000..43aad81e54 --- /dev/null +++ b/pylint/message/message_id.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- + +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/master/COPYING + +from pylint.constants import MSG_TYPES +from pylint.exceptions import InvalidMessageError + + +class MessageId: + def __init__(self, msgid, symbol): + self.msgid = msgid + self.symbol = symbol + + def __str__(self): + return "%s (%s)" % (self.symbol, self.msgid) + + def __hash__(self): + return "{}-{}".format(self.msgid, self.symbol).__hash__() + + def __eq__(self, other): + return self.msgid == other.msgid and self.symbol == other.symbol + + @staticmethod + def check_msgid(msgid: str) -> None: + """This is a static method used in MessageDefinition and not the + MessageId constructor for performance reasons.""" + if len(msgid) != 5: + raise InvalidMessageError("Invalid message id %r" % msgid) + if msgid[0] not in MSG_TYPES: + raise InvalidMessageError("Bad message type %s in %r" % (msgid[0], msgid)) diff --git a/tests/message/unittest_message.py b/tests/message/unittest_message.py index 3bc1578b5c..d11eb3681c 100644 --- a/tests/message/unittest_message.py +++ b/tests/message/unittest_message.py @@ -141,16 +141,3 @@ class CheckerTwo(BaseChecker): {"W1234": ("message two", "msg-symbol-two", "another msg description.")}, "Message id 'W1234' cannot have both 'msg-symbol-one' and 'msg-symbol-two' as symbolic name.", ) - - -@pytest.mark.parametrize( - "msgid,expected", - [ - ("Q1234", "Bad message type Q in 'Q1234'"), - ("W12345", "Invalid message id 'W12345'"), - ], -) -def test_create_invalid_message_type(msgid, expected): - with pytest.raises(InvalidMessageError) as cm: - MessageDefinition("checker", msgid, "msg", "descr", "symbol", "scope") - assert str(cm.value) == expected diff --git a/tests/message/unittest_message_id.py b/tests/message/unittest_message_id.py new file mode 100644 index 0000000000..b0224831c2 --- /dev/null +++ b/tests/message/unittest_message_id.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- + +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/master/COPYING + +import pytest + +from pylint.exceptions import InvalidMessageError +from pylint.message import MessageId + + +@pytest.fixture +def msgid(): + return "W1234" + + +@pytest.fixture +def symbol(): + return "msg-symbol" + + +@pytest.fixture +def message_id(msgid, symbol): + return MessageId(msgid, symbol) + + +@pytest.mark.parametrize( + "msgid,expected", + [ + ("Q1234", "Bad message type Q in 'Q1234'"), + ("W12345", "Invalid message id 'W12345'"), + ], +) +def test_create_invalid_message_type(msgid, expected): + with pytest.raises(InvalidMessageError) as cm: + MessageId.check_msgid(msgid) + assert str(cm.value) == expected + + +def test_hash_and_eq(message_id): + """MessageId should be hashable""" + dict_ = {} + message_id_2 = MessageId("W1235", "msg-symbol-2") + dict_[message_id] = 1 + dict_[message_id_2] = 2 + assert dict_[message_id] != dict_[message_id_2] + assert message_id != message_id_2 + + +def test_str(message_id, msgid, symbol): + result = str(message_id) + assert msgid in result + assert symbol in result From edd70677c0f706bdb2c41ae809c5db65defc8788 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 21 Jun 2019 13:50:06 +0200 Subject: [PATCH 4/4] [pylint.message] Create a function to explictely check message definition --- pylint/message/message_handler_mix_in.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pylint/message/message_handler_mix_in.py b/pylint/message/message_handler_mix_in.py index 912e76e925..8140c5d2f8 100644 --- a/pylint/message/message_handler_mix_in.py +++ b/pylint/message/message_handler_mix_in.py @@ -244,14 +244,11 @@ def add_message( message_definition, line, node, args, confidence, col_offset ) - def add_one_message( - self, message_definition, line, node, args, confidence, col_offset - ): - # backward compatibility, message may not have a symbol - symbol = message_definition.symbol or message_definition.msgid - # Fatal messages and reports are special, the node/scope distinction - # does not apply to them. + @staticmethod + def check_message_definition(message_definition, line, node): if message_definition.msgid[0] not in _SCOPE_EXEMPT: + # Fatal messages and reports are special, the node/scope distinction + # does not apply to them. if message_definition.scope == WarningScope.LINE: if line is None: raise InvalidMessageError( @@ -271,6 +268,12 @@ def add_one_message( % message_definition.msgid ) + def add_one_message( + self, message_definition, line, node, args, confidence, col_offset + ): + # backward compatibility, message may not have a symbol + symbol = message_definition.symbol or message_definition.msgid + self.check_message_definition(message_definition, line, node) if line is None and node is not None: line = node.fromlineno if col_offset is None and hasattr(node, "col_offset"):