diff --git a/ChangeLog b/ChangeLog index d3cf54d3bf..3b61f41eb9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -233,6 +233,12 @@ Release date: TBA * Allow a `.` as a prefix for Sphinx name resolution. +* Checkers must now keep a 1 to 1 relationship between "msgid" (ie: C1234) and "symbol" (ie : human-readable-symbol) +* In checkers, an old_names can now be used for multiple new messages + +Caused by #1164. It means if you do a partial old_names for a message definition an exception will tell you that you +must rename the associated identification. + What's New in Pylint 2.3.0? =========================== diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 8dac6e1d0a..15310cca26 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -2131,7 +2131,7 @@ class ComparisonChecker(_BasicChecker): "Python is to use isinstance(x, Y) rather than " "type(x) == Y, type(x) is Y. Though there are unusual " "situations where these give different results.", - {"old_names": [("W0154", "unidiomatic-typecheck")]}, + {"old_names": [("W0154", "old-unidiomatic-typecheck")]}, ), "R0123": ( "Comparison to literal", diff --git a/pylint/checkers/base_checker.py b/pylint/checkers/base_checker.py index 8229aab056..f2ae4e5306 100644 --- a/pylint/checkers/base_checker.py +++ b/pylint/checkers/base_checker.py @@ -54,8 +54,8 @@ def __gt__(self, other): def __repr__(self): status = "Checker" if self.enabled else "Disabled checker" - return "{} '{}' responsible for {}".format( - status, self.name, ", ".join(self.msgs.keys()) + return "{} '{}' (responsible for '{}')".format( + status, self.name, "', '".join(self.msgs.keys()) ) def __str__(self): diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index ab9f0c8d06..80c8f1a1f3 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -1680,8 +1680,8 @@ class SpecialMethodsChecker(BaseChecker): "iterable (i.e. has no `%s` method)" % NEXT_METHOD, { "old_names": [ - ("W0234", "non-iterator-returned"), - ("E0234", "non-iterator-returned"), + ("W0234", "old-non-iterator-returned-1"), + ("E0234", "old-non-iterator-returned-2"), ] }, ), diff --git a/pylint/checkers/imports.py b/pylint/checkers/imports.py index fcb84c4c89..e6f3a8e8d9 100644 --- a/pylint/checkers/imports.py +++ b/pylint/checkers/imports.py @@ -201,7 +201,7 @@ def _make_graph(filename, dep_info, sect, gtype): "Unable to import %s", "import-error", "Used when pylint has been unable to import a module.", - {"old_names": [("F0401", "import-error")]}, + {"old_names": [("F0401", "old-import-error")]}, ), "E0402": ( "Attempted relative import beyond top-level package", diff --git a/pylint/checkers/python3.py b/pylint/checkers/python3.py index 69776fbcaf..e6bd0f38e7 100644 --- a/pylint/checkers/python3.py +++ b/pylint/checkers/python3.py @@ -189,7 +189,7 @@ class Python3Checker(checkers.BaseChecker): "Python3 will not allow implicit unpacking of " "exceptions in except clauses. " "See http://www.python.org/dev/peps/pep-3110/", - {"old_names": [("W0712", "unpacking-in-except")]}, + {"old_names": [("W0712", "old-unpacking-in-except")]}, ), "E1604": ( "Use raise ErrorClass(args) instead of raise ErrorClass, args.", @@ -197,14 +197,14 @@ class Python3Checker(checkers.BaseChecker): "Used when the alternate raise syntax " "'raise foo, bar' is used " "instead of 'raise foo(bar)'.", - {"old_names": [("W0121", "old-raise-syntax")]}, + {"old_names": [("W0121", "old-old-raise-syntax")]}, ), "E1605": ( "Use of the `` operator", "backtick", 'Used when the deprecated "``" (backtick) operator is used ' "instead of the str() function.", - {"scope": WarningScope.NODE, "old_names": [("W0333", "backtick")]}, + {"scope": WarningScope.NODE, "old_names": [("W0333", "old-backtick")]}, ), "E1609": ( "Import * only allowed at module level", @@ -358,14 +358,14 @@ class Python3Checker(checkers.BaseChecker): "indexing-exception", "Indexing exceptions will not work on Python 3. Use " "`exception.args[index]` instead.", - {"old_names": [("W0713", "indexing-exception")]}, + {"old_names": [("W0713", "old-indexing-exception")]}, ), "W1625": ( "Raising a string exception", "raising-string", "Used when a string exception is raised. This will not " "work on Python 3.", - {"old_names": [("W0701", "raising-string")]}, + {"old_names": [("W0701", "old-raising-string")]}, ), "W1626": ( "reload built-in referenced", @@ -1369,7 +1369,7 @@ class Python3TokenChecker(checkers.BaseTokenChecker): "old-ne-operator", 'Used when the deprecated "<>" operator is used instead ' 'of "!=". This is removed in Python 3.', - {"maxversion": (3, 0), "old_names": [("W0331", "old-ne-operator")]}, + {"maxversion": (3, 0), "old_names": [("W0331", "old-old-ne-operator")]}, ), "E1608": ( "Use of old octal literal", diff --git a/pylint/checkers/refactoring.py b/pylint/checkers/refactoring.py index ae91c1110c..6291e91260 100644 --- a/pylint/checkers/refactoring.py +++ b/pylint/checkers/refactoring.py @@ -141,13 +141,13 @@ class RefactoringChecker(checkers.BaseTokenChecker): "Used when a function or a method has too many nested " "blocks. This makes the code less understandable and " "maintainable.", - {"old_names": [("R0101", "too-many-nested-blocks")]}, + {"old_names": [("R0101", "old-too-many-nested-blocks")]}, ), "R1703": ( "The if statement can be replaced with %s", "simplifiable-if-statement", "Used when an if statement can be replaced with 'bool(test)'. ", - {"old_names": [("R0102", "simplifiable-if-statement")]}, + {"old_names": [("R0102", "old-simplifiable-if-statement")]}, ), "R1704": ( "Redefining argument with the local name %r", diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index e32d92fd08..b12f761dad 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -289,7 +289,7 @@ def _missing_member_hint(owner, attrname, distance_threshold, max_choices): "assignment-from-none", "Used when an assignment is done on a function call but the " "inferred function returns nothing but None.", - {"old_names": [("W1111", "assignment-from-none")]}, + {"old_names": [("W1111", "old-assignment-from-none")]}, ), "E1129": ( "Context manager '%s' doesn't implement __enter__ and __exit__.", diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index fc5164977f..6cf86d457c 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -450,14 +450,14 @@ def _has_locals_call_after_node(stmt, scope): "left side has %d label(s), right side has %d value(s)", "unbalanced-tuple-unpacking", "Used when there is an unbalanced tuple unpacking in assignment", - {"old_names": [("E0632", "unbalanced-tuple-unpacking")]}, + {"old_names": [("E0632", "old-unbalanced-tuple-unpacking")]}, ), "E0633": ( "Attempting to unpack a non-sequence%s", "unpacking-non-sequence", "Used when something which is not " "a sequence is used in an unpack assignment", - {"old_names": [("W0633", "unpacking-non-sequence")]}, + {"old_names": [("W0633", "old-unpacking-non-sequence")]}, ), "W0640": ( "Cell variable %s defined in loop", diff --git a/pylint/extensions/docparams.py b/pylint/extensions/docparams.py index 939e2a3fa5..6336d194ba 100644 --- a/pylint/extensions/docparams.py +++ b/pylint/extensions/docparams.py @@ -78,7 +78,7 @@ class DocstringParameterChecker(BaseChecker): "Missing return documentation", "missing-return-doc", "Please add documentation about what this method returns.", - {"old_names": [("W9007", "missing-returns-doc")]}, + {"old_names": [("W9007", "old-missing-returns-doc")]}, ), "W9012": ( "Missing return type documentation", @@ -91,7 +91,7 @@ class DocstringParameterChecker(BaseChecker): "Missing yield documentation", "missing-yield-doc", "Please add documentation about what this generator yields.", - {"old_names": [("W9009", "missing-yields-doc")]}, + {"old_names": [("W9009", "old-missing-yields-doc")]}, ), "W9014": ( "Missing yield type documentation", @@ -104,13 +104,13 @@ class DocstringParameterChecker(BaseChecker): '"%s" missing in parameter documentation', "missing-param-doc", "Please add parameter declarations for all parameters.", - {"old_names": [("W9003", "missing-param-doc")]}, + {"old_names": [("W9003", "old-missing-param-doc")]}, ), "W9016": ( '"%s" missing in parameter type documentation', "missing-type-doc", "Please add parameter type declarations for all parameters.", - {"old_names": [("W9004", "missing-type-doc")]}, + {"old_names": [("W9004", "old-missing-type-doc")]}, ), "W9017": ( '"%s" differing in parameter documentation', diff --git a/pylint/message/__init__.py b/pylint/message/__init__.py index d552a61a29..5ac84118a2 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_store import MessageIdStore __all__ = [ "Message", "MessageDefinition", "MessageDefinitionStore", "MessagesHandlerMixIn", + "MessageIdStore", ] diff --git a/pylint/message/message_definition.py b/pylint/message/message_definition.py index cd33cd5752..e54c15aaec 100644 --- a/pylint/message/message_definition.py +++ b/pylint/message/message_definition.py @@ -24,10 +24,7 @@ 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.check_msgid(msgid) self.msgid = msgid self.symbol = symbol self.msg = msg @@ -35,7 +32,18 @@ def __init__( self.scope = scope self.minversion = minversion self.maxversion = maxversion - self.old_names = old_names or [] + self.old_names = [] + if old_names: + for old_msgid, old_symbol in old_names: + self.check_msgid(old_msgid) + self.old_names.append([old_msgid, old_symbol]) + + @staticmethod + def check_msgid(msgid: str) -> None: + 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)) def __repr__(self): return "MessageDefinition:%s (%s)" % (self.symbol, self.msgid) diff --git a/pylint/message/message_definition_store.py b/pylint/message/message_definition_store.py index fdd1f0b548..4d40c25e74 100644 --- a/pylint/message/message_definition_store.py +++ b/pylint/message/message_definition_store.py @@ -7,27 +7,27 @@ 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 @@ def messages(self) -> list: 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 @@ def register_message(self, message): :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 @@ def get_message_definitions(self, msgid_or_symbol: str) -> list: :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 0750c9ee68..ad8a11f456 100644 --- a/pylint/message/message_handler_mix_in.py +++ b/pylint/message/message_handler_mix_in.py @@ -105,10 +105,9 @@ def _set_msg_status( # 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/pylint/message/message_id_store.py b/pylint/message/message_id_store.py new file mode 100644 index 0000000000..b8f0206c96 --- /dev/null +++ b/pylint/message/message_id_store.py @@ -0,0 +1,123 @@ +# -*- 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 typing import List + +from pylint.exceptions import InvalidMessageError, UnknownMessageError + + +class MessageIdStore: + + """The MessageIdStore store MessageId and make sure that there is a 1-1 relation between msgid and symbol.""" + + def __init__(self): + self.__msgid_to_symbol = {} + self.__symbol_to_msgid = {} + self.__old_names = {} + + def __len__(self): + return len(self.__msgid_to_symbol) + + def __repr__(self): + result = "MessageIdStore: [\n" + for msgid, symbol in self.__msgid_to_symbol.items(): + result += " - {msgid} ({symbol})\n".format(msgid=msgid, symbol=symbol) + result += "]" + return result + + def get_symbol(self, msgid: str) -> str: + return self.__msgid_to_symbol[msgid] + + def get_msgid(self, symbol: str) -> str: + return self.__symbol_to_msgid[symbol] + + def register_message_definition(self, message_definition): + self.check_msgid_and_symbol(message_definition.msgid, message_definition.symbol) + self.add_msgid_and_symbol(message_definition.msgid, message_definition.symbol) + for old_msgid, old_symbol in message_definition.old_names: + self.check_msgid_and_symbol(old_msgid, old_symbol) + self.add_legacy_msgid_and_symbol( + old_msgid, old_symbol, message_definition.msgid + ) + + def add_msgid_and_symbol(self, msgid: str, symbol: str) -> None: + """Add valid message id. + + There is a little duplication with add_legacy_msgid_and_symbol to avoid a function call, + this is called a lot at initialization.""" + self.__msgid_to_symbol[msgid] = symbol + self.__symbol_to_msgid[symbol] = msgid + + def add_legacy_msgid_and_symbol(self, msgid: str, symbol: str, new_msgid: str): + """Add valid legacy message id. + + There is a little duplication with add_msgid_and_symbol to avoid a function call, + this is called a lot at initialization.""" + self.__msgid_to_symbol[msgid] = symbol + self.__symbol_to_msgid[symbol] = msgid + existing_old_names = self.__old_names.get(msgid, []) + existing_old_names.append(new_msgid) + self.__old_names[msgid] = existing_old_names + + def check_msgid_and_symbol(self, msgid: str, symbol: str) -> None: + existing_msgid = self.__symbol_to_msgid.get(symbol) + existing_symbol = self.__msgid_to_symbol.get(msgid) + if existing_symbol is None and existing_msgid is None: + return + if existing_msgid is not None: + if existing_msgid != msgid: + self._raise_duplicate_msgid(symbol, msgid, existing_msgid) + if existing_symbol != symbol: + self._raise_duplicate_symbol(msgid, symbol, existing_symbol) + + @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 " + "'{other_msgid}' and '{msgid}' at the same time." + " If you're creating an 'old_names' use 'old-{symbol}' as the old symbol." + ).format(symbol=symbol, other_msgid=msgids[0], msgid=msgids[1]) + raise InvalidMessageError(error_message) + + def get_active_msgids(self, msgid_or_symbol: str) -> List[str]: + """Return msgids but the input can be a symbol.""" + # Only msgid can have a digit as second letter + is_msgid = msgid_or_symbol[1:].isdigit() + if is_msgid: + msgid = msgid_or_symbol.upper() + symbol = self.__msgid_to_symbol.get(msgid) + else: + msgid = self.__symbol_to_msgid.get(msgid_or_symbol) + symbol = msgid_or_symbol + if not msgid or not symbol: + error_msg = "No such message id or symbol '{msgid_or_symbol}'.".format( + msgid_or_symbol=msgid_or_symbol + ) + raise UnknownMessageError(error_msg) + return [msgid] diff --git a/tests/message/__init__.py b/tests/message/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/message/generic_fixtures.py b/tests/message/generic_fixtures.py new file mode 100644 index 0000000000..bb12b02abb --- /dev/null +++ b/tests/message/generic_fixtures.py @@ -0,0 +1,77 @@ +# -*- 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.checkers import BaseChecker +from pylint.message import MessageDefinitionStore, MessageIdStore + + +@pytest.fixture +def msgid(): + return "W1234" + + +@pytest.fixture +def symbol(): + return "msg-symbol" + + +@pytest.fixture +def empty_store(): + return MessageDefinitionStore() + + +@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 + + +@pytest.fixture +def message_definitions(store): + return store.messages + + +@pytest.fixture +def msgids(): + return { + "W1234": "warning-symbol", + "W1235": "warning-symbol-two", + "C1234": "convention-symbol", + "E1234": "error-symbol", + } + + +@pytest.fixture +def empty_msgid_store(): + return MessageIdStore() + + +@pytest.fixture +def msgid_store(msgids): + msgid_store = MessageIdStore() + for msgid, symbol in msgids.items(): + msgid_store.add_msgid_and_symbol(msgid, symbol) + return msgid_store diff --git a/tests/message/unittest_message.py b/tests/message/unittest_message.py index 3bc1578b5c..205d06dec4 100644 --- a/tests/message/unittest_message.py +++ b/tests/message/unittest_message.py @@ -3,154 +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)" ) - - -@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 + 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.py b/tests/message/unittest_message_definition.py index 0b9b059079..6bf7c8b53c 100644 --- a/tests/message/unittest_message_definition.py +++ b/tests/message/unittest_message_definition.py @@ -5,11 +5,30 @@ import sys +import pytest + from pylint.checkers import BaseChecker from pylint.constants import WarningScope +from pylint.exceptions import InvalidMessageError from pylint.message import MessageDefinition +@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 invalid_message_error: + MessageDefinition.check_msgid(msgid) + with pytest.raises(InvalidMessageError) as other_invalid_message_error: + MessageDefinition("checker", msgid, "msg", "descr", "symbol", "scope") + assert str(invalid_message_error.value) == expected + assert str(other_invalid_message_error.value) == expected + + class FalseChecker(BaseChecker): name = "FalseChecker" msgs = { @@ -70,6 +89,15 @@ def test_repr(self): expected = "[MessageDefinition:msg-symbol-one (W1234), MessageDefinition:msg-symbol-two (W1235)]" assert str(FalseChecker().messages) == expected + def test_str(self): + msg = self.get_message_definition() + str_msg = str(msg) + assert "W1234" in str_msg + assert "msg-symbol" in str_msg + expected = """MessageDefinition:msg-symbol-one (W1234): +message one msg description""" + assert str(FalseChecker().messages[0]) == expected + def test_format_help(self): msg = self.get_message_definition() major = sys.version_info.major diff --git a/tests/message/unittest_message_definition_store.py b/tests/message/unittest_message_definition_store.py index c656781fe0..3a4ebf3496 100644 --- a/tests/message/unittest_message_definition_store.py +++ b/tests/message/unittest_message_definition_store.py @@ -10,32 +10,141 @@ 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 @@ def _compare_messages(self, desc, msg, checkerref=False): 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") diff --git a/tests/message/unittest_message_id_store.py b/tests/message/unittest_message_id_store.py new file mode 100644 index 0000000000..00fbd70280 --- /dev/null +++ b/tests/message/unittest_message_id_store.py @@ -0,0 +1,72 @@ +# -*- 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, UnknownMessageError +from pylint.message import MessageIdStore + +from .generic_fixtures import ( + empty_msgid_store, + message_definitions, + msgid_store, + msgids, + store, +) + + +def test_len_str(msgid_store, msgids): + assert len(msgid_store) == len(msgids) + str_result = str(msgid_store) + assert "MessageIdStore: [" in str_result + assert " - W1234 (warning-symbol)" in str_result + assert " - W1235 (warning-symbol-two)" in str_result + assert " - C1234 (convention-symbol)" in str_result + assert " - E1234 (error-symbol)" in str_result + assert "]" in str_result + + +def test_get_message_ids(msgid_store, msgids): + """We can get message id even with capitalization problem.""" + msgid = list(msgids.keys())[0] + msgids_result = msgid_store.get_active_msgids(msgid.lower()) + assert len(msgids_result) == 1 + assert msgid == msgids_result[0] + + +def test_get_message_ids_not_existing(empty_msgid_store): + with pytest.raises(UnknownMessageError) as error: + w9876 = "W9876" + empty_msgid_store.get_active_msgids(w9876) + assert w9876 in str(error.value) + + +def test_register_message_definitions(empty_msgid_store, message_definitions): + number_of_msgid = len(message_definitions) + for i, message_definition in enumerate(message_definitions): + empty_msgid_store.register_message_definition(message_definition) + if message_definition.old_names: + number_of_msgid += len(message_definition.old_names) + assert len(empty_msgid_store) == number_of_msgid + + +def test_duplicate_symbol(empty_msgid_store): + empty_msgid_store.add_msgid_and_symbol("W1234", "warning-symbol") + with pytest.raises(InvalidMessageError) as error: + empty_msgid_store.check_msgid_and_symbol("W1234", "other-symbol") + assert ( + "Message id 'W1234' cannot have both 'other-symbol' and 'warning-symbol' as symbolic name." + in str(error.value) + ) + + +def test_duplicate_msgid(msgid_store): + msgid_store.add_msgid_and_symbol("W1234", "warning-symbol") + with pytest.raises(InvalidMessageError) as error: + msgid_store.check_msgid_and_symbol("W1235", "warning-symbol") + assert ( + "Message symbol 'warning-symbol' cannot be used for 'W1234' and 'W1235'" + in str(error.value) + ) diff --git a/tests/unittest_checker_base.py b/tests/unittest_checker_base.py index b50fbf1a65..203a05990a 100644 --- a/tests/unittest_checker_base.py +++ b/tests/unittest_checker_base.py @@ -579,7 +579,7 @@ class LessBasicChecker(OtherBasicChecker): """ self.assertEqual(str(basic), expected_beginning + expected_end) - self.assertEqual(repr(basic), "Checker 'basic' responsible for W0001") + self.assertEqual(repr(basic), "Checker 'basic' (responsible for 'W0001')") less_basic = LessBasicChecker() self.assertEqual(