From 3671f48eae416b4d774563e3225b5b1698c57755 Mon Sep 17 00:00:00 2001 From: AllyW Date: Thu, 28 Nov 2024 18:22:12 +0800 Subject: [PATCH 1/2] add cmd example change --- azdev/commands.py | 1 + azdev/help.py | 9 +- azdev/operations/command_change/__init__.py | 18 ++- azdev/operations/command_change/custom.py | 56 ++++++- azdev/operations/command_change/util.py | 21 +++ azdev/operations/linter/__init__.py | 8 +- azdev/operations/linter/linter.py | 36 ++++- azdev/operations/linter/rule_decorators.py | 23 ++- .../linter/rules/extra_cli_linter_rules.py | 22 +++ azdev/operations/linter/util.py | 20 +++ .../tests/jsons/az_util_meta_after.json | 143 ++++++++++++++++++ .../tests/jsons/az_util_meta_before.json | 106 +++++++++++++ azdev/operations/tests/test_break_change.py | 10 +- azdev/params.py | 8 +- 14 files changed, 467 insertions(+), 14 deletions(-) create mode 100644 azdev/operations/linter/rules/extra_cli_linter_rules.py create mode 100644 azdev/operations/tests/jsons/az_util_meta_after.json create mode 100644 azdev/operations/tests/jsons/az_util_meta_before.json diff --git a/azdev/commands.py b/azdev/commands.py index 53974827f..d468383e9 100644 --- a/azdev/commands.py +++ b/azdev/commands.py @@ -46,6 +46,7 @@ def operation_group(name): g.command('meta-export', 'export_command_meta') g.command('meta-diff', 'cmp_command_meta') g.command('tree-export', 'export_command_tree') + g.command('cmd-example-diff', 'meta_command_example_diff') with CommandGroup(self, 'cli', operation_group('pypi')) as g: g.command('check-versions', 'verify_versions') diff --git a/azdev/help.py b/azdev/help.py index 1be1ca29c..8b09e9665 100644 --- a/azdev/help.py +++ b/azdev/help.py @@ -243,7 +243,7 @@ short-summary: Diff the command meta between provided meta files. examples: - name: Diff the command meta change from fileA to fileB - text: azdev statistics meta-diff --base-meta-file fileA --diff-meta-file fileB --only-break + text: azdev command-change meta-diff --base-meta-file fileA --diff-meta-file fileB --only-break """ helps['command-change tree-export'] = """ @@ -253,6 +253,13 @@ text: azdev command-change tree-export CLI --output-file command_tree.json """ +helps['command-change cmd-example-diff'] = """ + short-summary: Check the command example number between provided meta files. + examples: + - name: Check the command example number from fileA to fileB + text: azdev command-change cmd-example-diff --base-meta-file fileA --diff-meta-file fileB --only-break +""" + helps['perf'] = """ short-summary: Commands to test CLI performance. """ diff --git a/azdev/operations/command_change/__init__.py b/azdev/operations/command_change/__init__.py index da3f0a36e..163407e9a 100644 --- a/azdev/operations/command_change/__init__.py +++ b/azdev/operations/command_change/__init__.py @@ -7,12 +7,14 @@ # pylint: disable=no-else-return, too-many-nested-blocks, too-many-locals, too-many-branches import time +import os +import json from knack.log import get_logger import azure_cli_diff_tool from azdev.utilities import display, require_azure_cli, heading, get_path_table, filter_by_git_diff, \ calc_selected_mod_names -from .custom import DiffExportFormat, get_commands_meta, STORED_DEPRECATION_KEY +from .custom import DiffExportFormat, get_commands_meta, STORED_DEPRECATION_KEY, command_example_diff from .util import export_commands_meta, dump_command_tree, add_to_command_tree from ..statistics import _create_invoker_and_load_cmds, _get_command_source, \ _command_codegen_info # pylint: disable=protected-access @@ -178,3 +180,17 @@ def export_command_tree(modules, output_file=None): add_to_command_tree(command_tree, command_name, module_source) dump_command_tree(command_tree, output_file) + + +def meta_command_example_diff(base_meta_file, diff_meta_file): + if not os.path.exists(base_meta_file) and not os.path.exists(diff_meta_file): + raise Exception("base and/or diff meta file needed") + command_tree_before = {} + command_tree_after = {} + if os.path.exists(base_meta_file): + with open(base_meta_file, "r") as g: + command_tree_before = json.load(g) + if os.path.exists(diff_meta_file): + with open(diff_meta_file, "r") as g: + command_tree_after = json.load(g) + return command_example_diff(command_tree_before, command_tree_after) diff --git a/azdev/operations/command_change/custom.py b/azdev/operations/command_change/custom.py index 723810068..e18a3ed23 100644 --- a/azdev/operations/command_change/custom.py +++ b/azdev/operations/command_change/custom.py @@ -7,7 +7,7 @@ from enum import Enum from knack.log import get_logger -from .util import get_command_tree +from .util import get_command_tree, search_cmd_obj, ChangeType logger = get_logger(__name__) @@ -241,3 +241,57 @@ def get_commands_meta(command_group_table, commands_info, with_help, with_exampl command_group_info["commands"][command_name] = command_meta break return commands_meta + + +def command_example_diff(base_meta=None, diff_meta=None): + diff_cmds = [] + find_command_remove(base_meta, diff_meta, diff_cmds) + check_command_add(diff_meta, diff_cmds) + add_cmd_example = [] + for cmd_name, diff_type in diff_cmds: + if diff_type == ChangeType.REMOVE: + continue + diff_cmd_obj = search_cmd_obj(cmd_name, diff_meta) + add_cmd_example.append({ + "cmd": cmd_name, + "param_count": len(diff_cmd_obj["parameters"]) if "parameters" in diff_cmd_obj else 0, + "example_count": len(diff_cmd_obj["examples"]) if "examples" in diff_cmd_obj else 0 + }) + return add_cmd_example + + +def find_command_remove(base_meta, diff_meta, diff_cmds): + if "sub_groups" in base_meta: + for sub_group, group_item in base_meta["sub_groups"].items(): + find_command_remove(group_item, diff_meta, diff_cmds) + if "commands" in base_meta: + generate_command_remove(base_meta["commands"], diff_meta, diff_cmds) + + +def generate_command_remove(base_cmd_items, diff_meta, diff_cmds): + for cmd_name, cmd_obj in base_cmd_items.items(): + if cmd_obj.get("checked", None): + continue + cmd_obj["checked"] = True + diff_cmd_obj = search_cmd_obj(cmd_name, diff_meta) + if diff_cmd_obj is None: + # cmd lost + diff_cmds.append((cmd_name, ChangeType.REMOVE)) + continue + diff_cmd_obj["checked"] = True + + +def check_command_add(diff_meta, diff_cmds): + if "sub_groups" in diff_meta: + for sub_group, group_item in diff_meta["sub_groups"].items(): + check_command_add(group_item, diff_cmds) + if "commands" in diff_meta: + generate_command_add(diff_meta["commands"], diff_cmds) + + +def generate_command_add(diff_cmd_items, diff_cmds): + for cmd_name, cmd_obj in diff_cmd_items.items(): + if cmd_obj.get("checked", None): + continue + diff_cmds.append((cmd_name, ChangeType.ADD)) + cmd_obj["checked"] = True diff --git a/azdev/operations/command_change/util.py b/azdev/operations/command_change/util.py index cb198ac33..d5b830569 100644 --- a/azdev/operations/command_change/util.py +++ b/azdev/operations/command_change/util.py @@ -61,6 +61,27 @@ def get_command_tree(command_name): return ret +def search_cmd_obj(cmd_name, search_meta): + command_tree = get_command_tree(cmd_name) + meta_search = search_meta + while True: + if "is_group" not in command_tree: + break + if command_tree["is_group"]: + group_name = command_tree["group_name"] + if group_name not in meta_search["sub_groups"]: + return None + meta_search = meta_search["sub_groups"][group_name] + command_tree = command_tree["sub_info"] + else: + cmd_name = command_tree["cmd_name"] + if cmd_name not in meta_search["commands"]: + return None + meta_search = meta_search["commands"][cmd_name] + break + return meta_search + + def export_commands_meta(commands_meta, meta_output_path=None): options = jsbeautifier.default_options() options.indent_size = 4 diff --git a/azdev/operations/linter/__init__.py b/azdev/operations/linter/__init__.py index 18183af69..a333ac07e 100644 --- a/azdev/operations/linter/__init__.py +++ b/azdev/operations/linter/__init__.py @@ -29,7 +29,7 @@ # pylint:disable=too-many-locals, too-many-statements, too-many-branches def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None, git_source=None, git_target=None, git_repo=None, include_whl_extensions=False, - min_severity=None, save_global_exclusion=False): + min_severity=None, save_global_exclusion=False, base_meta_path=None, diff_meta_path=None): require_azure_cli() @@ -155,7 +155,10 @@ def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None, update_global_exclusion=update_global_exclusion, git_source=git_source, git_target=git_target, - git_repo=git_repo) + git_repo=git_repo, + base_meta_path=base_meta_path, + diff_meta_path=diff_meta_path + ) subheading('Results') logger.info('Running linter: %i commands, %i help entries', @@ -166,6 +169,7 @@ def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None, run_command_groups=not rule_types or 'command_groups' in rule_types, run_help_files_entries=not rule_types or 'help_entries' in rule_types, run_command_test_coverage=not rule_types or 'command_test_coverage' in rule_types, + run_extra_cli_linter=not rule_types or "extra_cli_linter" in rule_types, ) display(os.linesep + 'Run custom pylint rules.') exit_code += pylint_rules(selected_modules) diff --git a/azdev/operations/linter/linter.py b/azdev/operations/linter/linter.py index d5bfe41b0..c18be05e1 100644 --- a/azdev/operations/linter/linter.py +++ b/azdev/operations/linter/linter.py @@ -20,9 +20,10 @@ search_argument_context, search_command, search_command_group) +from azdev.operations.command_change import meta_command_example_diff 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, add_cmd_example_justification PACKAGE_NAME = 'azdev.operations.linter' _logger = get_logger(__name__) @@ -47,7 +48,7 @@ def get_ordered_members(): class Linter: # pylint: disable=too-many-public-methods, too-many-instance-attributes def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None, git_source=None, git_target=None, - git_repo=None, exclusions=None): + git_repo=None, exclusions=None, base_meta_path=None, diff_meta_path=None): self._all_yaml_help = help_file_entries self._loaded_help = loaded_help self._command_loader = command_loader @@ -63,6 +64,8 @@ def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None self.git_target = git_target self.git_repo = git_repo self.exclusions = exclusions + self.base_meta_path = base_meta_path + self.diff_meta_path = diff_meta_path self.diffed_lines = set() self._get_diffed_patches() @@ -371,6 +374,21 @@ 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 check_examples_from_added_command(self): + if not self.diff_meta_path: + return None + added_cmd_example_counts = [] + for root, _, files in os.walk(self.diff_meta_path): + for file_name in files: + diff_metadata_file = os.path.join(root, file_name) + base_metadata_file = os.path.join(self.base_meta_path, file_name) + if not os.path.exists(diff_metadata_file): + continue + added_cmd_example_counts += meta_command_example_diff(base_metadata_file, diff_metadata_file) + added_cmd_example_counts = list(map(add_cmd_example_justification, added_cmd_example_counts)) + filtered_cmd_example = list(filter(lambda x: not x or not x["valid"], added_cmd_example_counts)) + return filtered_cmd_example + def _get_diffed_patches(self): if not self.git_source or not self.git_target or not self.git_repo: return @@ -387,17 +405,20 @@ def _get_diffed_patches(self): # pylint: disable=too-many-instance-attributes class LinterManager: - _RULE_TYPES = {'help_file_entries', 'command_groups', 'commands', 'params', 'command_test_coverage'} + _RULE_TYPES = {'help_file_entries', 'command_groups', 'commands', 'params', 'command_test_coverage', + 'extra_cli_linter'} def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None, exclusions=None, rule_inclusions=None, use_ci_exclusions=None, min_severity=None, update_global_exclusion=None, - git_source=None, git_target=None, git_repo=None): + git_source=None, git_target=None, git_repo=None, base_meta_path=None, diff_meta_path=None): # default to running only rules of the highest severity self.min_severity = min_severity or LinterSeverity.get_ordered_members()[-1] self._exclusions = exclusions or {} self.linter = Linter(command_loader=command_loader, help_file_entries=help_file_entries, loaded_help=loaded_help, git_source=git_source, git_target=git_target, git_repo=git_repo, - exclusions=self._exclusions) + exclusions=self._exclusions, + base_meta_path=base_meta_path, + diff_meta_path=diff_meta_path) self._rules = {rule_type: {} for rule_type in LinterManager._RULE_TYPES} # initialize empty rules self._ci_exclusions = {} self._rule_inclusions = rule_inclusions @@ -440,7 +461,7 @@ 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, run_command_test_coverage=None): + run_help_files_entries=None, run_command_test_coverage=None, run_extra_cli_linter=None): paths = import_module('{}.rules'.format(PACKAGE_NAME)).__path__ if paths: @@ -478,6 +499,9 @@ def run(self, run_params=None, run_commands=None, run_command_groups=None, if run_command_test_coverage and self._rules.get('command_test_coverage'): self._run_rules('command_test_coverage') + if run_extra_cli_linter and self._rules.get("extra_cli_linter"): + self._run_rules("extra_cli_linter") + if not self.exit_code: print(os.linesep + 'No violations found for linter rules.') diff --git a/azdev/operations/linter/rule_decorators.py b/azdev/operations/linter/rule_decorators.py index cbaa4589b..8d686057c 100644 --- a/azdev/operations/linter/rule_decorators.py +++ b/azdev/operations/linter/rule_decorators.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 knack.util import CLIError from .linter import RuleError, LinterSeverity @@ -86,6 +86,27 @@ def wrapper(): return add_to_linter +class ExtraCliLinterRule(BaseRule): + + def __call__(self, func): + def add_to_linter(linter_manager): + def wrapper(): + linter = linter_manager.linter + try: + func(linter) + except RuleError as ex: + linter_manager.mark_rule_failure(self.severity) + yield (_create_violation_msg(ex, 'Repo: {}, Src Branch: {}, Target Branch: {}, base meta path: {}, diff meta path: {} \n', + linter.git_repo, linter.git_source, linter.git_target, linter.base_meta_path, linter.diff_meta_path), + (linter.base_meta_path, linter.diff_meta_path), + func.__name__) + + linter_manager.add_rule('extra_cli_linter', func.__name__, wrapper, self.severity) + + add_to_linter.linter_rule = True + return add_to_linter + + def _get_decorator(func, rule_group, print_format, severity): def add_to_linter(linter_manager): def wrapper(): diff --git a/azdev/operations/linter/rules/extra_cli_linter_rules.py b/azdev/operations/linter/rules/extra_cli_linter_rules.py new file mode 100644 index 000000000..e17c4f99b --- /dev/null +++ b/azdev/operations/linter/rules/extra_cli_linter_rules.py @@ -0,0 +1,22 @@ +# ----------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# ----------------------------------------------------------------------------- + +from ..rule_decorators import ExtraCliLinterRule +from ..linter import RuleError, LinterSeverity + +CMD_EXAMPLE_VIOLATE_MESSAGE = " cmd: {} should have as least {} examples, while {} detected" + + +@ExtraCliLinterRule(LinterSeverity.HIGH) +def missing_examples_from_added_command(linter): + filtered_cmd_example = linter.check_examples_from_added_command() + + def format_cmd_example_violation_message(cmd_obj): + return CMD_EXAMPLE_VIOLATE_MESSAGE.format(cmd_obj["cmd"], cmd_obj["min_example_count"], + cmd_obj["example_count"]) + if filtered_cmd_example: + violation_msg = "\n".join(list(map(format_cmd_example_violation_message, filtered_cmd_example))) + raise RuleError(violation_msg + "\n") diff --git a/azdev/operations/linter/util.py b/azdev/operations/linter/util.py index b3e70d34a..c3fcce02f 100644 --- a/azdev/operations/linter/util.py +++ b/azdev/operations/linter/util.py @@ -155,3 +155,23 @@ 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 is_substring_in_target(string_list, target): + return any(s in target for s in string_list) + + +# pylint: disable=line-too-long +def add_cmd_example_justification(cmd_example_count): + if "cmd" not in cmd_example_count or "param_count" not in cmd_example_count or\ + "example_count" not in cmd_example_count: + return None + cmd_name_arr = cmd_example_count["cmd"].split() + if is_substring_in_target(["add", "create", "update"], cmd_name_arr[-1]): + cmd_example_count["min_example_count"] = max(2, cmd_example_count["param_count"] / 5) + elif is_substring_in_target(["show", "list", "delete"], cmd_name_arr[-1]): + cmd_example_count["min_example_count"] = 1 + else: + cmd_example_count["min_example_count"] = 1 + cmd_example_count["valid"] = True if cmd_example_count["min_example_count"] <= cmd_example_count["example_count"] else False + return cmd_example_count diff --git a/azdev/operations/tests/jsons/az_util_meta_after.json b/azdev/operations/tests/jsons/az_util_meta_after.json new file mode 100644 index 000000000..15e2e9000 --- /dev/null +++ b/azdev/operations/tests/jsons/az_util_meta_after.json @@ -0,0 +1,143 @@ +{ + "module_name": "util", + "name": "az", + "commands": { + "rest": { + "name": "rest", + "is_aaz": false, + "examples": [{ + "name": "Get Audit log through Microsoft Graph", + "text": "az rest --method get --url https://graph.microsoft.com/beta/auditLogs/directoryAudits\n" + }, { + "name": "Update a Azure Active Directory Graph User's display name", + "text": "(Bash or CMD)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body \"{\\\"displayName\\\": \\\"johndoe2\\\"}\"\n\n(Bash)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body '{\"displayName\": \"johndoe2\"}'\n\n(PowerShell)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body '{\\\"displayName\\\": \\\"johndoe2\\\"}'\n" + }, { + "name": "Get a virtual machine", + "text": "az rest --method get --uri /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/virtualMachines/{vmName}?api-version=2019-03-01\n" + }, { + "name": "Create a public IP address from body.json file", + "text": "az rest --method put --url https://management.azure.com/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/publicIPAddresses/{publicIpAddressName}?api-version=2019-09-01 --body @body.json\n" + }, { + "name": "List the top three resources (Bash)", + "text": "az rest --method get --url https://management.azure.com/subscriptions/{subscriptionId}/resources?api-version=2019-07-01 --url-parameters \\$top=3\n" + }], + "parameters": [{ + "name": "url", + "options": ["--uri", "--url", "-u"], + "required": true + }, { + "name": "method", + "options": ["--method", "-m"], + "choices": ["delete", "get", "head", "options", "patch", "post", "put"], + "default": "get" + }, { + "name": "headers", + "options": ["--headers"], + "nargs": "+" + }, { + "name": "uri_parameters", + "options": ["--uri-parameters", "--url-parameters"], + "nargs": "+" + }, { + "name": "body", + "options": ["--body", "-b"] + }, { + "name": "skip_authorization_header", + "options": ["--skip-authorization-header"] + }, { + "name": "resource", + "options": ["--resource"] + }, { + "name": "output_file", + "options": ["--output-file"] + }] + }, + "version": { + "name": "version", + "is_aaz": false, + "parameters": [] + }, + "upgrade": { + "name": "upgrade", + "is_aaz": false, + "is_preview": true, + "examples": [{ + "name": "example1", + "text": "az upgrade -x aaa" + }, { + "name": "example1", + "text": "az upgrade -y bbb" + }], + "parameters": [{ + "name": "update_all", + "options": ["--all"], + "choices": ["false", "true"], + "nargs": "?", + "default": "true" + }, { + "name": "yes", + "options": ["--yes", "-y"] + }, { + "name": "allow_preview", + "options": ["--allow-preview", "--allow-preview-extensions"], + "choices": ["false", "true"], + "nargs": "?" + }] + } + }, + "sub_groups": { + "demo": { + "name": "demo", + "commands": { + "demo byo-access-token": { + "name": "demo byo-access-token", + "is_aaz": false, + "examples": [{ + "name": "List resource groups by bringing your own access token", + "text": "az demo byo-access-token --access-token \"eyJ0eXAiO...\" --subscription-id 00000000-0000-0000-0000-000000000000" + }], + "parameters": [{ + "name": "access_token", + "options": ["--access-token"], + "required": true + }, { + "name": "subscription_id", + "options": ["--subscription-id"], + "required": true + }] + } + }, + "sub_groups": { + "demo secret-store": { + "name": "demo secret-store", + "commands": { + "demo secret-store save": { + "name": "demo secret-store save", + "is_aaz": false, + "examples": [{ + "name": "Save data to secret store.", + "text": "az demo secret-store save \"name=Johann Sebastian Bach\" job=musician" + }], + "parameters": [{ + "name": "key_value", + "options": [], + "required": true, + "nargs": "+" + }] + }, + "demo secret-store load": { + "name": "demo secret-store load", + "is_aaz": false, + "parameters": [] + } + }, + "sub_groups": {} + } + }, + "deprecate_info": { + "target": "demo", + "hide": true + } + } + } +} \ No newline at end of file diff --git a/azdev/operations/tests/jsons/az_util_meta_before.json b/azdev/operations/tests/jsons/az_util_meta_before.json new file mode 100644 index 000000000..8e11ee5bf --- /dev/null +++ b/azdev/operations/tests/jsons/az_util_meta_before.json @@ -0,0 +1,106 @@ +{ + "module_name": "util", + "name": "az", + "commands": { + "rest": { + "name": "rest", + "is_aaz": false, + "examples": [{ + "name": "Get Audit log through Microsoft Graph", + "text": "az rest --method get --url https://graph.microsoft.com/beta/auditLogs/directoryAudits\n" + }, { + "name": "Update a Azure Active Directory Graph User's display name", + "text": "(Bash or CMD)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body \"{\\\"displayName\\\": \\\"johndoe2\\\"}\"\n\n(Bash)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body '{\"displayName\": \"johndoe2\"}'\n\n(PowerShell)\naz rest --method patch --url \"https://graph.microsoft.com/v1.0/users/johndoe@azuresdkteam.onmicrosoft.com\" --body '{\\\"displayName\\\": \\\"johndoe2\\\"}'\n" + }, { + "name": "Get a virtual machine", + "text": "az rest --method get --uri /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/virtualMachines/{vmName}?api-version=2019-03-01\n" + }, { + "name": "Create a public IP address from body.json file", + "text": "az rest --method put --url https://management.azure.com/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/publicIPAddresses/{publicIpAddressName}?api-version=2019-09-01 --body @body.json\n" + }, { + "name": "List the top three resources (Bash)", + "text": "az rest --method get --url https://management.azure.com/subscriptions/{subscriptionId}/resources?api-version=2019-07-01 --url-parameters \\$top=3\n" + }], + "parameters": [{ + "name": "url", + "options": ["--uri", "--url", "-u"], + "required": true + }, { + "name": "method", + "options": ["--method", "-m"], + "choices": ["delete", "get", "head", "options", "patch", "post", "put"], + "default": "get" + }, { + "name": "headers", + "options": ["--headers"], + "nargs": "+" + }, { + "name": "uri_parameters", + "options": ["--uri-parameters", "--url-parameters"], + "nargs": "+" + }, { + "name": "body", + "options": ["--body", "-b"] + }, { + "name": "skip_authorization_header", + "options": ["--skip-authorization-header"] + }, { + "name": "resource", + "options": ["--resource"] + }, { + "name": "output_file", + "options": ["--output-file"] + }] + } + }, + "sub_groups": { + "demo": { + "name": "demo", + "commands": { + "demo style": { + "name": "demo style", + "is_aaz": false, + "parameters": [{ + "name": "theme", + "options": ["--theme"], + "choices": ["cloud-shell", "dark", "light", "none"] + }] + }, + "demo byo-access-token": { + "name": "demo byo-access-token", + "is_aaz": false, + "examples": [{ + "name": "List resource groups by bringing your own access token", + "text": "az demo byo-access-token --access-token \"eyJ0eXAiO...\" --subscription-id 00000000-0000-0000-0000-000000000000" + }], + "parameters": [{ + "name": "access_token", + "options": ["--access-token"], + "required": true + }, { + "name": "subscription_id", + "options": ["--subscription-id"], + "required": true + }] + } + }, + "sub_groups": { + "demo secret-store": { + "name": "demo secret-store", + "commands": { + "demo secret-store load": { + "name": "demo secret-store load", + "is_aaz": false, + "parameters": [] + } + }, + "sub_groups": {} + } + }, + "deprecate_info": { + "target": "demo", + "hide": true + } + } + } +} \ No newline at end of file diff --git a/azdev/operations/tests/test_break_change.py b/azdev/operations/tests/test_break_change.py index 5d369dc83..c77e681d8 100644 --- a/azdev/operations/tests/test_break_change.py +++ b/azdev/operations/tests/test_break_change.py @@ -7,7 +7,7 @@ import unittest import os -from azdev.operations.command_change import export_command_meta, cmp_command_meta +from azdev.operations.command_change import export_command_meta, cmp_command_meta, meta_command_example_diff from azdev.operations.command_change.util import get_command_tree, add_to_command_tree @@ -73,6 +73,14 @@ def test_command_tree(self): } self.assertDictEqual(tree, expected) + def test_diff_cmd_example(self): + if not os.path.exists("./jsons/az_util_meta_before.json") or \ + not os.path.exists("./jsons/az_util_meta_after.json"): + raise Exception("Test json missing") + result = meta_command_example_diff(base_meta_file="./jsons/az_util_meta_before.json", + diff_meta_file="./jsons/az_util_meta_after.json") + self.assertEqual(len(result), 3) + if __name__ == '__main__': unittest.main() diff --git a/azdev/params.py b/azdev/params.py index 1eb36f8ac..abb24e17f 100644 --- a/azdev/params.py +++ b/azdev/params.py @@ -83,7 +83,7 @@ def load_arguments(self, _): with ArgumentsContext(self, 'linter') as c: c.positional('modules', modules_type) c.argument('rules', options_list=['--rules', '-r'], nargs='+', help='Space-separated list of rules to run. Omit to run all rules.') - c.argument('rule_types', options_list=['--rule-types', '-t'], nargs='+', choices=['params', 'commands', 'command_groups', 'help_entries', 'command_test_coverage'], help='Space-separated list of rule types to run. Omit to run all.') + c.argument('rule_types', options_list=['--rule-types', '-t'], nargs='+', choices=['params', 'commands', 'command_groups', 'help_entries', 'command_test_coverage', 'extra_cli_linter'], help='Space-separated list of rule types to run. Omit to run all.') c.argument('ci_exclusions', action='store_true', help='Force application of CI exclusions list when run locally.') c.argument('include_whl_extensions', action='store_true', @@ -98,6 +98,8 @@ def load_arguments(self, _): 'For example, specifying "medium" runs linter rules that have "high" or "medium" severity. ' 'However, specifying "low" runs the linter on every rule, regardless of severity. ' 'Defaults to "high".') + c.argument('base_meta_path', arg_group='Metadata', help='path to store command meta json file from base branch') + c.argument('diff_meta_path', arg_group='Metadata', help='path to store command meta json file from diff branch') # endregion # region scan & mask @@ -174,6 +176,10 @@ def load_arguments(self, _): c.positional('modules', modules_type) c.argument('output_file', help='command tree json file path to store') + with ArgumentsContext(self, 'command-change cmd-example-diff') as c: + c.argument('base_meta_file', required=True, help='command meta json file') + c.argument('diff_meta_file', required=True, help='command meta json file to diff') + # region cmdcov with ArgumentsContext(self, 'cmdcov') as c: c.positional('modules', modules_type) From f4ca1c0fc6ddbe55d069aeb5f6189a8a3b6dca8c Mon Sep 17 00:00:00 2001 From: AllyW Date: Wed, 11 Dec 2024 11:41:31 +0800 Subject: [PATCH 2/2] add remove --- azdev/operations/command_change/custom.py | 8 ++++++++ azdev/operations/linter/rules/extra_cli_linter_rules.py | 5 +++-- azdev/operations/linter/util.py | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/azdev/operations/command_change/custom.py b/azdev/operations/command_change/custom.py index e18a3ed23..2fe4b07b7 100644 --- a/azdev/operations/command_change/custom.py +++ b/azdev/operations/command_change/custom.py @@ -250,10 +250,18 @@ def command_example_diff(base_meta=None, diff_meta=None): add_cmd_example = [] for cmd_name, diff_type in diff_cmds: if diff_type == ChangeType.REMOVE: + cmd_obj = search_cmd_obj(cmd_name, base_meta) + add_cmd_example.append({ + "cmd": cmd_name, + "change_type": diff_type, + "param_count": len(cmd_obj["parameters"]) if "parameters" in cmd_obj else 0, + "example_count": len(cmd_obj["examples"]) if "examples" in cmd_obj else 0 + }) continue diff_cmd_obj = search_cmd_obj(cmd_name, diff_meta) add_cmd_example.append({ "cmd": cmd_name, + "change_type": diff_type, "param_count": len(diff_cmd_obj["parameters"]) if "parameters" in diff_cmd_obj else 0, "example_count": len(diff_cmd_obj["examples"]) if "examples" in diff_cmd_obj else 0 }) diff --git a/azdev/operations/linter/rules/extra_cli_linter_rules.py b/azdev/operations/linter/rules/extra_cli_linter_rules.py index e17c4f99b..304a40df4 100644 --- a/azdev/operations/linter/rules/extra_cli_linter_rules.py +++ b/azdev/operations/linter/rules/extra_cli_linter_rules.py @@ -7,7 +7,7 @@ from ..rule_decorators import ExtraCliLinterRule from ..linter import RuleError, LinterSeverity -CMD_EXAMPLE_VIOLATE_MESSAGE = " cmd: {} should have as least {} examples, while {} detected" +CMD_EXAMPLE_VIOLATE_MESSAGE = " cmd: `{}` should have as least {} examples, while {} detected" @ExtraCliLinterRule(LinterSeverity.HIGH) @@ -18,5 +18,6 @@ def format_cmd_example_violation_message(cmd_obj): return CMD_EXAMPLE_VIOLATE_MESSAGE.format(cmd_obj["cmd"], cmd_obj["min_example_count"], cmd_obj["example_count"]) if filtered_cmd_example: - violation_msg = "\n".join(list(map(format_cmd_example_violation_message, filtered_cmd_example))) + violation_msg = "Please check following cmd example violation messages: \n" + violation_msg += "\n".join(list(map(format_cmd_example_violation_message, filtered_cmd_example))) raise RuleError(violation_msg + "\n") diff --git a/azdev/operations/linter/util.py b/azdev/operations/linter/util.py index c3fcce02f..813c8943c 100644 --- a/azdev/operations/linter/util.py +++ b/azdev/operations/linter/util.py @@ -10,6 +10,7 @@ from knack.log import get_logger +from azdev.operations.command_change.util import ChangeType from azdev.utilities import get_name_index from azdev.operations.constant import ALLOWED_HTML_TAG @@ -166,6 +167,8 @@ def add_cmd_example_justification(cmd_example_count): if "cmd" not in cmd_example_count or "param_count" not in cmd_example_count or\ "example_count" not in cmd_example_count: return None + if cmd_example_count["change_type"] != ChangeType.ADD: + return None cmd_name_arr = cmd_example_count["cmd"].split() if is_substring_in_target(["add", "create", "update"], cmd_name_arr[-1]): cmd_example_count["min_example_count"] = max(2, cmd_example_count["param_count"] / 5)