refactor messages and checker handling

* new ast walker, built at registration time

* properly handle dependencies between checkers instead of
  priority based order

* only run checker necessary according to activated message

* much more simple command line: [en|dis]able-msg,
  [en|dis]able-msg-cat and [en|dis]able-report superseeded
  by [en|dis]able options

-> we should split current checkers into simpler checker with the
same name, and proper dependencies when needed

3 files changed, 160 insertions, 194 deletions

diff --git a/checkers/__init__.py b/checkers/__init__.py
index 84e4188..144b680 100644
--- a/checkers/__init__.py
+++ b/checkers/__init__.py
@@ -66,9 +66,7 @@ class BaseChecker(OptionsProviderMixIn, ASTWalker):
     """base class for checkers"""
 
     options = ()
-    priority = -9
-    enabled = True
-    may_be_disabled = True
+    needs_checkers = ()
     name = None
     level = 1
 
@@ -86,20 +84,6 @@ class BaseChecker(OptionsProviderMixIn, ASTWalker):
         """add a message of a given type"""
         self.linter.add_message(msg_id, line, node, args)
 
-    def is_enabled(self):
-        """return true if the checker is enabled"""
-        return self.enabled
-
-    def enable(self, enabled):
-        """enable / disable this checker if true / false is given
-
-        it false values has no effect if the checker can't be disabled
-        """
-        if not enabled and not self.may_be_disabled:
-            raise Exception("can't disable %s checker" % self.name)
-        if enabled or self.may_be_disabled:
-            self.enabled = enabled
-
     def package_dir(self):
         """return the base directory for the analysed package"""
         return dirname(self.linter.base_file)
diff --git a/lint.py b/lint.py
index d382e02..cbb165d 100644
--- a/lint.py
+++ b/lint.py
@@ -111,6 +111,37 @@ MSGS = {
               'Used when a bad value for an inline option is encountered.'),
     }
 
+
+class PyLintASTWalker(object):
+    def __init__(self):
+        # callbacks per node types
+        self.visit_events = {}
+        self.leave_events = {}
+
+    def add_checker(self, checker):
+        for member in dir(checker):
+            if member.startswith('visit_'):
+                cbs = self.visit_events.setdefault(cid, [])
+                cbs.append(getattr(checker, member[6:]))
+            if member.startswith('leave_'):
+                cbs = self.leave_events.setdefault(cid, [])
+                cbs.insert(0, getattr(checker, member[6:]))
+
+    def walk(self, astng):
+        """call visit events of astng checkers for the given node, recurse on
+        its children, then leave events.
+        """
+        cid = node.__class__.__name__.lower()
+        # generate events for this node on each checker
+        for cb in self.visit_events[cid]:
+            cb(astng)
+        # recurse on children
+        for child in astng.get_children():
+            self.walk(child)
+        for cb in self.leave_events[cid]:
+            cb(astng)
+
+
 class PyLinter(OptionsManagerMixIn, MessagesHandlerMixIn, ReportsHandlerMixIn,
                BaseRawChecker):
     """lint Python modules using external checkers.
@@ -136,19 +167,6 @@ class PyLinter(OptionsManagerMixIn, MessagesHandlerMixIn, ReportsHandlerMixIn,
                  'help' : 'Add <file or directory> to the black list. It \
 should be a base name, not a path. You may set this option multiple times.'}),
 
-               ('enable-checker',
-                {'type' : 'csv', 'metavar': '<checker ids>',
-                 'group': 'Messages control',
-                 'help' : 'Enable only checker(s) with the given id(s).\
-                 This option conflicts with the disable-checker option'}),
-
-               ('disable-checker',
-                {'type' : 'csv', 'metavar': '<checker ids>',
-                 'group': 'Messages control',
-                 'help' : 'Enable all checker(s) except those with the \
-                 given id(s).\
-                 This option conflicts with the enable-checker option'}),
-
                ('persistent',
                 {'default': True, 'type' : 'yn', 'metavar' : '<y_or_n>',
                  'level': 1,
@@ -214,25 +232,19 @@ This is used by the global evaluation report (R0004).'}),
                  'group': 'Reports',
                  'help' : 'Disable the report(s) with the given id(s).'}),
 
-               ('enable-msg-cat',
-                {'type' : 'string', 'metavar': '<msg cats>',
-                 'group': 'Messages control',
-                 'help' : 'Enable all messages in the listed categories (IRCWEF).'}),
-
-               ('disable-msg-cat',
-                {'type' : 'string', 'metavar': '<msg cats>', 'default': 'I',
-                 'group': 'Messages control',
-                 'help' : 'Disable all messages in the listed categories (IRCWEF).'}),
-
-               ('enable-msg',
+               ('enable',
                 {'type' : 'csv', 'metavar': '<msg ids>',
                  'group': 'Messages control',
-                 'help' : 'Enable the message(s) with the given id(s).'}),
+                 'help' : 'Enable the message or category or checker with the '
+                 'given id(s). You can either give multiple identifier '
+                 'separated by comma (,) or put this option multiple time.'}),
 
-               ('disable-msg',
+               ('disable',
                 {'type' : 'csv', 'metavar': '<msg ids>',
                  'group': 'Messages control',
-                 'help' : 'Disable the message(s) with the given id(s).'}),
+                 'help' : 'Disable themessage or category or checker with the '
+                 'given id(s). You can either give multiple identifier '
+                 'separated by comma (,) or put this option multiple time.'}),
                )
 
     option_groups = (
@@ -261,9 +273,7 @@ This is used by the global evaluation report (R0004).'}),
             'enable-report': self.enable_report,
             'disable-report': self.disable_report,
             'enable-msg': self.enable_message,
-            'disable-msg': self.disable_message,
-            'enable-msg-cat': self.enable_message_category,
-            'disable-msg-cat': self.disable_message_category}
+            'disable-msg': self.disable_message}
         full_version = '%%prog %s, \nastng %s, common %s\nPython %s' % (
             version, astng_version, common_version, sys.version)
         OptionsManagerMixIn.__init__(self, usage=__doc__,
@@ -318,11 +328,6 @@ This is used by the global evaluation report (R0004).'}),
                     meth(value)
         elif opt_name == 'output-format':
             self.set_reporter(REPORTER_OPT_MAP[value.lower()]())
-        elif opt_name in ('enable-checker', 'disable-checker'):
-            if not value:
-                return
-            checkerids = [v.lower() for v in check_csv(None, opt_name, value)]
-            self.enable_checkers(checkerids, opt_name == 'enable-checker')
         try:
             BaseRawChecker.set_option(self, opt_name, value, action, opt_dict)
         except UnsupportedAction:
@@ -337,7 +342,7 @@ This is used by the global evaluation report (R0004).'}),
         checker is an object implementing IRawChecker or / and IASTNGChecker
         """
         assert checker.priority <= 0, 'checker priority can\'t be >= 0'
-        self._checkers[checker.name] = checker
+        self._checkers.setdefault(checker.name, []).append(checker)
         if hasattr(checker, 'reports'):
             for r_id, r_title, r_cb in checker.reports:
                 self.register_report(r_id, r_title, r_cb, checker)
@@ -346,38 +351,14 @@ This is used by the global evaluation report (R0004).'}),
             self.register_messages(checker)
         checker.load_defaults()
 
-    def enable_checkers(self, listed, enabled):
-        """only enable/disable checkers from the given list"""
-        if enabled: # if we are activating a checker; deactivate them all first
-            for checker in self._checkers.values():
-                if not checker.may_be_disabled:
-                    continue
-                checker.enable(not enabled)
-        for checkerid in listed:
-            try:
-                checker = self._checkers[checkerid]
-            except KeyError:
-                raise Exception('no checker named %s' % checkerid)
-            checker.enable(enabled)
-
-    def disable_noerror_checkers(self):
-        """disable all checkers without error messages, and the
-        'miscellaneous' checker which can be safely deactivated in debug
-        mode
-        """
-        for checker in self._checkers.values():
-            if checker.name == 'miscellaneous':
-                checker.enable(False)
-                continue
-            # if checker is already explicitly disabled (e.g. rpython), don't
-            # enable it
-            if checker.enabled:
-                for msgid in getattr(checker, 'msgs', {}).keys():
-                    if msgid[0] == 'E':
-                        checker.enable(True)
-                        break
-                else:
-                    checker.enable(False)
+    def disable_noerror_messages(self):
+        for msgcat, msgids in self._msgs_by_category.iteritems():
+            if msgcat == 'E':
+                for msgid in msgids:
+                    self.enable_message(msgid)
+            else:
+                for msgid in msgids:
+                    self.disable_message(msgid)
 
     # block level option handling #############################################
     #
@@ -474,10 +455,22 @@ This is used by the global evaluation report (R0004).'}),
         self.reporter.include_ids = self.config.include_ids
         if not isinstance(files_or_modules, (list, tuple)):
             files_or_modules = (files_or_modules,)
-        checkers = sort_checkers(self._checkers.values())
+        rawcheckers = []
+        walker = PyLintASTWalker()
+        #
+        neededcheckers = set()
+        for checker in self._checkers.values():
+            for msgid in checker.msgs:
+                if self._msgs_state.get(msgid, True):
+                    neededcheckers.add(checker)
+                    break
         # notify global begin
-        for checker in checkers:
+        for checker in sort_checkers(neededcheckers):
             checker.open()
+            if implements(checker, IASTNGChecker):
+                walker.add_checker(checker)
+            if implements(checker, IRawChecker) and checker is not self: #XXX
+                rawcheckers.append(checker)
         # build ast and check modules or packages
         for descr in self.expand_files(files_or_modules):
             modname, filepath = descr['name'], descr['path']
@@ -495,11 +488,11 @@ This is used by the global evaluation report (R0004).'}),
             # fix the current file (if the source file was not available or
             # if it's actually a c extension)
             self.current_file = astng.file
-            self.check_astng_module(astng, checkers)
+            self.check_astng_module(astng, walker, rawcheckers)
         # notify global end
         self.set_current_module('')
         checkers.reverse()
-        for checker in  checkers:
+        for checker in checkers:
             checker.close()
 
     def expand_files(self, modules):
@@ -548,8 +541,7 @@ This is used by the global evaluation report (R0004).'}),
             #    traceback.print_exc()
             self.add_message('F0002', args=(ex.__class__, ex))
 
-
-    def check_astng_module(self, astng, checkers):
+    def check_astng_module(self, astng, walker, rawcheckers):
         """check a module from its astng representation, real work"""
         # call raw checkers if possible
         if not astng.pure_python:
@@ -566,34 +558,13 @@ This is used by the global evaluation report (R0004).'}),
             orig_state = self._module_msgs_state.copy()
             self._module_msgs_state = {}
             self.collect_block_lines(astng, orig_state)
-            for checker in checkers:
-                if implements(checker, IRawChecker) and checker is not self:
-                    stream.seek(0)
-                    checker.process_module(stream)
+            for checker in rawcheckers:
+                stream.seek(0)
+                checker.process_module(stream)
         # generate events to astng checkers
-        self.astng_events(astng, [checker for checker in checkers
-                                  if implements(checker, IASTNGChecker)])
+        walker.walk(astng)
         return True
 
-    def astng_events(self, astng, checkers, _reversed_checkers=None):
-        """generate event to astng checkers according to the current astng
-        node and recurse on its children
-        """
-        if _reversed_checkers is None:
-            _reversed_checkers = checkers[:]
-            _reversed_checkers.reverse()
-        if astng.is_statement:
-            self.stats['statement'] += 1
-        # generate events for this node on each checker
-        for checker in checkers:
-            checker.visit(astng)
-        # recurse on children
-        for child in astng.get_children():
-            self.astng_events(child, checkers, _reversed_checkers)
-        for checker in _reversed_checkers:
-            checker.leave(astng)
-
-
     # IASTNGChecker interface #################################################
 
     def open(self):
@@ -900,13 +871,14 @@ been issued by analysing pylint output status code
 
     def cb_error_mode(self, *args, **kwargs):
         """error mode:
-        * checkers without error messages are disabled
-        * for others, only the ERROR messages are displayed
+        * disable all but error messages
+        * disable the 'miscellaneous' checker which can be safely deactivated in
+          debug
         * disable reports
         * do not save execution information
         """
-        self.linter.disable_noerror_checkers()
-        self.linter.set_option('disable-msg-cat', 'WCRI')
+        self.linter.disable_noerror_messages()
+        self.disable_message('miscellaneous')
         self.linter.set_option('reports', False)
         self.linter.set_option('persistent', False)
 
diff --git a/utils.py b/utils.py
index 4f7f5e5..74946e1 100644
--- a/utils.py
+++ b/utils.py
@@ -24,6 +24,7 @@ from os import linesep
 from logilab.common.textutils import normalize_text
 from logilab.common.configuration import rest_format_section
 from logilab.common.ureports import Section
+from logilab.common.graph import ordered_nodes
 
 from logilab.astng import Module
 
@@ -41,6 +42,8 @@ MSG_TYPES = {
     'E' : 'error',
     'F' : 'fatal'
     }
+MSG_TYPES_LONG = dict([(v, k) for k, v in MSG_TYPES.iteritems()])
+
 MSG_TYPES_STATUS = {
     'I' : 0,
     'C' : 16,
@@ -51,12 +54,14 @@ MSG_TYPES_STATUS = {
     }
 
 def sort_checkers(checkers):
-    """return a list of enabled checker sorted by priority"""
-    checkers = [checker for checker in checkers if checker.is_enabled()]
-    checkers.sort(lambda x, y: cmp(-x.priority, -y.priority) )
-    return checkers
-
-def sort_msgs(msg_ids):
+    graph = {}
+    cls_instance = {}
+    for checker in checkers:
+        graph[cube.__class__] = set(checker.needs_checkers)
+        cls_instance[cube.__class__] = checker
+    return [cls_instance.get(cls) for cls in ordered_nodes(graph)]
+
+def sort_msgs(msgids):
     """sort message identifiers according to their category first"""
     msg_order = 'EWRCIF'
     def cmp_func(msgid1, msgid2):
@@ -65,8 +70,8 @@ def sort_msgs(msg_ids):
             return cmp(msg_order.index(msgid1[0]), msg_order.index(msgid2[0]))
         else:
             return cmp(msgid1, msgid2)
-    msg_ids.sort(cmp_func)
-    return msg_ids
+    msgids.sort(cmp_func)
+    return msgids
 
 def get_module_and_frameid(node):
     """return the module name and the frame id in the module"""
@@ -84,6 +89,12 @@ def get_module_and_frameid(node):
     obj.reverse()
     return module, '.'.join(obj)
 
+def category_id(id):
+    id = id.upper()
+    if id in MSG_TYPES:
+        return id
+    return MSG_TYPES_LONG.get(id)
+
 
 class Message:
     def __init__(self, checker, msgid, msg, descr):
@@ -105,8 +116,7 @@ class MessagesHandlerMixIn:
         self._messages = {}
         self._msgs_state = {}
         self._module_msgs_state = {} # None
-        self._msg_cats_state = {}
-        self._module_msg_cats_state = None
+        self._msgs_by_category = {}
         self.msg_status = 0
 
     def register_messages(self, checker):
@@ -128,10 +138,11 @@ class MessagesHandlerMixIn:
                    'Inconsistent checker part in message id %r' % msgid
             chkid = msgid[1:3]
             self._messages[msgid] = Message(checker, msgid, msg, msgdescr)
+            self._msgs_by_category.setdefault(msgid[0], []).append(msgid)
 
-    def get_message_help(self, msg_id, checkerref=False):
+    def get_message_help(self, msgid, checkerref=False):
         """return the help string for the given message id"""
-        msg = self.check_message_id(msg_id)
+        msg = self.check_message_id(msgid)
         desc = normalize_text(' '.join(msg.descr.split()), indent='  ')
         if checkerref:
             desc += ' This message belongs to the %s checker.' % \
@@ -142,17 +153,25 @@ class MessagesHandlerMixIn:
             return ':%s: *%s*\n%s' % (msg.msgid, title, desc)
         return ':%s:\n%s' % (msg.msgid, desc)
 
-    def disable_message(self, msg_id, scope='package', line=None):
+    def disable_message(self, msgid, scope='package', line=None):
         """don't output message of the given id"""
         assert scope in ('package', 'module')
-        msg = self.check_message_id(msg_id)
+        catid = category_id(msgid)
+        if catid is not None:
+            self._activate_msgs_cat(catid, scope, line, False, 'I0011')
+            return
+        # msgid is a checker name?
+        if msgid.lower() in self._checkers:
+            self._activate_checker_msgs(msgid.lower(), True)
+            return
+        msg = self.check_message_id(msgid)
         if scope == 'module':
             assert line > 0
             try:
                 self._module_msgs_state[msg.msgid][line] = False
             except KeyError:
                 self._module_msgs_state[msg.msgid] = {line: False}
-                if msg_id != 'I0011':
+                if msgid != 'I0011':
                     self.add_message('I0011', line=line, args=msg.msgid)
 
         else:
@@ -162,11 +181,20 @@ class MessagesHandlerMixIn:
             self.config.disable_msg = [mid for mid, val in msgs.items()
                                        if not val]
 
-    def enable_message(self, msg_id, scope='package', line=None):
+    def enable_message(self, msgid, scope='package', line=None):
         """reenable message of the given id"""
         assert scope in ('package', 'module')
-        msg = self.check_message_id(msg_id)
-        msg.checker.enabled = True # ensure the related checker is enabled
+        catid = category_id(msgid)
+        # msgid is a category?
+        if catid is not None:
+            self._activate_msgs_cat(catid, scope, line, True, 'I0012')
+            return
+        # msgid is a checker name?
+        if msgid.lower() in self._checkers:
+            self._activate_checker_msgs(msgid.lower(), True)
+            return
+        # msgid is a msgid.
+        msg = self.check_message_id(msgid)
         if scope == 'module':
             assert line > 0
             try:
@@ -180,59 +208,41 @@ class MessagesHandlerMixIn:
             # sync configuration object
             self.config.enable_msg = [mid for mid, val in msgs.items() if val]
 
-    def _cat_ids(self, categories):
-        for catid in categories:
-            catid = catid.upper()
-            if not catid in MSG_TYPES:
-                raise Exception('Unknown category identifier %s' % catid)
-            yield catid
+    def _activate_checker_msgs(self, checkername, value):
+        for checker in self._checkers[checkername]:
+            for msgid in checker.msgs:
+                msgsdict[msgid] = value
 
-    def disable_message_category(self, categories, scope='package', line=None):
-        """don't output message in the given category"""
+    def _activate_msgs_cat(self, catid, scope, line, value, warnmsgid):
         assert scope in ('package', 'module')
-        for catid in self._cat_ids(categories):
-            if scope == 'module':
-                self.add_message('I0011', line=line, args=catid)
-                self._module_msg_cats_state[catid] = False
-            else:
-                self._msg_cats_state[catid] = False
-
-    def enable_message_category(self, categories, scope='package', line=None):
-        """reenable message of the given category"""
-        assert scope in ('package', 'module')
-        for catid in self._cat_ids(categories):
-            if scope == 'module':
-                self.add_message('I0012', line=line, args=catid)
-                self._module_msg_cats_state[catid] = True
-            else:
-                self._msg_cats_state[catid] = True
+        if scope == 'module':
+            self.add_message(warnmsgid, line=line, args=catid)
+            msgsdict = self._module_msgs_state
+        else:
+            msgsdict = self._msgs_state
+        for msgid in self._msgs_by_category.get(catid):
+            msgsdict[msgid] = value
 
-    def check_message_id(self, msg_id):
+    def check_message_id(self, msgid):
         """raise UnknownMessage if the message id is not defined"""
-        msg_id = msg_id.upper()
+        msgid = msgid.upper()
         try:
-            return self._messages[msg_id]
+            return self._messages[msgid]
         except KeyError:
-            raise UnknownMessage('No such message id %s' % msg_id)
+            raise UnknownMessage('No such message id %s' % msgid)
 
-    def is_message_enabled(self, msg_id, line=None):
+    def is_message_enabled(self, msgid, line=None):
         """return true if the message associated to the given message id is
         enabled
         """
-        try:
-            if not self._module_msg_cats_state[msg_id[0]]:
-                return False
-        except (KeyError, TypeError):
-            if not self._msg_cats_state.get(msg_id[0], True):
-                return False
         if line is None:
-            return self._msgs_state.get(msg_id, True)
+            return self._msgs_state.get(msgid, True)
         try:
-            return self._module_msgs_state[msg_id][line]
+            return self._module_msgs_state[msgid][line]
         except (KeyError, TypeError):
-            return self._msgs_state.get(msg_id, True)
+            return self._msgs_state.get(msgid, True)
 
-    def add_message(self, msg_id, line=None, node=None, args=None):
+    def add_message(self, msgid, line=None, node=None, args=None):
         """add the message corresponding to the given id.
 
         If provided, msg is expanded using args
@@ -243,18 +253,18 @@ class MessagesHandlerMixIn:
         if line is None and node is not None:
             line = node.fromlineno
         # should this message be displayed
-        if not self.is_message_enabled(msg_id, line):
+        if not self.is_message_enabled(msgid, line):
             return
         # update stats
-        msg_cat = MSG_TYPES[msg_id[0]]
-        self.msg_status |= MSG_TYPES_STATUS[msg_id[0]]
+        msg_cat = MSG_TYPES[msgid[0]]
+        self.msg_status |= MSG_TYPES_STATUS[msgid[0]]
         self.stats[msg_cat] += 1
         self.stats['by_module'][self.current_name][msg_cat] += 1
         try:
-            self.stats['by_msg'][msg_id] += 1
+            self.stats['by_msg'][msgid] += 1
         except KeyError:
-            self.stats['by_msg'][msg_id] = 1
-        msg = self._messages[msg_id].msg
+            self.stats['by_msg'][msgid] = 1
+        msg = self._messages[msgid].msg
         # expand message ?
         if args:
             msg %= args
@@ -266,13 +276,13 @@ class MessagesHandlerMixIn:
             module, obj = get_module_and_frameid(node)
             path = node.root().file
         # add the message
-        self.reporter.add_message(msg_id, (path, module, obj, line or 1), msg)
+        self.reporter.add_message(msgid, (path, module, obj, line or 1), msg)
 
     def help_message(self, msgids):
         """display help messages for the given message identifiers"""
-        for msg_id in msgids:
+        for msgid in msgids:
             try:
-                print self.get_message_help(msg_id, True)
+                print self.get_message_help(msgid, True)
                 print
             except UnknownMessage, ex:
                 print ex
@@ -281,8 +291,8 @@ class MessagesHandlerMixIn:
 
     def list_checkers_messages(self, checker):
         """print checker's messages in reST format"""
-        for msg_id in sort_msgs(checker.msgs.keys()):
-            print self.get_message_help(msg_id, False)
+        for msgid in sort_msgs(checker.msgs.keys()):
+            print self.get_message_help(msgid, False)
 
     def print_full_documentation(self):
         """output a full documentation in ReST format"""
@@ -338,13 +348,13 @@ class MessagesHandlerMixIn:
 
     def list_sorted_messages(self):
         """output full sorted messages list in ReST format"""
-        msg_ids = []
+        msgids = []
         for checker in self._checkers.values():
-            for msg_id in checker.msgs.keys():
-                msg_ids.append(msg_id)
-        msg_ids.sort()
-        for msg_id in msg_ids:
-            print self.get_message_help(msg_id, False)
+            for msgid in checker.msgs.keys():
+                msgids.append(msgid)
+        msgids.sort()
+        for msgid in msgids:
+            print self.get_message_help(msgid, False)
         print