Fix false negative 'use-symbolic-message-instead' and optimize it

use-symbolic-message-instead was not working for message with
multiple new names and the function to get the symbol was
suboptimal. Also made the solution copy pastable.

22 files changed, 79 insertions, 52 deletions

diff --git a/ChangeLog b/ChangeLog
index 7e8ea36fa..a23865228 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -17,6 +17,8 @@ Release date: TBA
 ..
   Put bug fixes that will be cherry-picked to latest major version here
 
+* ``use-symbolic-message-instead`` now also works on legacy messages like ``C0111`` (``missing-docstring``).
+
 * Remove unwanted print to stdout from ``_emit_no_member``
 
 * Introduce a command-line option to specify pyreverse output directory
diff --git a/pylint/checkers/misc.py b/pylint/checkers/misc.py
index 2e460649f..9b75fd43b 100644
--- a/pylint/checkers/misc.py
+++ b/pylint/checkers/misc.py
@@ -34,11 +34,10 @@ from pylint.utils.pragma_parser import OPTION_PO, PragmaParserError, parse_pragm
 
 class ByIdManagedMessagesChecker(BaseChecker):
 
-    """checks for messages that are enabled or disabled by id instead of symbol."""
+    """Checks for messages that are enabled or disabled by id instead of symbol."""
 
     __implements__ = IRawChecker
 
-    # configuration section name
     name = "miscellaneous"
     msgs = {
         "I0023": (
@@ -51,12 +50,12 @@ class ByIdManagedMessagesChecker(BaseChecker):
     options = ()
 
     def process_module(self, module):
-        """inspect the source file to find messages activated or deactivated by id."""
+        """Inspect the source file to find messages activated or deactivated by id."""
         managed_msgs = MessagesHandlerMixIn.get_by_id_managed_msgs()
-        for (mod_name, msg_id, msg_symbol, lineno, is_disabled) in managed_msgs:
+        for (mod_name, msgid, symbol, lineno, is_disabled) in managed_msgs:
             if mod_name == module.name:
                 verb = "disable" if is_disabled else "enable"
-                txt = f"Id '{msg_id}' is used to {verb} '{msg_symbol}' message emission"
+                txt = f"'{msgid}' is cryptic: use '# pylint: {verb}={symbol}' instead"
                 self.add_message("use-symbolic-message-instead", line=lineno, args=txt)
         MessagesHandlerMixIn.clear_by_id_managed_msgs()
 
diff --git a/pylint/message/message_handler_mix_in.py b/pylint/message/message_handler_mix_in.py
index f9cb762e5..8667c84f8 100644
--- a/pylint/message/message_handler_mix_in.py
+++ b/pylint/message/message_handler_mix_in.py
@@ -2,6 +2,7 @@
 # For details: https://github.com/PyCQA/pylint/blob/master/COPYING
 
 import sys
+from typing import List, Tuple
 
 from pylint.constants import (
     _SCOPE_EXEMPT,
@@ -21,11 +22,9 @@ from pylint.utils import get_module_and_frameid, get_rst_section, get_rst_title
 
 
 class MessagesHandlerMixIn:
-    """a mix-in class containing all the messages related methods for the main
-    lint class
-    """
+    """A mix-in class containing all the messages related methods for the main lint class."""
 
-    __by_id_managed_msgs = []  # type: ignore
+    __by_id_managed_msgs: List[Tuple[str, str, str, int, bool]] = []
 
     def __init__(self):
         self._msgs_state = {}
@@ -43,27 +42,16 @@ class MessagesHandlerMixIn:
     def get_by_id_managed_msgs(cls):
         return cls.__by_id_managed_msgs
 
-    def _register_by_id_managed_msg(self, msgid, line, is_disabled=True):
+    def _register_by_id_managed_msg(self, msgid_or_symbol: str, line, is_disabled=True):
         """If the msgid is a numeric one, then register it to inform the user
         it could furnish instead a symbolic msgid."""
-        try:
-            message_definitions = self.msgs_store.get_message_definitions(msgid)
-            for message_definition in message_definitions:
-                if msgid == message_definition.msgid:
-                    MessagesHandlerMixIn.__by_id_managed_msgs.append(
-                        (
-                            self.current_name,
-                            message_definition.msgid,
-                            message_definition.symbol,
-                            line,
-                            is_disabled,
-                        )
-                    )
-        except UnknownMessageError:
-            pass
+        if msgid_or_symbol[1:].isdigit():
+            symbol = self.msgs_store.message_id_store.get_symbol(msgid=msgid_or_symbol)  # type: ignore
+            managed = (self.current_name, msgid_or_symbol, symbol, line, is_disabled)  # type: ignore
+            MessagesHandlerMixIn.__by_id_managed_msgs.append(managed)
 
     def disable(self, msgid, scope="package", line=None, ignore_unknown=False):
-        """don't output message of the given id"""
+        """Don't output message of the given id"""
         self._set_msg_status(
             msgid, enable=False, scope=scope, line=line, ignore_unknown=ignore_unknown
         )
@@ -195,7 +183,7 @@ class MessagesHandlerMixIn:
         except KeyError:
             # Check if the message's line is after the maximum line existing in ast tree.
             # This line won't appear in the ast tree and won't be referred in
-            #  self.file_state._module_msgs_state
+            # self.file_state._module_msgs_state
             # This happens for example with a commented line at the end of a module.
             max_line_number = self.file_state.get_effective_max_line_number()
             if max_line_number and line > max_line_number:
diff --git a/tests/functional/a/access/access_attr_before_def_false_positive.txt b/tests/functional/a/access/access_attr_before_def_false_positive.txt
new file mode 100644
index 000000000..c3bcdf8d8
--- /dev/null
+++ b/tests/functional/a/access/access_attr_before_def_false_positive.txt
@@ -0,0 +1,3 @@
+use-symbolic-message-instead:1:0::"'C0103' is cryptic: use '# pylint: disable=invalid-name' instead"
+use-symbolic-message-instead:1:0::"'R0904' is cryptic: use '# pylint: disable=too-many-public-methods' instead"
+use-symbolic-message-instead:1:0::"'W0201' is cryptic: use '# pylint: disable=attribute-defined-outside-init' instead"
diff --git a/tests/functional/b/base_init_vars.txt b/tests/functional/b/base_init_vars.txt
index 75c59d324..99d501ae9 100644
--- a/tests/functional/b/base_init_vars.txt
+++ b/tests/functional/b/base_init_vars.txt
@@ -1 +1 @@
-use-symbolic-message-instead:1:0::Id 'no-self-use' is used to disable 'no-self-use' message emission
+use-symbolic-message-instead:1:0::"'R0201' is cryptic: use '# pylint: disable=no-self-use' instead"
diff --git a/tests/functional/c/classes_meth_could_be_a_function.py b/tests/functional/c/classes_meth_could_be_a_function.py
index 2a0976d1a..59e585649 100644
--- a/tests/functional/c/classes_meth_could_be_a_function.py
+++ b/tests/functional/c/classes_meth_could_be_a_function.py
@@ -1,4 +1,4 @@
-# pylint: disable=C0111,too-few-public-methods,no-init, useless-object-inheritance
+# pylint: disable=C0111,too-few-public-methods,W0232, useless-object-inheritance  # [use-symbolic-message-instead,use-symbolic-message-instead]
 """
 #2479
 
@@ -18,7 +18,7 @@ class Aimpl(object):
     # disable "method could be a function" on classes which are not overriding
     # the factory method because in that case the usage of polymorphism is not
     # detected
-    # pylint: disable=no-self-use
+    # pylint: disable=R0201  # [use-symbolic-message-instead]
     def makex(self):
         return XAsub()
 
diff --git a/tests/functional/c/classes_meth_could_be_a_function.txt b/tests/functional/c/classes_meth_could_be_a_function.txt
index 3f9a0950b..a8a5ea27f 100644
--- a/tests/functional/c/classes_meth_could_be_a_function.txt
+++ b/tests/functional/c/classes_meth_could_be_a_function.txt
@@ -1 +1,3 @@
-use-symbolic-message-instead:1:0::Id 'W0232' is used to disable 'no-init' message emission
+use-symbolic-message-instead:1:0::"'C0111' is cryptic: use '# pylint: disable=missing-docstring' instead"
+use-symbolic-message-instead:1:0::"'W0232' is cryptic: use '# pylint: disable=no-init' instead"
+use-symbolic-message-instead:21:0::"'R0201' is cryptic: use '# pylint: disable=no-self-use' instead"
diff --git a/tests/functional/c/ctor_arguments.py b/tests/functional/c/ctor_arguments.py
index ee05126b1..8499e9374 100644
--- a/tests/functional/c/ctor_arguments.py
+++ b/tests/functional/c/ctor_arguments.py
@@ -2,7 +2,7 @@
 
 Based on tests/functional/a/arguments.py
 """
-# pylint: disable=C0111,too-few-public-methods,super-init-not-called, useless-object-inheritance
+# pylint: disable=C0111,too-few-public-methods,W0231, useless-object-inheritance  # [use-symbolic-message-instead,use-symbolic-message-instead]
 
 
 class Class1Arg(object):
diff --git a/tests/functional/c/ctor_arguments.txt b/tests/functional/c/ctor_arguments.txt
index 781715c0a..f08ccdec5 100644
--- a/tests/functional/c/ctor_arguments.txt
+++ b/tests/functional/c/ctor_arguments.txt
@@ -1,3 +1,5 @@
+use-symbolic-message-instead:5:0::"'C0111' is cryptic: use '# pylint: disable=missing-docstring' instead"
+use-symbolic-message-instead:5:0::"'W0231' is cryptic: use '# pylint: disable=super-init-not-called' instead"
 no-value-for-parameter:35:0::No value for argument 'first_argument' in constructor call
 too-many-function-args:36:0::Too many positional arguments for constructor call
 no-value-for-parameter:38:0::No value for argument 'third_argument' in constructor call
diff --git a/tests/functional/d/decorator_scope.txt b/tests/functional/d/decorator_scope.txt
index 3f9a0950b..b89662297 100644
--- a/tests/functional/d/decorator_scope.txt
+++ b/tests/functional/d/decorator_scope.txt
@@ -1 +1 @@
-use-symbolic-message-instead:1:0::Id 'W0232' is used to disable 'no-init' message emission
+use-symbolic-message-instead:1:0::"'W0232' is cryptic: use '# pylint: disable=no-init' instead"
diff --git a/tests/functional/e/external_classmethod_crash.txt b/tests/functional/e/external_classmethod_crash.txt
index 780ef6e6c..e80a3273d 100644
--- a/tests/functional/e/external_classmethod_crash.txt
+++ b/tests/functional/e/external_classmethod_crash.txt
@@ -1,2 +1,2 @@
-use-symbolic-message-instead:1:0::Id 'W0232' is used to disable 'no-init' message emission
-use-symbolic-message-instead:1:0::Id 'W0613' is used to disable 'unused-argument' message emission
+use-symbolic-message-instead:1:0::"'W0232' is cryptic: use '# pylint: disable=no-init' instead"
+use-symbolic-message-instead:1:0::"'W0613' is cryptic: use '# pylint: disable=unused-argument' instead"
diff --git a/tests/functional/g/genexp_in_class_scope.txt b/tests/functional/g/genexp_in_class_scope.txt
index 3f9a0950b..b89662297 100644
--- a/tests/functional/g/genexp_in_class_scope.txt
+++ b/tests/functional/g/genexp_in_class_scope.txt
@@ -1 +1 @@
-use-symbolic-message-instead:1:0::Id 'W0232' is used to disable 'no-init' message emission
+use-symbolic-message-instead:1:0::"'W0232' is cryptic: use '# pylint: disable=no-init' instead"
diff --git a/tests/functional/m/messages_managed_by_id.py b/tests/functional/m/messages_managed_by_id.py
index 12e4bf58e..ddcf0bedc 100644
--- a/tests/functional/m/messages_managed_by_id.py
+++ b/tests/functional/m/messages_managed_by_id.py
@@ -1,11 +1,18 @@
-# -*- encoding=utf-8 -*-
-#pylint: disable=C0111
-def foo(): #pylint: disable=C0102
+"""Use symbolic message instead are also tested in use_symbolic_message_instead.py"""
+# pylint: disable=C0111  # [use-symbolic-message-instead]
+
+
+def foo():  # pylint: disable=C0102  # [use-symbolic-message-instead]
     return 1
 
-def toto(): #pylint: disable=C0102,R1711  # [use-symbolic-message-instead]
+
+def toto():  # pylint: disable=C0102,R1711  # [use-symbolic-message-instead,use-symbolic-message-instead]
     return
 
-# +1: [missing-function-docstring]
-def test_enabled_by_id_msg(): #pylint: enable=C0111
+
+def test_enabled_by_id_msg():  # pylint: enable=C0111  # [use-symbolic-message-instead,missing-function-docstring]
     pass
+
+
+def baz(): #pylint: disable=blacklisted-name
+    return 1
diff --git a/tests/functional/m/messages_managed_by_id.txt b/tests/functional/m/messages_managed_by_id.txt
index 4b93742b0..83ef807ca 100644
--- a/tests/functional/m/messages_managed_by_id.txt
+++ b/tests/functional/m/messages_managed_by_id.txt
@@ -1,2 +1,6 @@
-use-symbolic-message-instead:6:0::Id 'R1711' is used to disable 'useless-return' message emission
-missing-function-docstring:10:0:test_enabled_by_id_msg:Missing function or method docstring
+use-symbolic-message-instead:2:0::"'C0111' is cryptic: use '# pylint: disable=missing-docstring' instead"
+use-symbolic-message-instead:5:0::"'C0102' is cryptic: use '# pylint: disable=blacklisted-name' instead"
+use-symbolic-message-instead:9:0::"'C0102' is cryptic: use '# pylint: disable=blacklisted-name' instead"
+use-symbolic-message-instead:9:0::"'R1711' is cryptic: use '# pylint: disable=useless-return' instead"
+missing-function-docstring:13:0:test_enabled_by_id_msg:Missing function or method docstring
+use-symbolic-message-instead:13:0::"'C0111' is cryptic: use '# pylint: enable=missing-docstring' instead"
diff --git a/tests/functional/u/unused/unused_argument.py b/tests/functional/u/unused/unused_argument.py
index 3c67e89fe..a1018453f 100644
--- a/tests/functional/u/unused/unused_argument.py
+++ b/tests/functional/u/unused/unused_argument.py
@@ -76,7 +76,7 @@ class AAAA(object):
                     description=[(etype,)]*size)
         def inner(row, col=0, etype=etype, req=self, rset=rset):
             """inner using all its argument"""
-            # pylint: disable = E1103
+            # pylint: disable = E1103  # [use-symbolic-message-instead]
             return req.vreg.etype_class(etype)(req, rset, row, col)
         # pylint: disable = attribute-defined-outside-init
         rset.get_entity = inner
diff --git a/tests/functional/u/unused/unused_argument.txt b/tests/functional/u/unused/unused_argument.txt
index b1ad4ad5a..1e59f7cc5 100644
--- a/tests/functional/u/unused/unused_argument.txt
+++ b/tests/functional/u/unused/unused_argument.txt
@@ -5,4 +5,5 @@ unused-argument:54:13:function:Unused argument 'arg'
 unused-argument:61:21:AAAA.method:Unused argument 'arg':INFERENCE
 unused-argument:68:0:AAAA.selected:Unused argument 'args':INFERENCE
 unused-argument:68:0:AAAA.selected:Unused argument 'kwargs':INFERENCE
+use-symbolic-message-instead:79:0::"'E1103' is cryptic: use '# pylint: disable=maybe-no-member' instead"
 unused-argument:87:23:BBBB.__init__:Unused argument 'arg':INFERENCE
diff --git a/tests/functional/u/use/use_symbolic_message_instead.py b/tests/functional/u/use/use_symbolic_message_instead.py
new file mode 100644
index 000000000..c5fea1261
--- /dev/null
+++ b/tests/functional/u/use/use_symbolic_message_instead.py
@@ -0,0 +1,11 @@
+"""Use symbolic message instead are also tested in messages_managed_by_id.py"""
+
+# pylint: disable=C0111,R0903,T1234  # [bad-option-value,use-symbolic-message-instead,use-symbolic-message-instead]
+# pylint: enable=C0111   # [use-symbolic-message-instead]
+
+def myfunction(arg):  # [missing-function-docstring]
+    return arg or True
+
+# pylint: disable=C0111  # [use-symbolic-message-instead]
+# pylint: enable=R0903  # [use-symbolic-message-instead]
+# pylint: disable=R0903  # [use-symbolic-message-instead]
diff --git a/tests/functional/u/use/use_symbolic_message_instead.txt b/tests/functional/u/use/use_symbolic_message_instead.txt
new file mode 100644
index 000000000..855c9d509
--- /dev/null
+++ b/tests/functional/u/use/use_symbolic_message_instead.txt
@@ -0,0 +1,8 @@
+bad-option-value:3:0::Bad option value 'T1234'
+use-symbolic-message-instead:3:0::"'C0111' is cryptic: use '# pylint: disable=missing-docstring' instead"
+use-symbolic-message-instead:3:0::"'R0903' is cryptic: use '# pylint: disable=too-few-public-methods' instead"
+use-symbolic-message-instead:4:0::"'C0111' is cryptic: use '# pylint: enable=missing-docstring' instead"
+missing-function-docstring:6:0:myfunction:Missing function or method docstring
+use-symbolic-message-instead:9:0::"'C0111' is cryptic: use '# pylint: disable=missing-docstring' instead"
+use-symbolic-message-instead:10:0::"'R0903' is cryptic: use '# pylint: enable=too-few-public-methods' instead"
+use-symbolic-message-instead:11:0::"'R0903' is cryptic: use '# pylint: disable=too-few-public-methods' instead"
diff --git a/tests/messages/func_i0011.txt b/tests/messages/func_i0011.txt
index 66d4bfc68..609f20861 100644
--- a/tests/messages/func_i0011.txt
+++ b/tests/messages/func_i0011.txt
@@ -1,3 +1,3 @@
-I:  1: Id 'W0404' is used to disable 'reimported' message emission
+I:  1: 'W0404' is cryptic: use '# pylint: disable=reimported' instead
 I:  1: Locally disabling reimported (W0404)
 I:  1: Useless suppression of 'reimported'
diff --git a/tests/messages/func_i0012.txt b/tests/messages/func_i0012.txt
index 5ad4cd6c6..205475890 100644
--- a/tests/messages/func_i0012.txt
+++ b/tests/messages/func_i0012.txt
@@ -1 +1 @@
-I:  1: Id 'W0404' is used to enable 'reimported' message emission
+I:  1: 'W0404' is cryptic: use '# pylint: enable=reimported' instead
diff --git a/tests/messages/func_i0020.txt b/tests/messages/func_i0020.txt
index ddd1cb4a6..5ae78a4a9 100644
--- a/tests/messages/func_i0020.txt
+++ b/tests/messages/func_i0020.txt
@@ -1,3 +1,3 @@
-I:  7: Id 'W0612' is used to disable 'unused-variable' message emission
+I:  7: 'W0612' is cryptic: use '# pylint: disable=unused-variable' instead
 I:  7: Locally disabling unused-variable (W0612)
 I:  8: Suppressed 'unused-variable' (from line 7)
diff --git a/tests/messages/func_i0022.txt b/tests/messages/func_i0022.txt
index 28b7d71c1..0108fb107 100644
--- a/tests/messages/func_i0022.txt
+++ b/tests/messages/func_i0022.txt
@@ -9,13 +9,13 @@ I: 12: Locally disabling invalid-name (C0103)
 I: 12: Pragma "disable-msg" is deprecated, use "disable" instead
 I: 13: Suppressed 'invalid-name' (from line 12)
 I: 14: Pragma "enable-msg" is deprecated, use "enable" instead
-I: 16: Id 'C0103' is used to disable 'invalid-name' message emission
+I: 16: 'C0103' is cryptic: use '# pylint: disable=invalid-name' instead
 I: 16: Locally disabling invalid-name (C0103)
 I: 16: Pragma "disable-msg" is deprecated, use "disable" instead
 I: 17: Suppressed 'invalid-name' (from line 16)
-I: 18: Id 'C0103' is used to enable 'invalid-name' message emission
+I: 18: 'C0103' is cryptic: use '# pylint: enable=invalid-name' instead
 I: 18: Pragma "enable-msg" is deprecated, use "enable" instead
-I: 20: Id 'C0103' is used to disable 'invalid-name' message emission
+I: 20: 'C0103' is cryptic: use '# pylint: disable=invalid-name' instead
 I: 20: Locally disabling invalid-name (C0103)
 I: 21: Suppressed 'invalid-name' (from line 20)
-I: 22: Id 'C0103' is used to enable 'invalid-name' message emission
+I: 22: 'C0103' is cryptic: use '# pylint: enable=invalid-name' instead