Added module-level ci exclusions, other minor stuff (Azure#6088)

* implemented exclusions of modules for specific rules in ci

* added all modules with rule violations to ci exclusions

* empty exclusion file case

* get mod from loader class irrespective of CLI installation method

* feedback

tools/automation/cli_linter/__init__.py

@@ -5,13 +5,10 @@
 
 
 import sys
-import yaml
 import os
-import inspect
+import yaml
 from knack.help_files import helps
-from importlib import import_module
-from pkgutil import iter_modules
-from .linter import LinterManager, _get_command_groups
+from .linter import LinterManager
 
 
 def define_arguments(parser):
@@ -56,24 +53,16 @@ def main(args):
     from ..utilities.path import get_command_modules_paths
     exclusions = {}
     command_modules_paths = get_command_modules_paths()
-    for _, path in get_command_modules_paths():
+    for _, path in command_modules_paths:
         exclusion_path = os.path.join(path, 'linter_exclusions.yml')
         if os.path.isfile(exclusion_path):
             mod_exclusions = yaml.load(open(exclusion_path))
             exclusions.update(mod_exclusions)
 
     # only run linter on modules specified
     if args.modules:
-        allowed_module_paths = tuple(path for mod, path in command_modules_paths if mod in args.modules)
-        for command_name, command in list(command_table.items()):
-            # brute force way to remove all traces from excluded modules
-            loader_cls = command.loader.__class__
-            loader_file_path = inspect.getfile(loader_cls)
-            if not loader_file_path.startswith(allowed_module_paths):
-                del command_table[command_name]
-                help_file_entries.pop(command_name, None)
-                for group_name in _get_command_groups(command_name):
-                    help_file_entries.pop(group_name, None)
+        from .util import include_mods
+        command_table, help_file_entries = include_mods(command_table, help_file_entries, args.modules)
 
 
     # Instantiate and run Linter

tools/automation/cli_linter/linter.py

@@ -8,7 +8,9 @@
 import argparse
 from importlib import import_module
 from pkgutil import iter_modules
+import yaml
 import colorama
+from .util import get_command_groups, share_element, exclude_mods
 
 
 class Linter(object):
@@ -30,7 +32,7 @@ def __init__(self, command_table=None, help_file_entries=None, loaded_help=None)
 
         # populate command groups
         for command_name in self._commands:
-            self._command_groups.update(_get_command_groups(command_name))
+            self._command_groups.update(get_command_groups(command_name))
 
     @property
     def commands(self):
@@ -59,13 +61,12 @@ def get_command_group_help(self, command_group_name):
     def get_parameter_help(self, command_name, parameter_name):
         options = self._command_table.get(command_name).arguments.get(parameter_name).type.settings.get('options_list')
         parameter_helps = self._loaded_help.get(command_name).parameters
-        param_help = next((param for param in parameter_helps if _share_element(options, param.name.split())), None)
+        param_help = next((param for param in parameter_helps if share_element(options, param.name.split())), None)
         # workaround for --ids which is not does not generate doc help (BUG)
         if not param_help:
             return self._command_table.get(command_name).arguments.get(parameter_name).type.settings.get('help')
         return param_help.short_summary or param_help.long_summary
 
-
     def _get_loaded_help_description(self, entry):
         return self._loaded_help.get(entry).short_summary or self._loaded_help.get(entry).long_summary
 
@@ -80,11 +81,24 @@ def __init__(self, command_table=None, help_file_entries=None, loaded_help=None,
             'commands': {},
             'params': {}
         }
+        self._ci_exclusions = {}
+        self._loaded_help = loaded_help
+        self._command_table = command_table
+        self._help_file_entries = help_file_entries
         self._exit_code = 0
+        self._ci = False
 
     def add_rule(self, rule_type, rule_name, rule_callable):
         if rule_type in self._rules:
-            self._rules.get(rule_type)[rule_name] = rule_callable
+            def get_linter():
+                if rule_name in self._ci_exclusions and self._ci:
+                    mod_exclusions = self._ci_exclusions[rule_name]
+                    command_table, help_file_entries = exclude_mods(self._command_table, self._help_file_entries,
+                                                                    mod_exclusions)
+                    return Linter(command_table=command_table, help_file_entries=help_file_entries,
+                                  loaded_help=self._loaded_help)
+                return self.linter
+            self._rules[rule_type][rule_name] = rule_callable, get_linter
 
     def mark_rule_failure(self):
         self._exit_code = 1
@@ -98,8 +112,13 @@ def exit_code(self):
         return self._exit_code
 
     def run(self, run_params=None, run_commands=None, run_command_groups=None, run_help_files_entries=None, ci=False):
+        self._ci = ci
         paths = import_module('automation.cli_linter.rules').__path__
 
+        if paths:
+            ci_exclusions_path = os.path.join(paths[0], 'ci_exclusions.yml')
+            self._ci_exclusions = yaml.load(open(ci_exclusions_path)) or {}
+
         # find all defined rules and check for name conflicts
         found_rules = set()
         for _, name, _ in iter_modules(paths):
@@ -109,8 +128,6 @@ def run(self, run_params=None, run_commands=None, run_command_groups=None, run_h
                 if hasattr(add_to_linter_func, 'linter_rule'):
                     if rule_name in found_rules:
                         raise Exception('Multiple rules found with the same name: %s' % rule_name)
-                    if ci and hasattr(add_to_linter_func, 'exclude_from_ci'):
-                        continue
                     found_rules.add(rule_name)
                     add_to_linter_func(self)
 
@@ -129,21 +146,23 @@ def run(self, run_params=None, run_commands=None, run_command_groups=None, run_h
             self._run_rules('params')
 
         if not self.exit_code:
-            print('\nNo violations found.')
+            print(os.linesep + 'No violations found.')
         colorama.deinit()
         return self.exit_code
 
     def _run_rules(self, rule_group):
         from colorama import Fore
-        for rule_name, rule_func in self._rules.get(rule_group).items():
-            violations = sorted(rule_func()) or []
-            if violations:
-                print('- {} FAIL{}: {}'.format(Fore.RED, Fore.RESET, rule_name))
-                for violation_msg in violations:
-                    print(violation_msg)
-                print()
-            else:
-                print('- {} pass{}: {} '.format(Fore.GREEN, Fore.RESET, rule_name))
+        for rule_name, (rule_func, linter_callable) in self._rules.get(rule_group).items():
+            # use new linter if needed
+            with LinterScope(self, linter_callable):
+                violations = sorted(rule_func()) or []
+                if violations:
+                    print('- {} FAIL{}: {}'.format(Fore.RED, Fore.RESET, rule_name))
+                    for violation_msg in violations:
+                        print(violation_msg)
+                    print()
+                else:
+                    print('- {} pass{}: {} '.format(Fore.GREEN, Fore.RESET, rule_name))
 
 
 class RuleError(Exception):
@@ -153,12 +172,14 @@ class RuleError(Exception):
     pass
 
 
-def _share_element(first_iter, second_iter):
-    return any(element in first_iter for element in second_iter)
+class LinterScope():
+    def __init__(self, linter_manager, linter_callable):
+        self.linter_manager = linter_manager
+        self.linter = linter_callable()
+        self.main_linter = linter_manager.linter
+
+    def __enter__(self):
+        self.linter_manager.linter = self.linter
 
-def _get_command_groups(command_name):
-    command_args = []
-    for arg in command_name.split()[:-1]:
-        command_args.append(arg)
-        if command_args:
-            yield ' '.join(command_args)
+    def __exit__(self, exc_type, value, traceback):
+        self.linter_manager.linter = self.main_linter

tools/automation/cli_linter/rule_decorators.py

@@ -6,11 +6,6 @@
 from .linter import RuleError
 
 
-def exclude_from_ci(func):
-    func.exclude_from_ci = True
-    return func
-
-
 def help_file_entry_rule(func):
     return _get_decorator(func, 'help_file_entries', 'Help-Entry: `{}`')
 

tools/automation/cli_linter/rules/ci_exclusions.yml

@@ -0,0 +1,38 @@
+---
+# the following are the rules and the modules that are excluded from those rules in the ci
+# note that this is a TEMPORARY stop-gap and the rule violations should be addressed
+
+missing_command_help_rule:
+- appservice
+- dla
+- rdbms
+- role
+- vm
+no_ids_for_list_commands_rule:
+- advisor
+- appservice
+- dla
+- dls
+- eventgrid
+- iot
+- rdbms
+- sql
+- vm
+missing_parameter_help_rule:
+- acs
+- appservice
+- cdn
+- dla
+- dls
+- eventgrid
+- eventhubs
+- keyvault
+- rdbms
+- resource
+- role
+- servicebus
+- servicefabric
+- sql
+- storage
+- vm
+...

tools/automation/cli_linter/rules/command_group_rules.py

@@ -3,7 +3,7 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from ..rule_decorators import command_group_rule, exclude_from_ci
+from ..rule_decorators import command_group_rule
 from ..linter import RuleError
 
 

tools/automation/cli_linter/rules/command_rules.py

@@ -3,18 +3,16 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from ..rule_decorators import command_rule, exclude_from_ci
+from ..rule_decorators import command_rule
 from ..linter import RuleError
 
 
-@exclude_from_ci
 @command_rule
 def missing_command_help_rule(linter, command_name):
     if not linter.get_command_help(command_name):
         raise RuleError('Missing help')
 
 
-@exclude_from_ci
 @command_rule
 def no_ids_for_list_commands_rule(linter, command_name):
     if command_name.split()[-1] == 'list' and 'ids' in linter.get_command_parameters(command_name):

tools/automation/cli_linter/rules/parameter_rules.py

@@ -3,11 +3,10 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from ..rule_decorators import parameter_rule, exclude_from_ci
+from ..rule_decorators import parameter_rule
 from ..linter import RuleError
 
 
-@exclude_from_ci
 @parameter_rule
 def missing_parameter_help_rule(linter, command_name, parameter_name):
     if not linter.get_parameter_help(command_name, parameter_name):

tools/automation/cli_linter/util.py

@@ -0,0 +1,80 @@
+# --------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+# --------------------------------------------------------------------------------------------
+
+
+import re
+import os
+import inspect
+
+
+_LOADER_CLS_RE = re.compile('.*azure/cli/command_modules/(?P<module>[^/]*)/__init__.*')
+
+
+def exclude_mods(command_table, help_file_entries, module_exclusions):
+    return _filter_mods(command_table, help_file_entries, module_exclusions, True)
+
+
+def include_mods(command_table, help_file_entries, module_inclusions):
+    return _filter_mods(command_table, help_file_entries, module_inclusions, False)
+
+
+def _filter_mods(command_table, help_file_entries, modules, exclude):
+    from ..utilities.path import get_command_modules_paths
+
+    command_modules_paths = get_command_modules_paths()
+    filtered_module_names = {mod for mod, path in command_modules_paths if mod in modules}
+
+    command_table = command_table.copy()
+    help_file_entries = help_file_entries.copy()
+
+    for command_name in list(command_table.keys()):
+        try:
+            mod_name = _get_command_module(command_name, command_table)
+        except LinterError as ex:
+            print(ex)
+            continue
+        if (mod_name in filtered_module_names) == exclude:
+            # brute force method of clearing traces of a module
+            del command_table[command_name]
+            help_file_entries.pop(command_name, None)
+            for group_name in get_command_groups(command_name):
+                help_file_entries.pop(group_name, None)
+
+    return command_table, help_file_entries
+
+
+def share_element(first_iter, second_iter):
+    return any(element in first_iter for element in second_iter)
+
+
+def get_command_groups(command_name):
+    command_args = []
+    for arg in command_name.split()[:-1]:
+        command_args.append(arg)
+        if command_args:
+            yield ' '.join(command_args)
+
+
+def _get_command_module(command_name, command_table):
+    # hacky way to get a command's module
+    command = command_table.get(command_name)
+    loader_cls = command.loader.__class__
+    loader_file_path = inspect.getfile(loader_cls)
+    # normalize os path to '/' for regex
+    loader_file_path = '/'.join(loader_file_path.split(os.path.sep))
+    match = _LOADER_CLS_RE.match(loader_file_path)
+    # loader class path is consistent due to convention, throw error if no match
+    # this will likely throw error for extensions
+    if not match:
+        raise LinterError('`{}`\'s loader class path does not match regex pattern: {}' \
+            .format(command_name, _LOADER_CLS_RE.pattern))
+    return match.groupdict().get('module')
+
+
+class LinterError(Exception):
+    """
+    Exception thrown by linter for non rule violation reasons
+    """
+    pass