From 381d1f792ca4e1807ca0dd97b447e1419aeddcd8 Mon Sep 17 00:00:00 2001 From: AllyW Date: Mon, 30 Dec 2024 15:10:06 +0800 Subject: [PATCH] add example missing check (#500) * add example missing check --- azdev/operations/constant.py | 7 +- .../linter/data/cmd_example_config.json | 10 +++ azdev/operations/linter/linter.py | 74 ++++++++++++++++++- .../linter/rules/command_coverage_rules.py | 8 ++ azdev/operations/linter/util.py | 28 ++++++- azdev/operations/regex.py | 28 +++++++ setup.py | 1 + 7 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 azdev/operations/linter/data/cmd_example_config.json diff --git a/azdev/operations/constant.py b/azdev/operations/constant.py index 72d785f2..cadda981 100644 --- a/azdev/operations/constant.py +++ b/azdev/operations/constant.py @@ -4,7 +4,7 @@ # license information. # ----------------------------------------------------------------------------- # pylint: disable=line-too-long - +import os ENCODING = 'utf-8' # Base on https://github.com/Azure/azure-cli/blob/dev/.github/CODEOWNERS @@ -259,3 +259,8 @@ PREVIEW_INIT_SUFFIX = "b1" CLI_EXTENSION_INDEX_URL = "https://azcliextensionsync.blob.core.windows.net/index1/index.json" + +CMD_EXAMPLE_CONFIG_FILE = "./data/cmd_example_config.json" +CMD_EXAMPLE_CONFIG_FILE_PATH = f"{os.path.dirname(os.path.realpath(__file__))}/linter/{CMD_EXAMPLE_CONFIG_FILE}" +CMD_EXAMPLE_CONFIG_FILE_URL = "https://azcmdchangemgmt.blob.core.windows.net/azure-cli-dev-tool-config/cmd_example_config.json" +CMD_EXAMPLE_DEFAULT = 1 diff --git a/azdev/operations/linter/data/cmd_example_config.json b/azdev/operations/linter/data/cmd_example_config.json new file mode 100644 index 00000000..0c0d7f5d --- /dev/null +++ b/azdev/operations/linter/data/cmd_example_config.json @@ -0,0 +1,10 @@ +{ + "create": 1, + "add": 1, + "update": 1, + "list": 0, + "delete": 0, + "remove": 0, + "show": 0, + "wait": 0 +} \ No newline at end of file diff --git a/azdev/operations/linter/linter.py b/azdev/operations/linter/linter.py index 2e163b0c..6f48947b 100644 --- a/azdev/operations/linter/linter.py +++ b/azdev/operations/linter/linter.py @@ -3,7 +3,7 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # ----------------------------------------------------------------------------- - +# pylint: disable=line-too-long from difflib import context_diff from enum import Enum from importlib import import_module @@ -17,6 +17,7 @@ from azdev.operations.regex import ( get_all_tested_commands_from_regex, + search_aaz_raw_command, search_aaz_custom_command, search_argument, search_argument_context, search_command, @@ -24,7 +25,8 @@ search_command_group) from azdev.utilities import diff_branches_detail, diff_branch_file_patch from azdev.utilities.path import get_cli_repo_path, get_ext_repo_paths -from .util import share_element, exclude_commands, LinterError +from .util import (share_element, exclude_commands, LinterError, get_cmd_example_configurations, + get_cmd_example_threshold) PACKAGE_NAME = 'azdev.operations.linter' _logger = get_logger(__name__) @@ -224,6 +226,30 @@ def get_parameter_test_coverage(self): all_tested_command = self._detect_tested_command(diff_index) return self._run_parameter_test_coverage(parameters, all_tested_command) + def check_missing_command_example(self): + _exclude_commands = self._get_cmd_exclusions(rule_name="missing_command_example") + cmd_example_config = get_cmd_example_configurations() + commands = self._detect_modified_command() + violations = [] + for cmd in commands: + if cmd in _exclude_commands: + continue + cmd_help = self._loaded_help.get(cmd, None) + if not cmd_help: + continue + # parameters = cmd_help.parameters + # add if future parameter set required + cmd_suffix = cmd.split()[-1] + cmd_example_threshold = get_cmd_example_threshold(cmd_suffix, cmd_example_config) + if cmd_example_threshold == 0: + continue + if not hasattr(cmd_help, "examples") or len(cmd_help.examples) < cmd_example_threshold: + violations.append(f'Command `{cmd}` should have at least {cmd_example_threshold} example(s)') + if violations: + violations.insert(0, 'Check command example failed.') + violations.extend(['Please add examples for the modified command or add the command in rule_exclusions: missing_command_example in linter_exclusions.yml']) + return violations + def _get_exclusions(self): _exclude_commands = set() _exclude_parameters = set() @@ -238,6 +264,16 @@ def _get_exclusions(self): _logger.debug('exclude_comands: %s', _exclude_commands) return _exclude_commands, _exclude_parameters + def _get_cmd_exclusions(self, rule_name=None): + _exclude_commands = set() + if not rule_name: + return _exclude_commands + for command, details in self.exclusions.items(): + if 'rule_exclusions' in details and rule_name in details['rule_exclusions']: + _exclude_commands.add(command) + _logger.debug('exclude_commands: %s', _exclude_commands) + return _exclude_commands + def _split_path(self, path: str): parts = path.rsplit('/', maxsplit=1) return parts if len(parts) == 2 else ('', parts[0]) @@ -387,6 +423,40 @@ def _run_parameter_test_coverage(parameters, all_tested_command): 'Or add the parameter with missing_parameter_test_coverage rule in linter_exclusions.yml']) return exec_state, violations + def _detect_modified_command(self): + modified_commands = set() + diff_patches = diff_branch_file_patch(repo=self.git_repo, target=self.git_target, source=self.git_source) + for change in diff_patches: + file_path, filename = self._split_path(change.a_path) + if "commands.py" not in filename and "aaz" not in file_path: + continue + current_lines = self._read_blob_lines(change.b_blob) + patch = change.diff.decode("utf-8") + patch_lines = patch.splitlines() + if 'commands.py' in filename: + added_lines = [line for line in patch_lines if line.startswith('+') and not line.startswith('+++')] + for line in added_lines: + if aaz_custom_command := search_aaz_custom_command(line): + modified_commands.add(aaz_custom_command) + + for row_num, line in enumerate(patch_lines): + if not line.startswith("+") or line.startswith('+++'): + continue + manual_command_suffix = search_command(line) + if manual_command_suffix: + idx = self._get_line_number(patch_lines, row_num, r'@@ -(\d+),(?:\d+) \+(?:\d+),(?:\d+) @@') + manual_command = search_command_group(idx, current_lines, manual_command_suffix) + if manual_command: + modified_commands.add(manual_command) + + if "aaz" in file_path: + if aaz_raw_command := search_aaz_raw_command(patch): + modified_commands.add(aaz_raw_command) + + commands = list(modified_commands) + _logger.debug('Modified commands: %s', modified_commands) + return commands + def _get_diffed_patches(self): if not self.git_source or not self.git_target or not self.git_repo: return diff --git a/azdev/operations/linter/rules/command_coverage_rules.py b/azdev/operations/linter/rules/command_coverage_rules.py index f52a0246..20b7de6a 100644 --- a/azdev/operations/linter/rules/command_coverage_rules.py +++ b/azdev/operations/linter/rules/command_coverage_rules.py @@ -22,3 +22,11 @@ def missing_parameter_test_coverage(linter): if not exec_state: violation_msg = "\n\t".join(violations) raise RuleError(violation_msg + "\n") + + +@CommandCoverageRule(LinterSeverity.HIGH) +def missing_command_example(linter): + violations = linter.check_missing_command_example() + if violations: + violation_msg = "\n\t".join(violations) + raise RuleError(violation_msg + "\n") diff --git a/azdev/operations/linter/util.py b/azdev/operations/linter/util.py index b3e70d34..3551cb06 100644 --- a/azdev/operations/linter/util.py +++ b/azdev/operations/linter/util.py @@ -6,12 +6,15 @@ import copy import re +import os +import json import requests from knack.log import get_logger from azdev.utilities import get_name_index -from azdev.operations.constant import ALLOWED_HTML_TAG +from azdev.operations.constant import (ALLOWED_HTML_TAG, CMD_EXAMPLE_CONFIG_FILE_URL, + CMD_EXAMPLE_CONFIG_FILE_PATH, CMD_EXAMPLE_DEFAULT) logger = get_logger(__name__) @@ -155,3 +158,26 @@ def has_broken_site_links(help_message, filtered_lines=None): if filtered_lines: invalid_urls = [s for s in invalid_urls if any(s in diff_line for diff_line in filtered_lines)] return invalid_urls + + +def get_cmd_example_configurations(): + cmd_example_threshold = {} + remote_res = requests.get(CMD_EXAMPLE_CONFIG_FILE_URL) + if remote_res.status_code != 200: + logger.warning("remote cmd example configuration fetch error, use local dict") + if not os.path.exists(CMD_EXAMPLE_CONFIG_FILE_PATH): + logger.info("cmd_example_config.json not exist, skipped") + return cmd_example_threshold + with open(CMD_EXAMPLE_CONFIG_FILE_PATH, "r") as f_in: + cmd_example_threshold = json.load(f_in) + else: + logger.info("remote cmd example configuration fetch success") + cmd_example_threshold = remote_res.json() + return cmd_example_threshold + + +def get_cmd_example_threshold(cmd_suffix, cmd_example_config): + for cmd_type, threshold in cmd_example_config.items(): + if cmd_suffix.find(cmd_type) != -1: + return threshold + return CMD_EXAMPLE_DEFAULT diff --git a/azdev/operations/regex.py b/azdev/operations/regex.py index 1bd3c164..6f566f16 100644 --- a/azdev/operations/regex.py +++ b/azdev/operations/regex.py @@ -173,3 +173,31 @@ def search_deleted_command(line): if ref: command = ref[0].split(',')[0].strip("'") return command + + +def search_aaz_custom_command(line): + """ + re match pattern + + self.command_table['monitor autoscale update'] = AutoScaleUpdate(loader=self) + """ + cmd = '' + aaz_custom_cmd_pattern = r"\+.*\.command_table\[['\"](.*?)['\"]\]" + ref = re.findall(aaz_custom_cmd_pattern, line) + if ref: + cmd = ref[0].strip() + return cmd + + +def search_aaz_raw_command(lines): + """ + re match pattern + +@register_command( + + "monitor autoscale update", + +) + """ + cmd = '' + aaz_raw_cmd_pattern = r"\+@register_command\([\s\S]*?\+.*?['\"](.*?)['\"]" + ref = re.findall(aaz_raw_cmd_pattern, str(lines)) + if ref: + cmd = ref[0].strip() + return cmd diff --git a/setup.py b/setup.py index 7ca25582..4c4c8794 100644 --- a/setup.py +++ b/setup.py @@ -93,6 +93,7 @@ 'azdev.config': ['*.*', 'cli_pylintrc', 'ext_pylintrc'], 'azdev.mod_templates': ['*.*'], 'azdev.operations.linter.rules': ['ci_exclusions.yml'], + 'azdev.operations.linter': ["data/*"], 'azdev.operations.cmdcov': ['*.*'], 'azdev.operations.breaking_change': ['*.*'], },