diff options
author | Pierre Sassoulas <pierre.sassoulas@gmail.com> | 2021-01-02 14:02:36 +0100 |
---|---|---|
committer | Pierre Sassoulas <pierre.sassoulas@gmail.com> | 2021-03-17 21:47:40 +0100 |
commit | f501b87844e08aaf2d1ecce1e494f77ad7762bc7 (patch) | |
tree | 8d3cf36a434478680ed0b4f684503e1f97c71897 | |
parent | a20c4de75bcd7ca44fae627c9783e998c79b86b2 (diff) | |
download | pylint-git-f501b87844e08aaf2d1ecce1e494f77ad7762bc7.tar.gz |
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, 76 insertions, 52 deletions
@@ -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 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 13fc48b40..ddcf0bedc 100644 --- a/tests/functional/m/messages_managed_by_id.py +++ b/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 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 |