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.

Author: Pierre-Sassoulas

ChangeLog

@@ -23,6 +23,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

pylint/checkers/misc.py

@@ -34,11 +34,10 @@
 
 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()
 

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 @@
 
 
 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 @@ def clear_by_id_managed_msgs(cls):
     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 @@ def is_one_message_enabled(self, msgid, line):
         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:

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"

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"

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()
 

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"

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):

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

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"

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"

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"

tests/functional/m/messages_managed_by_id.py

@@ -1,14 +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

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"

tests/functional/u/unused/unused_argument.py

@@ -76,7 +76,7 @@ def using_inner_function(self, etype, size=1):
                     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

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

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]

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"

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'

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

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)

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