diff --git a/tools/automation/cli_linter/__init__.py b/tools/automation/cli_linter/__init__.py index 899ab2046d2..04719bbba94 100644 --- a/tools/automation/cli_linter/__init__.py +++ b/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,7 +53,7 @@ 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)) @@ -64,16 +61,8 @@ def main(args): # 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 diff --git a/tools/automation/cli_linter/linter.py b/tools/automation/cli_linter/linter.py index de73a6d0343..2b98a95636a 100644 --- a/tools/automation/cli_linter/linter.py +++ b/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 diff --git a/tools/automation/cli_linter/rule_decorators.py b/tools/automation/cli_linter/rule_decorators.py index b770aaaf01e..b938c320c9b 100644 --- a/tools/automation/cli_linter/rule_decorators.py +++ b/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: `{}`') diff --git a/tools/automation/cli_linter/rules/ci_exclusions.yml b/tools/automation/cli_linter/rules/ci_exclusions.yml new file mode 100644 index 00000000000..3aed106bd12 --- /dev/null +++ b/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 +... \ No newline at end of file diff --git a/tools/automation/cli_linter/rules/command_group_rules.py b/tools/automation/cli_linter/rules/command_group_rules.py index 31f74547875..b7e3318b528 100644 --- a/tools/automation/cli_linter/rules/command_group_rules.py +++ b/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 diff --git a/tools/automation/cli_linter/rules/command_rules.py b/tools/automation/cli_linter/rules/command_rules.py index 73491412d6c..96ff1cf8ecf 100644 --- a/tools/automation/cli_linter/rules/command_rules.py +++ b/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): diff --git a/tools/automation/cli_linter/rules/parameter_rules.py b/tools/automation/cli_linter/rules/parameter_rules.py index bc4f4b5b5ac..0cf99d41320 100644 --- a/tools/automation/cli_linter/rules/parameter_rules.py +++ b/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): diff --git a/tools/automation/cli_linter/util.py b/tools/automation/cli_linter/util.py new file mode 100644 index 00000000000..4faa13af7c3 --- /dev/null +++ b/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[^/]*)/__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