Add messages I0020 and I0021 for reporting of suppressed messages and useless suppression pragmas. Closes #110840

Both messages are disabled by default, and only emitted after all other
checkers have been processed.

8 files changed, 114 insertions, 4 deletions

diff --git a/ChangeLog b/ChangeLog
index cc1accaf1..6b219ad16 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,9 +9,11 @@ ChangeLog for PyLint
       'disable-all' inline directive in favour of 'skip-file' (patch by
       A.Fayolle)
 
+    * #110840: Add messages I0020 and I0021 for reporting of suppressed messages
+      and useless suppression pragmas. (patch by Torsten Marek)
 
-    * Changed the regular expression for inline options so that
-      it must be preceeded by a # (patch by Torsten Marek)
+    * Changed the regular expression for inline options so that it must be
+      preceeded by a # (patch by Torsten Marek)
 
 2012-10-05  --  0.26.0
     * #106534: add --ignore-imports option to code similarity checking
diff --git a/lint.py b/lint.py
index 94d683ece..c52391e32 100644
--- a/lint.py
+++ b/lint.py
@@ -131,6 +131,16 @@ MSGS = {
               'You should preferably use "pylint:skip-file" as this directive '
               'has a less confusing name. Do this only if you are sure that all '
               'people running Pylint on your code have version >= 0.26'),
+    'I0020': ('Suppressed %s (from line %d)',
+              'suppressed-message',
+              'A message was triggered on a line, but suppressed explicitly '
+              'by a disable= comment in the file. This message is not '
+              'generated for messages that are ignored due to configuration '
+              'settings.'),
+    'I0021': ('Useless suppression of %s',
+              'useless-suppression',
+              'Reported when a message is explicitly disabled for a line or '
+              'a block of code, but never triggered.'),
 
 
     'E0001': ('%s',
@@ -485,6 +495,7 @@ This is used by the global evaluation report (RP0004).'}),
             firstchildlineno = last
         for msgid, lines in msg_state.iteritems():
             for lineno, state in lines.items():
+                original_lineno = lineno
                 if first <= lineno <= last:
                     if lineno > firstchildlineno:
                         state = True
@@ -495,6 +506,9 @@ This is used by the global evaluation report (RP0004).'}),
                         if not line in self._module_msgs_state.get(msgid, ()):
                             if line in lines: # state change in the same block
                                 state = lines[line]
+                                original_lineno = line
+                            if not state:
+                                self._suppression_mapping[(msgid, line)] = original_lineno
                             try:
                                 self._module_msgs_state[msgid][line] = state
                             except KeyError:
@@ -599,6 +613,8 @@ This is used by the global evaluation report (RP0004).'}),
         if modname:
             self._module_msgs_state = {}
             self._module_msg_cats_state = {}
+            self._raw_module_msgs_state = {}
+            self._ignored_msgs = {}
 
     def get_astng(self, filepath, modname):
         """return a astng representation for a module"""
@@ -626,8 +642,11 @@ This is used by the global evaluation report (RP0004).'}),
             if self._ignore_file:
                 return False
             # walk ast to collect line numbers
+            for msg, lines in self._module_msgs_state.iteritems():
+                self._raw_module_msgs_state[msg] = lines.copy()
             orig_state = self._module_msgs_state.copy()
             self._module_msgs_state = {}
+            self._suppression_mapping = {}
             self.collect_block_lines(astng, orig_state)
             for checker in rawcheckers:
                 checker.process_module(astng)
@@ -650,6 +669,7 @@ This is used by the global evaluation report (RP0004).'}),
 
         if persistent run, pickle results for later comparison
         """
+        self._add_suppression_messages()
         if self.base_name is not None:
             # load previous results if any
             previous_stats = config.load_results(self.base_name)
@@ -670,6 +690,16 @@ This is used by the global evaluation report (RP0004).'}),
 
     # specific reports ########################################################
 
+    def _add_suppression_messages(self):
+        for warning, lines in self._raw_module_msgs_state.iteritems():
+            for line, enable in lines.iteritems():
+                if not enable and (warning, line) not in self._ignored_msgs:
+                    self.add_message('I0021', line, None, (warning,))
+
+        for (warning, from_), lines in self._ignored_msgs.iteritems():
+            for line in lines:
+                self.add_message('I0020', line, None, (warning, from_))
+
     def report_evaluation(self, sect, stats, previous_stats):
         """make the global evaluation report"""
         # check with at least check 1 statements (usually 0 when there is a
@@ -908,6 +938,8 @@ are done by default'''}),
         level=1)
         # read configuration
         linter.disable('W0704')
+        linter.disable('I0020')
+        linter.disable('I0021')
         linter.read_config_file()
         # is there some additional plugins in the file configuration, in
         config_parser = linter.cfgfile_parser
diff --git a/test/input/func_block_disable_msg.py b/test/input/func_block_disable_msg.py
index caf2d5b62..1a43ede62 100644
--- a/test/input/func_block_disable_msg.py
+++ b/test/input/func_block_disable_msg.py
@@ -101,6 +101,15 @@ class Foo(object):
         # error
         print self.blip
 
+    def meth10(self):
+        """Test double disable"""
+        # pylint: disable=E1101
+        # no error
+        print self.bla
+        # pylint: disable=E1101
+        print self.blu
+
+
 class ClassLevelMessage(object):
     """shouldn't display to much attributes/not enough methods messages
     """
diff --git a/test/input/func_i0020.py b/test/input/func_i0020.py
new file mode 100644
index 000000000..8c0fe9f5c
--- /dev/null
+++ b/test/input/func_i0020.py
@@ -0,0 +1,8 @@
+"""Test for reporting of suppressed messages."""
+
+__revision__ = 0
+
+def suppressed():
+    """A function with an unused variable."""
+    # pylint: disable=W0612
+    var = 0
diff --git a/test/messages/func_i0011.txt b/test/messages/func_i0011.txt
index 667e36401..4b5ef231f 100644
--- a/test/messages/func_i0011.txt
+++ b/test/messages/func_i0011.txt
@@ -1,2 +1,2 @@
 I:  1: Locally disabling W0404
-
+I:  1: Useless suppression of W0404
diff --git a/test/messages/func_i0020.txt b/test/messages/func_i0020.txt
new file mode 100644
index 000000000..5a9ef9737
--- /dev/null
+++ b/test/messages/func_i0020.txt
@@ -0,0 +1,2 @@
+I:  7: Locally disabling W0612
+I:  8: Suppressed W0612 (from line 7)
diff --git a/test/unittest_lint.py b/test/unittest_lint.py
index 533aa94a6..2043ca64d 100644
--- a/test/unittest_lint.py
+++ b/test/unittest_lint.py
@@ -26,7 +26,9 @@ from logilab.common.compat import reload
 from pylint import config
 from pylint.lint import PyLinter, Run, UnknownMessage, preprocess_options, \
      ArgumentPreprocessingError
-from pylint.utils import sort_msgs, PyLintASTWalker
+from pylint.utils import sort_msgs, PyLintASTWalker, MSG_STATE_SCOPE_CONFIG, \
+     MSG_STATE_SCOPE_MODULE
+
 from pylint import checkers
 
 class SortMessagesTC(TestCase):
@@ -131,6 +133,21 @@ class PyLinterTC(TestCase):
         self.assertTrue(linter.is_message_enabled('C0121'))
         self.assertTrue(linter.is_message_enabled('C0121', line=1))
 
+    def test_message_state_scope(self):
+        linter = self.linter
+        linter.open()
+        linter.disable('C0121')
+        self.assertEqual(MSG_STATE_SCOPE_CONFIG,
+                         linter.get_message_state_scope('C0121'))
+        linter.disable('W0101', scope='module', line=3)
+        self.assertEqual(MSG_STATE_SCOPE_CONFIG,
+                         linter.get_message_state_scope('C0121'))
+        self.assertEqual(MSG_STATE_SCOPE_MODULE,
+                         linter.get_message_state_scope('W0101', 3))
+        linter.enable('W0102', scope='module', line=3)
+        self.assertEqual(MSG_STATE_SCOPE_MODULE,
+                         linter.get_message_state_scope('W0102', 3))
+
     def test_enable_message_block(self):
         linter = self.linter
         linter.open()
@@ -140,6 +157,7 @@ class PyLinterTC(TestCase):
         linter.process_module(astng)
         orig_state = linter._module_msgs_state.copy()
         linter._module_msgs_state = {}
+        linter._suppression_mapping = {}
         linter.collect_block_lines(astng, orig_state)
         # global (module level)
         self.assertTrue(linter.is_message_enabled('W0613'))
@@ -177,6 +195,17 @@ class PyLinterTC(TestCase):
         self.assertTrue(linter.is_message_enabled('E1101', 75))
         self.assertTrue(linter.is_message_enabled('E1101', 77))
 
+        self.assertEqual(17, linter._suppression_mapping['W0613', 18])
+        self.assertEqual(30, linter._suppression_mapping['E1101', 33])
+        self.assert_(('E1101', 46) not in linter._suppression_mapping)
+        self.assertEqual(1, linter._suppression_mapping['C0302', 18])
+        self.assertEqual(1, linter._suppression_mapping['C0302', 50])
+        # This is tricky. While the disable in line 106 is disabling
+        # both 108 and 110, this is usually not what the user wanted.
+        # Therefore, we report the closest previous disable comment.
+        self.assertEqual(106, linter._suppression_mapping['E1101', 108])
+        self.assertEqual(109, linter._suppression_mapping['E1101', 110])
+
     def test_enable_by_symbol(self):
         """messages can be controlled by symbolic names.
 
diff --git a/utils.py b/utils.py
index 32ad4e942..c8b07e567 100644
--- a/utils.py
+++ b/utils.py
@@ -57,6 +57,8 @@ MSG_TYPES_STATUS = {
     }
 
 _MSG_ORDER = 'EWRCIF'
+MSG_STATE_SCOPE_CONFIG = 0
+MSG_STATE_SCOPE_MODULE = 1
 
 def sort_msgs(msgids):
     """sort message identifiers according to their category first"""
@@ -115,8 +117,10 @@ class MessagesHandlerMixIn:
         self._messages_by_symbol = {}
         self._msgs_state = {}
         self._module_msgs_state = {} # None
+        self._raw_module_msgs_state = {}
         self._msgs_by_category = {}
         self.msg_status = 0
+        self._ignored_msgs = {}
 
     def register_messages(self, checker):
         """register a dictionary of messages
@@ -260,6 +264,14 @@ class MessagesHandlerMixIn:
         except KeyError:
             raise UnknownMessage('No such message id %s' % msgid)
 
+    def get_message_state_scope(self, msgid, line=None):
+        """Returns the scope at which a message was enabled/disabled."""
+        try:
+            if line in self._module_msgs_state[msgid]:
+                return MSG_STATE_SCOPE_MODULE
+        except (KeyError, TypeError):
+            return MSG_STATE_SCOPE_CONFIG
+
     def is_message_enabled(self, msgid, line=None):
         """return true if the message associated to the given message id is
         enabled
@@ -275,6 +287,20 @@ class MessagesHandlerMixIn:
         except (KeyError, TypeError):
             return self._msgs_state.get(msgid, True)
 
+    def handle_ignored_message(self, state_scope, msgid, line, node, args):
+        """Report an ignored message.
+
+        state_scope is either MSG_STATE_SCOPE_MODULE or MSG_STATE_SCOPE_CONFIG,
+        depending on whether the message was disabled locally in the module,
+        or globally. The other arguments are the same as for add_message.
+        """
+        if state_scope == MSG_STATE_SCOPE_MODULE:
+            try:
+                orig_line = self._suppression_mapping[(msgid, line)]
+                self._ignored_msgs.setdefault((msgid, orig_line), set()).add(line)
+            except KeyError:
+                pass
+
     def add_message(self, msgid, line=None, node=None, args=None):
         """add the message corresponding to the given id.
 
@@ -291,6 +317,8 @@ class MessagesHandlerMixIn:
             col_offset = None
         # should this message be displayed
         if not self.is_message_enabled(msgid, line):
+            self.handle_ignored_message(
+                self.get_message_state_scope(msgid, line), msgid, line, node, args)
             return
         # update stats
         msg_cat = MSG_TYPES[msgid[0]]