diff options
| -rw-r--r-- | pylint/message/message_definition_store.py | 164 | ||||
| -rw-r--r-- | pylint/message/message_handler_mix_in.py | 3 | ||||
| -rw-r--r-- | tests/message/unittest_message.py | 187 | ||||
| -rw-r--r-- | tests/message/unittest_message_definition_store.py | 164 |
4 files changed, 225 insertions, 293 deletions
diff --git a/pylint/message/message_definition_store.py b/pylint/message/message_definition_store.py index fdd1f0b54..4d40c25e7 100644 --- a/pylint/message/message_definition_store.py +++ b/pylint/message/message_definition_store.py @@ -7,27 +7,27 @@ from __future__ import print_function import collections -from pylint.exceptions import InvalidMessageError, UnknownMessageError +from pylint.exceptions import UnknownMessageError +from pylint.message.message_id_store import MessageIdStore class MessageDefinitionStore: - """The messages store knows information about every possible message but has + """The messages store knows information about every possible message definition but has no particular state during analysis. """ def __init__(self): - # Primary registry for all active messages (i.e. all messages - # that can be emitted by pylint for the underlying Python - # version). It contains the 1:1 mapping from symbolic names - # to message definition objects. - # Keys are msg ids, values are a 2-uple with the msg type and the - # msg itself + self.message_id_store = MessageIdStore() + # Primary registry for all active messages definitions. + # It contains the 1:1 mapping from msgid to MessageDefinition. + # Keys are msgid, values are MessageDefinition self._messages_definitions = {} - # Maps alternative names (numeric IDs, deprecated names) to - # message definitions. May contain several names for each definition - # object. - self._alternative_names = {} + # Secondary registry for all old names kept for compatibility reasons + # May contain identical values under different MessageId + # (ie a MessageDefinition was renamed more than once) + self._old_message_definitions = {} + # MessageDefinition kept by category self._msgs_by_category = collections.defaultdict(list) @property @@ -36,7 +36,7 @@ class MessageDefinitionStore: return self._messages_definitions.values() def register_messages_from_checker(self, checker): - """Register all messages from a checker. + """Register all messages definitions from a checker. :param BaseChecker checker: """ @@ -49,106 +49,13 @@ class MessageDefinitionStore: :param MessageDefinition message: The message definition being added. """ - self._check_id_and_symbol_consistency(message.msgid, message.symbol) - self._check_symbol(message.msgid, message.symbol) - self._check_msgid(message.msgid, message.symbol) - for old_name in message.old_names: - self._check_symbol(message.msgid, old_name[1]) - self._messages_definitions[message.symbol] = message - self._register_alternative_name(message, message.msgid, message.symbol) - for old_id, old_symbol in message.old_names: - self._register_alternative_name(message, old_id, old_symbol) + self.message_id_store.register_message_definition(message) + self._messages_definitions[message.msgid] = message + self._old_message_definitions[message.msgid] = message + for old_msgid, _ in message.old_names: + self._old_message_definitions[old_msgid] = message self._msgs_by_category[message.msgid[0]].append(message.msgid) - def _register_alternative_name(self, msg, msgid, symbol): - """helper for register_message()""" - self._check_id_and_symbol_consistency(msgid, symbol) - self._alternative_names[msgid] = msg - self._alternative_names[symbol] = msg - - def _check_symbol(self, msgid, symbol): - """Check that a symbol is not already used. """ - other_message = self._messages_definitions.get(symbol) - if other_message: - self._raise_duplicate_msgid(symbol, msgid, other_message.msgid) - else: - alternative_msgid = None - alternative_message = self._alternative_names.get(symbol) - if alternative_message: - if alternative_message.symbol == symbol: - alternative_msgid = alternative_message.msgid - else: - for old_msgid, old_symbol in alternative_message.old_names: - if old_symbol == symbol: - alternative_msgid = old_msgid - break - if msgid != alternative_msgid: - self._raise_duplicate_msgid(symbol, msgid, alternative_msgid) - - def _check_msgid(self, msgid, symbol): - for message in self._messages_definitions.values(): - if message.msgid == msgid: - self._raise_duplicate_symbol(msgid, symbol, message.symbol) - - def _check_id_and_symbol_consistency(self, msgid, symbol): - try: - alternative = self._alternative_names[msgid] - except KeyError: - alternative = False - try: - if not alternative: - alternative = self._alternative_names[symbol] - except KeyError: - # There is no alternative names concerning this msgid/symbol. - # So nothing to check - return None - old_symbolic_name = None - old_symbolic_id = None - for alternate_msgid, alternate_symbol in alternative.old_names: - if alternate_msgid == msgid or alternate_symbol == symbol: - old_symbolic_id = alternate_msgid - old_symbolic_name = alternate_symbol - if symbol not in (alternative.symbol, old_symbolic_name): - if msgid == old_symbolic_id: - self._raise_duplicate_symbol(msgid, symbol, old_symbolic_name) - else: - self._raise_duplicate_symbol(msgid, symbol, alternative.symbol) - return None - - @staticmethod - def _raise_duplicate_symbol(msgid, symbol, other_symbol): - """Raise an error when a symbol is duplicated. - - :param str msgid: The msgid corresponding to the symbols - :param str symbol: Offending symbol - :param str other_symbol: Other offending symbol - :raises InvalidMessageError:""" - symbols = [symbol, other_symbol] - symbols.sort() - error_message = "Message id '{msgid}' cannot have both ".format(msgid=msgid) - error_message += "'{other_symbol}' and '{symbol}' as symbolic name.".format( - other_symbol=symbols[0], symbol=symbols[1] - ) - raise InvalidMessageError(error_message) - - @staticmethod - def _raise_duplicate_msgid(symbol, msgid, other_msgid): - """Raise an error when a msgid is duplicated. - - :param str symbol: The symbol corresponding to the msgids - :param str msgid: Offending msgid - :param str other_msgid: Other offending msgid - :raises InvalidMessageError:""" - msgids = [msgid, other_msgid] - msgids.sort() - error_message = "Message symbol '{symbol}' cannot be used for ".format( - symbol=symbol - ) - error_message += "'{other_msgid}' and '{msgid}' at the same time.".format( - other_msgid=msgids[0], msgid=msgids[1] - ) - raise InvalidMessageError(error_message) - def get_message_definitions(self, msgid_or_symbol: str) -> list: """Returns the Message object for this message. :param str msgid_or_symbol: msgid_or_symbol may be either a numeric or symbolic id. @@ -156,32 +63,29 @@ class MessageDefinitionStore: :rtype: List of MessageDefinition :return: A message definition corresponding to msgid_or_symbol """ - # Only msgid can have a digit as second letter - is_msgid = msgid_or_symbol[1:].isdigit() - if is_msgid: - msgid_or_symbol = msgid_or_symbol.upper() - for source in (self._alternative_names, self._messages_definitions): - try: - return [source[msgid_or_symbol]] - except KeyError: - pass - error_msg = "No such message id or symbol '{msgid_or_symbol}'.".format( - msgid_or_symbol=msgid_or_symbol - ) - raise UnknownMessageError(error_msg) - - def get_msg_display_string(self, msgid): + message_definitions = [] + message_ids = self.message_id_store.get_active_msgids(msgid_or_symbol) + for message_id in message_ids: + message_definition = self._messages_definitions.get(message_id) + if message_definition is None: + message_definition = self._old_message_definitions.get(message_id) + message_definitions.append(message_definition) + return message_definitions + + def get_msg_display_string(self, msgid_or_symbol: str): """Generates a user-consumable representation of a message. """ - message_definitions = self.get_message_definitions(msgid) + message_definitions = self.get_message_definitions(msgid_or_symbol) if len(message_definitions) == 1: return repr(message_definitions[0].symbol) return repr([md.symbol for md in message_definitions]) - def help_message(self, msgids): + def help_message(self, msgids_or_symbols: list): """Display help messages for the given message identifiers""" - for msgid in msgids: + for msgids_or_symbol in msgids_or_symbols: try: - for message_definition in self.get_message_definitions(msgid): + for message_definition in self.get_message_definitions( + msgids_or_symbol + ): print(message_definition.format_help(checkerref=True)) print("") except UnknownMessageError as ex: diff --git a/pylint/message/message_handler_mix_in.py b/pylint/message/message_handler_mix_in.py index 0750c9ee6..ad8a11f45 100644 --- a/pylint/message/message_handler_mix_in.py +++ b/pylint/message/message_handler_mix_in.py @@ -105,10 +105,9 @@ class MessagesHandlerMixIn: # msgid is a checker name? if msgid.lower() in self._checkers: - msgs_store = self.msgs_store for checker in self._checkers[msgid.lower()]: for _msgid in checker.msgs: - if _msgid in msgs_store._alternative_names: + if _msgid in self.msgs_store._old_message_definitions: self._set_msg_status(_msgid, enable, scope, line) return diff --git a/tests/message/unittest_message.py b/tests/message/unittest_message.py index d11eb3681..205d06dec 100644 --- a/tests/message/unittest_message.py +++ b/tests/message/unittest_message.py @@ -3,141 +3,54 @@ # 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.checkers import BaseChecker -from pylint.exceptions import InvalidMessageError -from pylint.message import MessageDefinition, MessageDefinitionStore - - -@pytest.fixture -def store(): - return MessageDefinitionStore() - - -@pytest.mark.parametrize( - "messages,expected", - [ - ( - { - "W1234": ("message one", "msg-symbol-one", "msg description"), - "W4321": ("message two", "msg-symbol-two", "msg description"), - }, - r"Inconsistent checker part in message id 'W4321' (expected 'x12xx' because we already had ['W1234']).", - ), - ( - { - "W1233": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1234", "old-symbol")]}, - ), - "W1234": ("message one", "msg-symbol-one", "msg description"), - }, - "Message id 'W1234' cannot have both 'msg-symbol-one' and 'old-symbol' as symbolic name.", - ), - ( - { - "W1234": ("message one", "msg-symbol-one", "msg description"), - "W1235": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1234", "old-symbol")]}, - ), - }, - "Message id 'W1234' cannot have both 'msg-symbol-one' and 'old-symbol' as symbolic name.", - ), - ( - { - "W1234": ( - "message one", - "msg-symbol-one", - "msg description", - {"old_names": [("W1201", "old-symbol-one")]}, - ), - "W1235": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1201", "old-symbol-two")]}, - ), - }, - "Message id 'W1201' cannot have both 'old-symbol-one' and 'old-symbol-two' as symbolic name.", - ), - ( - { - "W1234": ("message one", "msg-symbol", "msg description"), - "W1235": ("message two", "msg-symbol", "msg description"), - }, - "Message symbol 'msg-symbol' cannot be used for 'W1234' and 'W1235' at the same time.", - ), - ( - { - "W1233": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1230", "msg-symbol-one")]}, - ), - "W1234": ("message one", "msg-symbol-one", "msg description"), - }, - "Message symbol 'msg-symbol-one' cannot be used for 'W1230' and 'W1234' at the same time.", - ), - ( - { - "W1234": ("message one", "msg-symbol-one", "msg description"), - "W1235": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1230", "msg-symbol-one")]}, - ), - }, - "Message symbol 'msg-symbol-one' cannot be used for 'W1234' and 'W1235' at the same time.", - ), - ( - { - "W1234": ( - "message one", - "msg-symbol-one", - "msg description", - {"old_names": [("W1230", "old-symbol-one")]}, - ), - "W1235": ( - "message two", - "msg-symbol-two", - "msg description", - {"old_names": [("W1231", "old-symbol-one")]}, - ), - }, - "Message symbol 'old-symbol-one' cannot be used for 'W1230' and 'W1235' at the same time.", - ), - ], -) -def test_register_error(store, messages, expected): - class Checker(BaseChecker): - name = "checker" - msgs = messages - - with pytest.raises(InvalidMessageError) as cm: - store.register_messages_from_checker(Checker()) - assert str(cm.value) == expected - - -def test_register_error_new_id_duplicate_of_new(store): - class CheckerOne(BaseChecker): - name = "checker_one" - msgs = {"W1234": ("message one", "msg-symbol-one", "msg description.")} - - class CheckerTwo(BaseChecker): - name = "checker_two" - msgs = {"W1234": ("message two", "msg-symbol-two", "another msg description.")} - - store.register_messages_from_checker(CheckerOne()) - test_register_error( - store, - {"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.", +from pylint.message import Message + +from .generic_fixtures import message_definitions, store + + +def test_new_message(message_definitions): + def build_message(message_definition, location_value): + return Message( + symbol=message_definition.symbol, + msg_id=message_definition.msgid, + location=[ + location_value["abspath"], + location_value["path"], + location_value["module"], + location_value["obj"], + location_value["line"], + location_value["column"], + ], + msg=message_definition.msg, + confidence="high", + ) + + template = "{path}:{line}:{column}: {msg_id}: {msg} ({symbol})" + for message_definition in message_definitions: + if message_definition.msgid == "E1234": + e1234_message_definition = message_definition + if message_definition.msgid == "W1234": + w1234_message_definition = message_definition + e1234_location_values = { + "abspath": "1", + "path": "2", + "module": "3", + "obj": "4", + "line": "5", + "column": "6", + } + w1234_location_values = { + "abspath": "7", + "path": "8", + "module": "9", + "obj": "10", + "line": "11", + "column": "12", + } + expected = ( + "2:5:6: E1234: Duplicate keyword argument %r in %s call (duplicate-keyword-arg)" ) + e1234 = build_message(e1234_message_definition, e1234_location_values) + w1234 = build_message(w1234_message_definition, w1234_location_values) + assert e1234.format(template) == expected + assert w1234.format(template) == "8:11:12: W1234: message (msg-symbol)" diff --git a/tests/message/unittest_message_definition_store.py b/tests/message/unittest_message_definition_store.py index c656781fe..3a4ebf349 100644 --- a/tests/message/unittest_message_definition_store.py +++ b/tests/message/unittest_message_definition_store.py @@ -10,32 +10,141 @@ import pytest from pylint.checkers import BaseChecker from pylint.exceptions import InvalidMessageError, UnknownMessageError -from pylint.message import MessageDefinition, MessageDefinitionStore +from pylint.message import MessageDefinition + +from .generic_fixtures import empty_store, store + + +@pytest.mark.parametrize( + "messages,expected", + [ + ( + { + "W1234": ("message one", "msg-symbol-one", "msg description"), + "W4321": ("message two", "msg-symbol-two", "msg description"), + }, + r"Inconsistent checker part in message id 'W4321' (expected 'x12xx' because we already had ['W1234']).", + ), + ( + { + "W1233": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1234", "old-symbol")]}, + ), + "W1234": ("message one", "msg-symbol-one", "msg description"), + }, + "Message id 'W1234' cannot have both 'msg-symbol-one' and 'old-symbol' as symbolic name.", + ), + ( + { + "W1234": ("message one", "msg-symbol-one", "msg description"), + "W1235": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1234", "old-symbol")]}, + ), + }, + "Message id 'W1234' cannot have both 'msg-symbol-one' and 'old-symbol' as symbolic name.", + ), + ( + { + "W1234": ( + "message one", + "msg-symbol-one", + "msg description", + {"old_names": [("W1201", "old-symbol-one")]}, + ), + "W1235": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1201", "old-symbol-two")]}, + ), + }, + "Message id 'W1201' cannot have both 'old-symbol-one' and 'old-symbol-two' as symbolic name.", + ), + ( + { + "W1234": ("message one", "msg-symbol", "msg description"), + "W1235": ("message two", "msg-symbol", "msg description"), + }, + "Message symbol 'msg-symbol' cannot be used for 'W1234' and 'W1235' at the same time. " + "If you're creating an 'old_names' use 'old-msg-symbol' as the old symbol.", + ), + ( + { + "W1233": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1230", "msg-symbol-one")]}, + ), + "W1234": ("message one", "msg-symbol-one", "msg description"), + }, + "Message symbol 'msg-symbol-one' cannot be used for 'W1230' and 'W1234' at the same time." + " If you're creating an 'old_names' use 'old-msg-symbol-one' as the old symbol.", + ), + ( + { + "W1234": ("message one", "msg-symbol-one", "msg description"), + "W1235": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1230", "msg-symbol-one")]}, + ), + }, + "Message symbol 'msg-symbol-one' cannot be used for 'W1230' and 'W1234' at the same time. " + "If you're creating an 'old_names' use 'old-msg-symbol-one' as the old symbol.", + ), + ( + { + "W1234": ( + "message one", + "msg-symbol-one", + "msg description", + {"old_names": [("W1230", "old-symbol-one")]}, + ), + "W1235": ( + "message two", + "msg-symbol-two", + "msg description", + {"old_names": [("W1231", "old-symbol-one")]}, + ), + }, + "Message symbol 'old-symbol-one' cannot be used for 'W1230' and 'W1231' at the same time. " + "If you're creating an 'old_names' use 'old-old-symbol-one' as the old symbol.", + ), + ], +) +def test_register_error(empty_store, messages, expected): + class Checker(BaseChecker): + name = "checker" + msgs = messages + with pytest.raises(InvalidMessageError) as cm: + empty_store.register_messages_from_checker(Checker()) + assert str(cm.value) == expected -@pytest.fixture -def store(): - store = MessageDefinitionStore() - class Checker(BaseChecker): - name = "achecker" - msgs = { - "W1234": ( - "message", - "msg-symbol", - "msg description.", - {"old_names": [("W0001", "old-symbol")]}, - ), - "E1234": ( - "Duplicate keyword argument %r in %s call", - "duplicate-keyword-arg", - "Used when a function call passes the same keyword argument multiple times.", - {"maxversion": (2, 6)}, - ), - } - - store.register_messages_from_checker(Checker()) - return store +def test_register_error_new_id_duplicate_of_new(empty_store): + class CheckerOne(BaseChecker): + name = "checker_one" + msgs = {"W1234": ("message one", "msg-symbol-one", "msg description.")} + + class CheckerTwo(BaseChecker): + name = "checker_two" + msgs = {"W1234": ("message two", "msg-symbol-two", "another msg description.")} + + empty_store.register_messages_from_checker(CheckerOne()) + test_register_error( + empty_store, + {"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.", + ) def test_format_help(capsys, store): @@ -70,7 +179,14 @@ class TestMessageDefinitionStore(object): assert desc == msg.format_help(checkerref=checkerref) def test_check_message_id(self, store): - assert isinstance(store.get_message_definitions("W1234")[0], MessageDefinition) + w1234 = store.get_message_definitions("W1234")[0] + w0001 = store.get_message_definitions("W0001")[0] + e1234 = store.get_message_definitions("E1234")[0] + old_symbol = store.get_message_definitions("old-symbol")[0] + assert isinstance(w1234, MessageDefinition) + assert isinstance(e1234, MessageDefinition) + assert w1234 == w0001 + assert w1234 == old_symbol with pytest.raises(UnknownMessageError): store.get_message_definitions("YB12") |