From c54845c3ae728512e3920519abbd1f9fc1c3d484 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 10 Aug 2019 13:16:20 +0200 Subject: [PATCH 1/6] [tests.message] Add a test for multiple messages with the same old name --- pylint/message/message_id_store.py | 7 +++- .../unittest_message_definition_store.py | 33 +++++++++++++++++++ tests/message/unittest_message_id_store.py | 20 +++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/pylint/message/message_id_store.py b/pylint/message/message_id_store.py index b8f0206c96..756888a4bd 100644 --- a/pylint/message/message_id_store.py +++ b/pylint/message/message_id_store.py @@ -120,4 +120,9 @@ def get_active_msgids(self, msgid_or_symbol: str) -> List[str]: msgid_or_symbol=msgid_or_symbol ) raise UnknownMessageError(error_msg) - return [msgid] + # logging.debug( + # "Return for {} and msgid {} is {}".format( + # msgid_or_symbol, msgid, self.__old_names.get(msgid, [msgid]) + # ) + # ) + return self.__old_names.get(msgid, [msgid]) diff --git a/tests/message/unittest_message_definition_store.py b/tests/message/unittest_message_definition_store.py index 3a4ebf3496..0af2c9b30e 100644 --- a/tests/message/unittest_message_definition_store.py +++ b/tests/message/unittest_message_definition_store.py @@ -234,3 +234,36 @@ def test_list_messages(self, store): def test_renamed_message_register(self, store): assert "msg-symbol" == store.get_message_definitions("W0001")[0].symbol assert "msg-symbol" == store.get_message_definitions("old-symbol")[0].symbol + + +def test_multiple_child_of_old_name(store): + """ We can define multiple name with the same old name. """ + + class FamillyChecker(BaseChecker): + name = "famillychecker" + msgs = { + "W1235": ( + "Child 1", + "child-one", + "Child one description.", + {"old_names": [("C1234", "mother")]}, + ), + "W1236": ( + "Child 2", + "child-two", + "Child two description", + {"old_names": [("C1234", "mother")]}, + ), + } + + store.register_messages_from_checker(FamillyChecker()) + mother = store.get_message_definitions("C1234") + child = store.get_message_definitions("W1235") + other_child = store.get_message_definitions("W1236") + assert len(mother) == 2 + assert len(child) == 1 + assert len(other_child) == 1 + child = child[0] + other_child = other_child[0] + assert child in mother + assert other_child in mother diff --git a/tests/message/unittest_message_id_store.py b/tests/message/unittest_message_id_store.py index 00fbd70280..dc2723b342 100644 --- a/tests/message/unittest_message_id_store.py +++ b/tests/message/unittest_message_id_store.py @@ -52,6 +52,26 @@ def test_register_message_definitions(empty_msgid_store, message_definitions): assert len(empty_msgid_store) == number_of_msgid +def test_add_msgid_and_symbol(empty_msgid_store): + empty_msgid_store.add_msgid_and_symbol("E1235", "new-sckiil") + empty_msgid_store.add_legacy_msgid_and_symbol("C1235", "old-sckiil", "E1235") + assert len(empty_msgid_store) == 2 + message_ids = empty_msgid_store.get_active_msgids("E1235") + assert len(message_ids) == 1 + assert message_ids[0] == "E1235" + message_ids = empty_msgid_store.get_active_msgids("old-sckiil") + assert len(message_ids) == 1 + assert message_ids[0] == "E1235" + assert empty_msgid_store.get_symbol("C1235") == "old-sckiil" + assert empty_msgid_store.get_symbol("E1235") == "new-sckiil" + assert empty_msgid_store.get_msgid("old-sckiil") == "C1235" + assert empty_msgid_store.get_msgid("new-sckiil") == "E1235" + with pytest.raises(KeyError) as e: + empty_msgid_store.get_symbol("C1234") + with pytest.raises(KeyError) as e: + empty_msgid_store.get_msgid("not-exist") + + def test_duplicate_symbol(empty_msgid_store): empty_msgid_store.add_msgid_and_symbol("W1234", "warning-symbol") with pytest.raises(InvalidMessageError) as error: From cd6c168e6c29e4e505efbe741c561fac5cfb53e7 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 4 Aug 2019 21:23:16 +0200 Subject: [PATCH 2/6] [test/functional] Better error message for fail --- tests/test_functional.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_functional.py b/tests/test_functional.py index 9ff9e77632..95f52f55df 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -347,7 +347,7 @@ def _split_lines(self, expected_messages, lines): def _check_output_text(self, expected_messages, expected_lines, received_lines): assert ( self._split_lines(expected_messages, expected_lines)[0] == received_lines - ), self._test_file.base + ), "Error with the following functional test: {}".format(self._test_file.base) class LintModuleOutputUpdate(LintModuleTest): From 17f345cae61a38cba489ba87ab019addb10ceb3b Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 10 Aug 2019 18:28:36 +0200 Subject: [PATCH 3/6] [pylint.message] Simplify get_message_definitions --- pylint/message/message_definition_store.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pylint/message/message_definition_store.py b/pylint/message/message_definition_store.py index 4d40c25e74..c233979a6c 100644 --- a/pylint/message/message_definition_store.py +++ b/pylint/message/message_definition_store.py @@ -63,14 +63,10 @@ def get_message_definitions(self, msgid_or_symbol: str) -> list: :rtype: List of MessageDefinition :return: A message definition corresponding to msgid_or_symbol """ - 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 + return [ + self._messages_definitions[m] + for m in self.message_id_store.get_active_msgids(msgid_or_symbol) + ] def get_msg_display_string(self, msgid_or_symbol: str): """Generates a user-consumable representation of a message. """ From 8c53606999674a336e8e68438daf771d7565a421 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 10 Aug 2019 18:55:45 +0200 Subject: [PATCH 4/6] [pylint.message] Optimize MessageDefinitionStore storage --- pylint/message/message_definition_store.py | 11 ++++------- pylint/message/message_handler_mix_in.py | 3 ++- pylint/message/message_id_store.py | 4 ++++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pylint/message/message_definition_store.py b/pylint/message/message_definition_store.py index c233979a6c..1439b032c3 100644 --- a/pylint/message/message_definition_store.py +++ b/pylint/message/message_definition_store.py @@ -23,10 +23,6 @@ def __init__(self): # It contains the 1:1 mapping from msgid to MessageDefinition. # Keys are msgid, values are MessageDefinition self._messages_definitions = {} - # 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) @@ -51,11 +47,12 @@ def register_message(self, message): """ 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) + @property + def msgids(self): + return self.message_id_store.msgids + 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. diff --git a/pylint/message/message_handler_mix_in.py b/pylint/message/message_handler_mix_in.py index ad8a11f456..5a7482951d 100644 --- a/pylint/message/message_handler_mix_in.py +++ b/pylint/message/message_handler_mix_in.py @@ -106,8 +106,9 @@ def _set_msg_status( # msgid is a checker name? if msgid.lower() in self._checkers: for checker in self._checkers[msgid.lower()]: + msgids = self.msgs_store.msgids for _msgid in checker.msgs: - if _msgid in self.msgs_store._old_message_definitions: + if _msgid in msgids: 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 index 756888a4bd..2cf970aecc 100644 --- a/pylint/message/message_id_store.py +++ b/pylint/message/message_id_store.py @@ -33,6 +33,10 @@ def get_symbol(self, msgid: str) -> str: def get_msgid(self, symbol: str) -> str: return self.__symbol_to_msgid[symbol] + @property + def msgids(self): + return list(self.__msgid_to_symbol.keys()) + list(self.__old_names.keys()) + 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) From 2a266ccd3dbdf0d4d3a62478aaa6aa3aee7f5247 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 10 Aug 2019 19:00:47 +0200 Subject: [PATCH 5/6] [pylint.message] Remove a check that never fail It seems to make performance 2/3 % better during test and during pylint own analysis. --- pylint/message/message_definition_store.py | 4 ---- pylint/message/message_handler_mix_in.py | 4 +--- pylint/message/message_id_store.py | 4 ---- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/pylint/message/message_definition_store.py b/pylint/message/message_definition_store.py index 1439b032c3..ef0d8e2ee7 100644 --- a/pylint/message/message_definition_store.py +++ b/pylint/message/message_definition_store.py @@ -49,10 +49,6 @@ def register_message(self, message): self._messages_definitions[message.msgid] = message self._msgs_by_category[message.msgid[0]].append(message.msgid) - @property - def msgids(self): - return self.message_id_store.msgids - 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. diff --git a/pylint/message/message_handler_mix_in.py b/pylint/message/message_handler_mix_in.py index 5a7482951d..ad2bda045c 100644 --- a/pylint/message/message_handler_mix_in.py +++ b/pylint/message/message_handler_mix_in.py @@ -106,10 +106,8 @@ def _set_msg_status( # msgid is a checker name? if msgid.lower() in self._checkers: for checker in self._checkers[msgid.lower()]: - msgids = self.msgs_store.msgids for _msgid in checker.msgs: - if _msgid in msgids: - self._set_msg_status(_msgid, enable, scope, line) + self._set_msg_status(_msgid, enable, scope, line) return # msgid is report id? diff --git a/pylint/message/message_id_store.py b/pylint/message/message_id_store.py index 2cf970aecc..756888a4bd 100644 --- a/pylint/message/message_id_store.py +++ b/pylint/message/message_id_store.py @@ -33,10 +33,6 @@ def get_symbol(self, msgid: str) -> str: def get_msgid(self, symbol: str) -> str: return self.__symbol_to_msgid[symbol] - @property - def msgids(self): - return list(self.__msgid_to_symbol.keys()) + list(self.__old_names.keys()) - 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) From 877a68e82a5fd2a6094a47152338f0046c5b274f Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Mon, 19 Aug 2019 18:20:24 +0200 Subject: [PATCH 6/6] [changelog] Add 'multiple message with the same old name' --- CONTRIBUTORS.txt | 2 ++ ChangeLog | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index cf20df5ace..712c9351e9 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -288,6 +288,8 @@ contributors: * Pierre Sassoulas : contributor - Made C0412 (ungrouped import) compatible with isort + - Made multiple message with the same old name possible + - Made Pylint a little faster by refactoring the message store * Nathan Marrow diff --git a/ChangeLog b/ChangeLog index 3b61f41eb9..4d853f96d0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -234,7 +234,7 @@ 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 +* In checkers, an old_names can now be used for multiple new messages and pylint is now a little faster 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.