From 4c401c9176b3390c4691c1d4786396e2a458882a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 2 Jan 2021 14:02:36 +0100 Subject: 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. --- ChangeLog | 2 ++ pylint/checkers/misc.py | 9 +++--- pylint/message/message_handler_mix_in.py | 32 +++++++--------------- .../access_attr_before_def_false_positive.txt | 3 ++ tests/functional/b/base_init_vars.txt | 2 +- .../c/classes_meth_could_be_a_function.py | 4 +-- .../c/classes_meth_could_be_a_function.txt | 4 ++- tests/functional/c/ctor_arguments.py | 2 +- tests/functional/c/ctor_arguments.txt | 2 ++ tests/functional/d/decorator_scope.txt | 2 +- tests/functional/e/external_classmethod_crash.txt | 4 +-- tests/functional/g/genexp_in_class_scope.txt | 2 +- tests/functional/m/messages_managed_by_id.py | 19 +++++++++---- tests/functional/m/messages_managed_by_id.txt | 8 ++++-- tests/functional/u/unused/unused_argument.py | 2 +- tests/functional/u/unused/unused_argument.txt | 1 + .../u/use/use_symbolic_message_instead.py | 11 ++++++++ .../u/use/use_symbolic_message_instead.txt | 8 ++++++ tests/messages/func_i0011.txt | 2 +- tests/messages/func_i0012.txt | 2 +- tests/messages/func_i0020.txt | 2 +- tests/messages/func_i0022.txt | 8 +++--- 22 files changed, 79 insertions(+), 52 deletions(-) create mode 100644 tests/functional/a/access/access_attr_before_def_false_positive.txt create mode 100644 tests/functional/u/use/use_symbolic_message_instead.py create mode 100644 tests/functional/u/use/use_symbolic_message_instead.txt 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 -- cgit v1.2.1