From b4130a8bbdf9f9152700bf50c26b720390b0c1a2 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 14:48:23 -0500 Subject: [PATCH 01/14] Add quit without saving option. --- awsclilinter/README.md | 3 ++- awsclilinter/awsclilinter/cli.py | 11 ++++++++--- awsclilinter/tests/test_cli.py | 26 ++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/awsclilinter/README.md b/awsclilinter/README.md index a9636356d408..3c71e60f6f96 100644 --- a/awsclilinter/README.md +++ b/awsclilinter/README.md @@ -67,7 +67,8 @@ In interactive mode, you can: - Press `y` to accept the current change - Press `n` to skip the current change - Press `u` to accept all remaining changes -- Press `s` to save and exit (applies all accepted changes so far) +- Press `s` to save and exit +- Press `q` to quit without saving ## Development diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index 5bc7d26ac981..d80091b12db8 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -22,9 +22,9 @@ def get_user_choice(prompt: str) -> str: """Get user input for interactive mode.""" while True: choice = input(prompt).lower().strip() - if choice in ["y", "n", "u", "s"]: + if choice in ["y", "n", "u", "s", "q"]: return choice - print("Invalid choice. Please enter y, n, u, or s.") + print("Invalid choice. Please enter y, n, u, s, or q.") def display_finding(finding: LintFinding, index: int, total: int, script_content: str): @@ -80,7 +80,9 @@ def interactive_mode(findings: List[LintFinding], script_content: str) -> List[L accepted = [] for i, finding in enumerate(findings, 1): display_finding(finding, i, len(findings), script_content) - choice = get_user_choice("\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit: ") + choice = get_user_choice( + "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " + ) if choice == "y": accepted.append(finding) @@ -89,6 +91,9 @@ def interactive_mode(findings: List[LintFinding], script_content: str) -> List[L break elif choice == "s": break + elif choice == "q": + print("Quit without saving.") + sys.exit(0) return accepted diff --git a/awsclilinter/tests/test_cli.py b/awsclilinter/tests/test_cli.py index 8e507a382008..55e2622f925c 100644 --- a/awsclilinter/tests/test_cli.py +++ b/awsclilinter/tests/test_cli.py @@ -174,3 +174,29 @@ def test_interactive_mode_save_and_exit(self, tmp_path): fixed_content = output_file.read_text() # Only first change should be applied since we pressed 's' on the second assert fixed_content.count("--cli-binary-format") == 1 + + def test_interactive_mode_quit(self, tmp_path): + """Test interactive mode with 'q' to quit without saving.""" + script_file = tmp_path / "test.sh" + output_file = tmp_path / "output.sh" + script_file.write_text( + "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" + ) + + with patch( + "sys.argv", + [ + "upgrade-aws-cli", + "--script", + str(script_file), + "--interactive", + "--output", + str(output_file), + ], + ): + with patch("builtins.input", return_value="q"): + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code == 0 + # Output file should not exist since we quit without saving + assert not output_file.exists() From 77e93a98b1331aa81615443fed33549806301e7c Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 14:56:06 -0500 Subject: [PATCH 02/14] Modify the base64 parameter rule to not require file:// in the parameter value. --- awsclilinter/awsclilinter/rules/base64_rule.py | 9 ++++----- awsclilinter/tests/test_rules.py | 13 ------------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/awsclilinter/awsclilinter/rules/base64_rule.py b/awsclilinter/awsclilinter/rules/base64_rule.py index 6da9ad61fcf8..18bbdd015b72 100644 --- a/awsclilinter/awsclilinter/rules/base64_rule.py +++ b/awsclilinter/awsclilinter/rules/base64_rule.py @@ -6,9 +6,8 @@ class Base64BinaryFormatRule(LintRule): - """Detects AWS CLI commands with file:// that need --cli-binary-format. This is a best-effort - attempt at statically detecting the breaking change with how AWS CLI v2 treats binary - parameters.""" + """Detects any AWS CLI command that does not specify the --cli-binary-format. This mitigates + the breaking change with how AWS CLI v2 treats binary parameters.""" @property def name(self) -> str: @@ -23,13 +22,13 @@ def description(self) -> str: ) def check(self, root: SgRoot) -> List[LintFinding]: - """Check for AWS CLI commands with file:// missing --cli-binary-format.""" + """Check for AWS CLI commands missing --cli-binary-format.""" node = root.root() base64_broken_nodes = node.find_all( all=[ {"kind": "command"}, {"pattern": "aws $SERVICE $OPERATION $$$ARGS"}, - {"has": {"kind": "word", "regex": r"\Afile://"}}, + {"kind": "command"}, {"not": {"has": {"kind": "word", "pattern": "--cli-binary-format"}}}, ] ) diff --git a/awsclilinter/tests/test_rules.py b/awsclilinter/tests/test_rules.py index dc886b4b6fb0..74c6ad66afd3 100644 --- a/awsclilinter/tests/test_rules.py +++ b/awsclilinter/tests/test_rules.py @@ -32,16 +32,3 @@ def test_no_detection_with_flag(self): findings = rule.check(root) assert len(findings) == 0 - - def test_no_detection_without_file_protocol(self): - """Test no detection when file:// is not used. Even though the breaking change may - still occur without the use of file://, only the case where file:// is used can be detected - statically.""" - script = ( - "aws secretsmanager put-secret-value --secret-id secret1213 --secret-string secret123" - ) - root = SgRoot(script, "bash") - rule = Base64BinaryFormatRule() - findings = rule.check(root) - - assert len(findings) == 0 From 041169519c6687cb3971a56ffa283003bd38fba0 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 15:40:40 -0500 Subject: [PATCH 03/14] Test for multiple rules --- awsclilinter/awsclilinter/cli.py | 93 ++++++++++++++----- awsclilinter/awsclilinter/linter.py | 30 ++++-- awsclilinter/awsclilinter/rules/base.py | 9 +- .../awsclilinter/rules/base64_rule.py | 6 +- .../awsclilinter/rules/copy_props_rule.py | 51 ++++++++++ awsclilinter/tests/test_linter.py | 18 ++-- 6 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 awsclilinter/awsclilinter/rules/copy_props_rule.py diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index d80091b12db8..d921944eb11b 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -2,10 +2,10 @@ import difflib import sys from pathlib import Path -from typing import List +from typing import Dict, List, Tuple from awsclilinter.linter import ScriptLinter -from awsclilinter.rules import LintFinding +from awsclilinter.rules import LintFinding, LintRule from awsclilinter.rules.base64_rule import Base64BinaryFormatRule # ANSI color codes @@ -75,19 +75,59 @@ def display_finding(finding: LintFinding, index: int, total: int, script_content print(line, end="") -def interactive_mode(findings: List[LintFinding], script_content: str) -> List[LintFinding]: - """Run interactive mode and return accepted findings.""" - accepted = [] - for i, finding in enumerate(findings, 1): - display_finding(finding, i, len(findings), script_content) +def interactive_mode( + findings_with_rules: List[Tuple[LintFinding, LintRule]], + script_content: str, + linter: ScriptLinter, +) -> Tuple[str, bool]: + """Run interactive mode with cursor-based workflow. + + Returns: + Tuple of (modified_script, changes_made) + """ + current_script = script_content + cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} + changes_made = False + i = 0 + + while i < len(findings_with_rules): + finding, rule = findings_with_rules[i] + + # Refresh finding if cursor has moved past it + if finding.edit.start_pos < cursors[rule.name]: + refreshed = linter.refresh_finding(current_script, rule, cursors[rule.name]) + if refreshed: + finding = refreshed + findings_with_rules[i] = (finding, rule) + else: + # No more findings for this rule + i += 1 + continue + + display_finding(finding, i + 1, len(findings_with_rules), current_script) choice = get_user_choice( "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " ) if choice == "y": - accepted.append(finding) + current_script = linter.apply_single_fix(current_script, finding) + cursors[rule.name] = finding.edit.end_pos + len(finding.edit.inserted_text) + changes_made = True + i += 1 + elif choice == "n": + cursors[rule.name] = finding.edit.start_pos + len(finding.original_text) + i += 1 elif choice == "u": - accepted.extend(findings[i - 1 :]) + # Accept all remaining + for j in range(i, len(findings_with_rules)): + f, r = findings_with_rules[j] + if f.edit.start_pos < cursors[r.name]: + f = linter.refresh_finding(current_script, r, cursors[r.name]) + if not f: + continue + current_script = linter.apply_single_fix(current_script, f) + cursors[r.name] = f.edit.end_pos + len(f.edit.inserted_text) + changes_made = True break elif choice == "s": break @@ -95,7 +135,7 @@ def interactive_mode(findings: List[LintFinding], script_content: str) -> List[L print("Quit without saving.") sys.exit(0) - return accepted + return current_script, changes_made def main(): @@ -134,29 +174,38 @@ def main(): rules = [Base64BinaryFormatRule()] linter = ScriptLinter(rules) - findings = linter.lint(script_content) + findings_with_rules = linter.lint(script_content) - if not findings: + if not findings_with_rules: print("No issues found.") return if args.interactive: - findings = interactive_mode(findings, script_content) - if not findings: + fixed_content, changes_made = interactive_mode(findings_with_rules, script_content, linter) + if not changes_made: print("No changes accepted.") return - - if args.fix or args.output or args.interactive: - # Interactive mode is functionally equivalent to --fix, except the user - # can select a subset of the changes to apply. - fixed_content = linter.apply_fixes(script_content, findings) output_path = Path(args.output) if args.output else script_path output_path.write_text(fixed_content) print(f"Fixed script written to: {output_path}") + elif args.fix or args.output: + # Auto-accept all findings + current_script = script_content + cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} + for finding, rule in findings_with_rules: + if finding.edit.start_pos < cursors[rule.name]: + finding = linter.refresh_finding(current_script, rule, cursors[rule.name]) + if not finding: + continue + current_script = linter.apply_single_fix(current_script, finding) + cursors[rule.name] = finding.edit.end_pos + len(finding.edit.inserted_text) + output_path = Path(args.output) if args.output else script_path + output_path.write_text(current_script) + print(f"Fixed script written to: {output_path}") else: - print(f"\nFound {len(findings)} issue(s):\n") - for i, finding in enumerate(findings, 1): - display_finding(finding, i, len(findings), script_content) + print(f"\nFound {len(findings_with_rules)} issue(s):\n") + for i, (finding, _) in enumerate(findings_with_rules, 1): + display_finding(finding, i, len(findings_with_rules), script_content) print("\n\nRun with --fix to apply changes or --interactive to review each change.") diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index 25b7c311e26b..73656544c480 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -1,4 +1,4 @@ -from typing import List +from typing import List, Tuple from ast_grep_py import SgRoot @@ -11,16 +11,28 @@ class ScriptLinter: def __init__(self, rules: List[LintRule]): self.rules = rules - def lint(self, script_content: str) -> List[LintFinding]: - """Lint the script and return all findings.""" + def lint(self, script_content: str) -> List[Tuple[LintFinding, LintRule]]: + """Lint the script and return all findings with their associated rules.""" root = SgRoot(script_content, "bash") - findings = [] + findings_with_rules = [] for rule in self.rules: - findings.extend(rule.check(root)) - return sorted(findings, key=lambda f: (f.line_start, f.line_end)) + findings = rule.check(root) + for finding in findings: + findings_with_rules.append((finding, rule)) + return sorted( + findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) + ) - def apply_fixes(self, script_content: str, findings: List[LintFinding]) -> str: - """Apply fixes to the script content.""" + def apply_single_fix(self, script_content: str, finding: LintFinding) -> str: + """Apply a single fix to the script content.""" root = SgRoot(script_content, "bash") node = root.root() - return node.commit_edits([f.edit for f in findings]) + return node.commit_edits([finding.edit]) + + def refresh_finding( + self, script_content: str, rule: LintRule, cursor_pos: int + ) -> LintFinding | None: + """Re-lint from cursor position and return the first finding for this rule.""" + root = SgRoot(script_content, "bash") + findings = rule.check(root, start_pos=cursor_pos) + return findings[0] if findings else None diff --git a/awsclilinter/awsclilinter/rules/base.py b/awsclilinter/awsclilinter/rules/base.py index 5900a2c09ff8..e9d8efd94e03 100644 --- a/awsclilinter/awsclilinter/rules/base.py +++ b/awsclilinter/awsclilinter/rules/base.py @@ -33,6 +33,11 @@ def description(self) -> str: pass @abstractmethod - def check(self, root: SgRoot) -> List[LintFinding]: - """Check the AST root for violations and return findings.""" + def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: + """Check the AST root for violations and return findings. + + Args: + root: The AST root to check + start_pos: Starting position in the script to begin checking from + """ pass diff --git a/awsclilinter/awsclilinter/rules/base64_rule.py b/awsclilinter/awsclilinter/rules/base64_rule.py index 18bbdd015b72..6b5c836a32c7 100644 --- a/awsclilinter/awsclilinter/rules/base64_rule.py +++ b/awsclilinter/awsclilinter/rules/base64_rule.py @@ -21,20 +21,22 @@ def description(self) -> str: "add `--cli-binary-format raw-in-base64-out`." ) - def check(self, root: SgRoot) -> List[LintFinding]: + def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: """Check for AWS CLI commands missing --cli-binary-format.""" node = root.root() base64_broken_nodes = node.find_all( all=[ {"kind": "command"}, {"pattern": "aws $SERVICE $OPERATION $$$ARGS"}, - {"kind": "command"}, {"not": {"has": {"kind": "word", "pattern": "--cli-binary-format"}}}, ] ) findings = [] for stmt in base64_broken_nodes: + # Skip nodes before start_pos + if stmt.range().start.index < start_pos: + continue original = stmt.text() # To retain v1 behavior after migrating to v2, append # --cli-binary-format raw-in-base64-out diff --git a/awsclilinter/awsclilinter/rules/copy_props_rule.py b/awsclilinter/awsclilinter/rules/copy_props_rule.py new file mode 100644 index 000000000000..2652c765fcbb --- /dev/null +++ b/awsclilinter/awsclilinter/rules/copy_props_rule.py @@ -0,0 +1,51 @@ +from typing import List + +from ast_grep_py.ast_grep_py import SgRoot + +from awsclilinter.rules import LintFinding, LintRule + + +class CopyPropsRule(LintRule): + """Dummy test rule that adds --copy-props none to every AWS CLI command.""" + + @property + def name(self) -> str: + return "copy-props-none" + + @property + def description(self) -> str: + return "Add --copy-props none to AWS CLI commands for testing multiple rules." + + def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: + """Check for AWS CLI commands missing --copy-props none.""" + node = root.root() + nodes = node.find_all( + all=[ + {"kind": "command"}, + {"pattern": "aws $SERVICE $OPERATION $$$ARGS"}, + {"not": {"has": {"kind": "word", "pattern": "--copy-props"}}}, + ] + ) + + findings = [] + for stmt in nodes: + # Skip nodes before start_pos + if stmt.range().start.index < start_pos: + continue + + original = stmt.text() + suggested = original + " --copy-props none" + edit = stmt.replace(suggested) + + findings.append( + LintFinding( + line_start=stmt.range().start.line, + line_end=stmt.range().end.line, + edit=edit, + original_text=original, + rule_name=self.name, + description=self.description, + ) + ) + + return findings diff --git a/awsclilinter/tests/test_linter.py b/awsclilinter/tests/test_linter.py index 1f3afb274259..aab2a0324cea 100644 --- a/awsclilinter/tests/test_linter.py +++ b/awsclilinter/tests/test_linter.py @@ -9,18 +9,20 @@ def test_lint_finds_issues(self): """Test that linter finds issues in script.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" linter = ScriptLinter([Base64BinaryFormatRule()]) - findings = linter.lint(script) + findings_with_rules = linter.lint(script) - assert len(findings) == 1 - assert findings[0].rule_name == "binary-params-base64" - assert "file://" in findings[0].original_text + assert len(findings_with_rules) == 1 + finding, rule = findings_with_rules[0] + assert finding.rule_name == "binary-params-base64" + assert "file://" in finding.original_text def test_apply_fixes(self): """Test that fixes are applied correctly.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" linter = ScriptLinter([Base64BinaryFormatRule()]) - findings = linter.lint(script) - fixed = linter.apply_fixes(script, findings) + findings_with_rules = linter.lint(script) + finding, _ = findings_with_rules[0] + fixed = linter.apply_single_fix(script, finding) assert "--cli-binary-format raw-in-base64-out" in fixed assert "file://data.json" in fixed @@ -34,6 +36,6 @@ def test_multiple_issues(self): "--data file://data --partition-key samplepartitionkey" ) linter = ScriptLinter([Base64BinaryFormatRule()]) - findings = linter.lint(script) + findings_with_rules = linter.lint(script) - assert len(findings) == 2 + assert len(findings_with_rules) == 2 From c7f013a76725df758453f8523be59e6dca1b6fc4 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 16:06:38 -0500 Subject: [PATCH 04/14] Working prototype for multiple linting rules. --- awsclilinter/awsclilinter/cli.py | 38 +++++++++++++------ ...{copy_props_rule.py => pagination_rule.py} | 17 +++++---- awsclilinter/tests/test_cli.py | 15 +++++++- awsclilinter/tests/test_linter.py | 3 +- 4 files changed, 51 insertions(+), 22 deletions(-) rename awsclilinter/awsclilinter/rules/{copy_props_rule.py => pagination_rule.py} (67%) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index d921944eb11b..e04155de687f 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -7,6 +7,7 @@ from awsclilinter.linter import ScriptLinter from awsclilinter.rules import LintFinding, LintRule from awsclilinter.rules.base64_rule import Base64BinaryFormatRule +from awsclilinter.rules.pagination_rule import PaginationRule # ANSI color codes RED = "\033[31m" @@ -86,6 +87,7 @@ def interactive_mode( Tuple of (modified_script, changes_made) """ current_script = script_content + # Track the position where we've processed up to for each rule cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} changes_made = False i = 0 @@ -93,14 +95,16 @@ def interactive_mode( while i < len(findings_with_rules): finding, rule = findings_with_rules[i] - # Refresh finding if cursor has moved past it - if finding.edit.start_pos < cursors[rule.name]: + # After any script modification, we need fresh findings from the current cursor position + # because all positions after the modification have shifted + if changes_made: + # Get a fresh finding for this rule starting from where we left off refreshed = linter.refresh_finding(current_script, rule, cursors[rule.name]) if refreshed: finding = refreshed findings_with_rules[i] = (finding, rule) else: - # No more findings for this rule + # No more findings for this rule from this position i += 1 continue @@ -110,23 +114,27 @@ def interactive_mode( ) if choice == "y": + # Apply the fix current_script = linter.apply_single_fix(current_script, finding) - cursors[rule.name] = finding.edit.end_pos + len(finding.edit.inserted_text) + # Update cursor to the end of what we just fixed + cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) changes_made = True i += 1 elif choice == "n": - cursors[rule.name] = finding.edit.start_pos + len(finding.original_text) + # Skip this fix, move cursor past it + cursors[rule.name] = finding.edit.end_pos i += 1 elif choice == "u": - # Accept all remaining + # Accept all remaining findings for j in range(i, len(findings_with_rules)): f, r = findings_with_rules[j] - if f.edit.start_pos < cursors[r.name]: + # Refresh if we've made changes + if changes_made: f = linter.refresh_finding(current_script, r, cursors[r.name]) if not f: continue current_script = linter.apply_single_fix(current_script, f) - cursors[r.name] = f.edit.end_pos + len(f.edit.inserted_text) + cursors[r.name] = f.edit.start_pos + len(f.edit.inserted_text) changes_made = True break elif choice == "s": @@ -172,7 +180,7 @@ def main(): script_content = script_path.read_text() - rules = [Base64BinaryFormatRule()] + rules = [Base64BinaryFormatRule(), PaginationRule()] linter = ScriptLinter(rules) findings_with_rules = linter.lint(script_content) @@ -192,13 +200,19 @@ def main(): # Auto-accept all findings current_script = script_content cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} - for finding, rule in findings_with_rules: - if finding.edit.start_pos < cursors[rule.name]: + changes_made = False + + for i, (finding, rule) in enumerate(findings_with_rules): + # Refresh if we've made changes + if changes_made: finding = linter.refresh_finding(current_script, rule, cursors[rule.name]) if not finding: continue + current_script = linter.apply_single_fix(current_script, finding) - cursors[rule.name] = finding.edit.end_pos + len(finding.edit.inserted_text) + cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) + changes_made = True + output_path = Path(args.output) if args.output else script_path output_path.write_text(current_script) print(f"Fixed script written to: {output_path}") diff --git a/awsclilinter/awsclilinter/rules/copy_props_rule.py b/awsclilinter/awsclilinter/rules/pagination_rule.py similarity index 67% rename from awsclilinter/awsclilinter/rules/copy_props_rule.py rename to awsclilinter/awsclilinter/rules/pagination_rule.py index 2652c765fcbb..087179cc38e4 100644 --- a/awsclilinter/awsclilinter/rules/copy_props_rule.py +++ b/awsclilinter/awsclilinter/rules/pagination_rule.py @@ -5,25 +5,28 @@ from awsclilinter.rules import LintFinding, LintRule -class CopyPropsRule(LintRule): - """Dummy test rule that adds --copy-props none to every AWS CLI command.""" +class PaginationRule(LintRule): + """Detects AWS CLI commands missing --no-cli-paginate flag.""" @property def name(self) -> str: - return "copy-props-none" + return "no-cli-paginate" @property def description(self) -> str: - return "Add --copy-props none to AWS CLI commands for testing multiple rules." + return ( + "AWS CLI v2 uses pagination by default for commands that return large result sets. " + "Add --no-cli-paginate to disable pagination and match v1 behavior." + ) def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: - """Check for AWS CLI commands missing --copy-props none.""" + """Check for AWS CLI commands missing --no-cli-paginate.""" node = root.root() nodes = node.find_all( all=[ {"kind": "command"}, {"pattern": "aws $SERVICE $OPERATION $$$ARGS"}, - {"not": {"has": {"kind": "word", "pattern": "--copy-props"}}}, + {"not": {"has": {"kind": "word", "pattern": "--no-cli-paginate"}}}, ] ) @@ -34,7 +37,7 @@ def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: continue original = stmt.text() - suggested = original + " --copy-props none" + suggested = original + " --no-cli-paginate" edit = stmt.replace(suggested) findings.append( diff --git a/awsclilinter/tests/test_cli.py b/awsclilinter/tests/test_cli.py index 55e2622f925c..e90f07c763bc 100644 --- a/awsclilinter/tests/test_cli.py +++ b/awsclilinter/tests/test_cli.py @@ -66,7 +66,9 @@ def test_fix_mode(self, tmp_path): with patch("sys.argv", ["upgrade-aws-cli", "--script", str(script_file), "--fix"]): main() fixed_content = script_file.read_text() + # 1 command, 2 rules = 2 flags added assert "--cli-binary-format" in fixed_content + assert "--no-cli-paginate" in fixed_content def test_output_mode(self, tmp_path): """Test output mode creates new file.""" @@ -82,7 +84,10 @@ def test_output_mode(self, tmp_path): ): main() assert output_file.exists() - assert "--cli-binary-format" in output_file.read_text() + content = output_file.read_text() + # 1 command, 2 rules = 2 flags added + assert "--cli-binary-format" in content + assert "--no-cli-paginate" in content def test_interactive_mode_accept_all(self, tmp_path): """Test interactive mode with 'y' to accept all changes.""" @@ -105,10 +110,12 @@ def test_interactive_mode_accept_all(self, tmp_path): str(output_file), ], ): - with patch("builtins.input", side_effect=["y", "y"]): + with patch("builtins.input", side_effect=["y", "y", "y", "y"]): main() fixed_content = output_file.read_text() + # 2 commands, 2 rules = 4 findings, so 2 of each flag assert fixed_content.count("--cli-binary-format") == 2 + assert fixed_content.count("--no-cli-paginate") == 2 def test_interactive_mode_reject_all(self, tmp_path, capsys): """Test interactive mode with 'n' to reject all changes.""" @@ -146,7 +153,9 @@ def test_interactive_mode_update_all(self, tmp_path): with patch("builtins.input", return_value="u"): main() fixed_content = output_file.read_text() + # 2 commands, 2 rules = 4 findings, so 2 of each flag assert fixed_content.count("--cli-binary-format") == 2 + assert fixed_content.count("--no-cli-paginate") == 2 def test_interactive_mode_save_and_exit(self, tmp_path): """Test interactive mode with 's' to save and exit.""" @@ -173,6 +182,8 @@ def test_interactive_mode_save_and_exit(self, tmp_path): main() fixed_content = output_file.read_text() # Only first change should be applied since we pressed 's' on the second + # First finding is binary-params-base64 for cmd1 + assert "--cli-binary-format" in fixed_content assert fixed_content.count("--cli-binary-format") == 1 def test_interactive_mode_quit(self, tmp_path): diff --git a/awsclilinter/tests/test_linter.py b/awsclilinter/tests/test_linter.py index aab2a0324cea..45b926a5fcdf 100644 --- a/awsclilinter/tests/test_linter.py +++ b/awsclilinter/tests/test_linter.py @@ -14,7 +14,7 @@ def test_lint_finds_issues(self): assert len(findings_with_rules) == 1 finding, rule = findings_with_rules[0] assert finding.rule_name == "binary-params-base64" - assert "file://" in finding.original_text + assert "aws" in finding.original_text def test_apply_fixes(self): """Test that fixes are applied correctly.""" @@ -38,4 +38,5 @@ def test_multiple_issues(self): linter = ScriptLinter([Base64BinaryFormatRule()]) findings_with_rules = linter.lint(script) + # 2 commands, 1 rule = 2 findings assert len(findings_with_rules) == 2 From f4a1be29287950422bc136976673527f345103b9 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 16:10:21 -0500 Subject: [PATCH 05/14] Refactor to use a apply_all_fixes helper function. --- awsclilinter/awsclilinter/cli.py | 76 +++++++++++++++++--------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index e04155de687f..3bed3c5bbd41 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -76,6 +76,42 @@ def display_finding(finding: LintFinding, index: int, total: int, script_content print(line, end="") +def apply_all_fixes( + findings_with_rules: List[Tuple[LintFinding, LintRule]], + script_content: str, + linter: ScriptLinter, + start_index: int = 0, +) -> str: + """Apply all fixes from start_index onwards with cursor-based refresh. + + Args: + findings_with_rules: List of findings and their rules + script_content: Current script content + linter: The linter instance + start_index: Index to start applying fixes from + + Returns: + Modified script content + """ + current_script = script_content + cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} + changes_made = start_index > 0 # If we're starting mid-list, changes were already made + + for i in range(start_index, len(findings_with_rules)): + finding, rule = findings_with_rules[i] + + if changes_made: + finding = linter.refresh_finding(current_script, rule, cursors[rule.name]) + if not finding: + continue + + current_script = linter.apply_single_fix(current_script, finding) + cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) + changes_made = True + + return current_script + + def interactive_mode( findings_with_rules: List[Tuple[LintFinding, LintRule]], script_content: str, @@ -87,7 +123,6 @@ def interactive_mode( Tuple of (modified_script, changes_made) """ current_script = script_content - # Track the position where we've processed up to for each rule cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} changes_made = False i = 0 @@ -95,16 +130,12 @@ def interactive_mode( while i < len(findings_with_rules): finding, rule = findings_with_rules[i] - # After any script modification, we need fresh findings from the current cursor position - # because all positions after the modification have shifted if changes_made: - # Get a fresh finding for this rule starting from where we left off refreshed = linter.refresh_finding(current_script, rule, cursors[rule.name]) if refreshed: finding = refreshed findings_with_rules[i] = (finding, rule) else: - # No more findings for this rule from this position i += 1 continue @@ -114,28 +145,16 @@ def interactive_mode( ) if choice == "y": - # Apply the fix current_script = linter.apply_single_fix(current_script, finding) - # Update cursor to the end of what we just fixed cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) changes_made = True i += 1 elif choice == "n": - # Skip this fix, move cursor past it cursors[rule.name] = finding.edit.end_pos i += 1 elif choice == "u": - # Accept all remaining findings - for j in range(i, len(findings_with_rules)): - f, r = findings_with_rules[j] - # Refresh if we've made changes - if changes_made: - f = linter.refresh_finding(current_script, r, cursors[r.name]) - if not f: - continue - current_script = linter.apply_single_fix(current_script, f) - cursors[r.name] = f.edit.start_pos + len(f.edit.inserted_text) - changes_made = True + current_script = apply_all_fixes(findings_with_rules, current_script, linter, i) + changes_made = True break elif choice == "s": break @@ -197,24 +216,9 @@ def main(): output_path.write_text(fixed_content) print(f"Fixed script written to: {output_path}") elif args.fix or args.output: - # Auto-accept all findings - current_script = script_content - cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} - changes_made = False - - for i, (finding, rule) in enumerate(findings_with_rules): - # Refresh if we've made changes - if changes_made: - finding = linter.refresh_finding(current_script, rule, cursors[rule.name]) - if not finding: - continue - - current_script = linter.apply_single_fix(current_script, finding) - cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) - changes_made = True - + fixed_content = apply_all_fixes(findings_with_rules, script_content, linter) output_path = Path(args.output) if args.output else script_path - output_path.write_text(current_script) + output_path.write_text(fixed_content) print(f"Fixed script written to: {output_path}") else: print(f"\nFound {len(findings_with_rules)} issue(s):\n") From f5ef14c96e66f5cfd219241f3c974737dda6f5d3 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 16:41:42 -0500 Subject: [PATCH 06/14] Add performance optimization to only parse into AST at most once per detection. --- awsclilinter/awsclilinter/cli.py | 17 +++++++----- awsclilinter/awsclilinter/linter.py | 26 ++++++++++++++----- .../awsclilinter/rules/base64_rule.py | 1 - .../awsclilinter/rules/pagination_rule.py | 2 -- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index 3bed3c5bbd41..a4ad9f9bdef4 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -99,13 +99,15 @@ def apply_all_fixes( for i in range(start_index, len(findings_with_rules)): finding, rule = findings_with_rules[i] + root = None if changes_made: - finding = linter.refresh_finding(current_script, rule, cursors[rule.name]) - if not finding: + result = linter.refresh_finding(current_script, rule, cursors[rule.name]) + if not result: continue + finding, root = result - current_script = linter.apply_single_fix(current_script, finding) + current_script = linter.apply_single_fix(current_script, finding, root) cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) changes_made = True @@ -129,11 +131,12 @@ def interactive_mode( while i < len(findings_with_rules): finding, rule = findings_with_rules[i] + root = None if changes_made: - refreshed = linter.refresh_finding(current_script, rule, cursors[rule.name]) - if refreshed: - finding = refreshed + result = linter.refresh_finding(current_script, rule, cursors[rule.name]) + if result: + finding, root = result findings_with_rules[i] = (finding, rule) else: i += 1 @@ -145,7 +148,7 @@ def interactive_mode( ) if choice == "y": - current_script = linter.apply_single_fix(current_script, finding) + current_script = linter.apply_single_fix(current_script, finding, root) cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) changes_made = True i += 1 diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index 73656544c480..e038c74e492a 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -23,16 +23,30 @@ def lint(self, script_content: str) -> List[Tuple[LintFinding, LintRule]]: findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) ) - def apply_single_fix(self, script_content: str, finding: LintFinding) -> str: - """Apply a single fix to the script content.""" - root = SgRoot(script_content, "bash") + def apply_single_fix( + self, script_content: str, finding: LintFinding, root: SgRoot | None = None + ) -> str: + """Apply a single fix to the script content. + + Args: + script_content: The script content to modify + finding: The finding with edit to apply + root: Optional pre-parsed SgRoot to reuse (avoids re-parsing) + """ + if root is None: + root = SgRoot(script_content, "bash") node = root.root() return node.commit_edits([finding.edit]) def refresh_finding( self, script_content: str, rule: LintRule, cursor_pos: int - ) -> LintFinding | None: - """Re-lint from cursor position and return the first finding for this rule.""" + ) -> Tuple[LintFinding, SgRoot] | None: + """Re-lint from cursor position and return the first finding and parsed root. + + Returns: + Tuple of (finding, root) if found, None otherwise. The root can be reused + in apply_single_fix to avoid re-parsing. + """ root = SgRoot(script_content, "bash") findings = rule.check(root, start_pos=cursor_pos) - return findings[0] if findings else None + return (findings[0], root) if findings else None diff --git a/awsclilinter/awsclilinter/rules/base64_rule.py b/awsclilinter/awsclilinter/rules/base64_rule.py index 6b5c836a32c7..193e00e69d04 100644 --- a/awsclilinter/awsclilinter/rules/base64_rule.py +++ b/awsclilinter/awsclilinter/rules/base64_rule.py @@ -34,7 +34,6 @@ def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: findings = [] for stmt in base64_broken_nodes: - # Skip nodes before start_pos if stmt.range().start.index < start_pos: continue original = stmt.text() diff --git a/awsclilinter/awsclilinter/rules/pagination_rule.py b/awsclilinter/awsclilinter/rules/pagination_rule.py index 087179cc38e4..58839ca2e705 100644 --- a/awsclilinter/awsclilinter/rules/pagination_rule.py +++ b/awsclilinter/awsclilinter/rules/pagination_rule.py @@ -32,10 +32,8 @@ def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: findings = [] for stmt in nodes: - # Skip nodes before start_pos if stmt.range().start.index < start_pos: continue - original = stmt.text() suggested = original + " --no-cli-paginate" edit = stmt.replace(suggested) From 10caf05195fdd5502ff0a38a06f91017a399abf0 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 12 Nov 2025 16:59:35 -0500 Subject: [PATCH 07/14] Add assertion to verify one LintRule fix won't resolve another rule's fix. --- awsclilinter/awsclilinter/cli.py | 14 +++----------- awsclilinter/awsclilinter/linter.py | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index a4ad9f9bdef4..6b04b5832871 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -102,10 +102,7 @@ def apply_all_fixes( root = None if changes_made: - result = linter.refresh_finding(current_script, rule, cursors[rule.name]) - if not result: - continue - finding, root = result + finding, root = linter.refresh_finding(current_script, rule, cursors[rule.name]) current_script = linter.apply_single_fix(current_script, finding, root) cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) @@ -134,13 +131,8 @@ def interactive_mode( root = None if changes_made: - result = linter.refresh_finding(current_script, rule, cursors[rule.name]) - if result: - finding, root = result - findings_with_rules[i] = (finding, rule) - else: - i += 1 - continue + finding, root = linter.refresh_finding(current_script, rule, cursors[rule.name]) + findings_with_rules[i] = (finding, rule) display_finding(finding, i + 1, len(findings_with_rules), current_script) choice = get_user_choice( diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index e038c74e492a..8e95f9486fa8 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -40,13 +40,24 @@ def apply_single_fix( def refresh_finding( self, script_content: str, rule: LintRule, cursor_pos: int - ) -> Tuple[LintFinding, SgRoot] | None: + ) -> Tuple[LintFinding, SgRoot]: """Re-lint from cursor position and return the first finding and parsed root. Returns: - Tuple of (finding, root) if found, None otherwise. The root can be reused - in apply_single_fix to avoid re-parsing. + Tuple of (finding, root). The root can be reused in apply_single_fix to + avoid re-parsing. + + Raises: + RuntimeError: If no finding is found after cursor_pos. This should never + happen as our rules are designed such that fixing one finding never + inadvertently resolves another. """ root = SgRoot(script_content, "bash") findings = rule.check(root, start_pos=cursor_pos) - return (findings[0], root) if findings else None + if not findings: + raise RuntimeError( + f"Expected to find at least one issue for rule '{rule.name}' after position " + f"{cursor_pos}, but found none. This indicates a rule fix inadvertently " + f"resolved another finding, which should not happen." + ) + return (findings[0], root) From 7a6a1599d6ef98b13c4eee97104dd38e08219dea Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 17 Nov 2025 14:57:23 -0500 Subject: [PATCH 08/14] Progress on multiple linting rules. --- awsclilinter/awsclilinter/cli.py | 136 ++++++++++++------ awsclilinter/awsclilinter/linter.py | 56 ++++---- awsclilinter/awsclilinter/rules/base.py | 2 +- .../awsclilinter/rules/base64_rule.py | 4 +- .../awsclilinter/rules/pagination_rule.py | 4 +- awsclilinter/tests/test_linter.py | 4 +- 6 files changed, 121 insertions(+), 85 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index 6b04b5832871..b31468c191be 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -80,84 +80,92 @@ def apply_all_fixes( findings_with_rules: List[Tuple[LintFinding, LintRule]], script_content: str, linter: ScriptLinter, - start_index: int = 0, ) -> str: - """Apply all fixes from start_index onwards with cursor-based refresh. + """Apply all fixes using rule-by-rule processing. + + Since multiple rules can target the same command, we must process one rule + at a time and re-parse between rules to get fresh Edit objects. Args: findings_with_rules: List of findings and their rules script_content: Current script content linter: The linter instance - start_index: Index to start applying fixes from Returns: Modified script content """ current_script = script_content - cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} - changes_made = start_index > 0 # If we're starting mid-list, changes were already made - - for i in range(start_index, len(findings_with_rules)): - finding, rule = findings_with_rules[i] - root = None - if changes_made: - finding, root = linter.refresh_finding(current_script, rule, cursors[rule.name]) + # Group findings by rule + findings_by_rule: Dict[str, List[LintFinding]] = {} + for finding, rule in findings_with_rules: + if rule.name not in findings_by_rule: + findings_by_rule[rule.name] = [] + findings_by_rule[rule.name].append(finding) - current_script = linter.apply_single_fix(current_script, finding, root) - cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) - changes_made = True + # Process one rule at a time, re-parsing between rules + for rule in linter.rules: + if rule.name in findings_by_rule: + current_script = linter.apply_fixes(current_script, findings_by_rule[rule.name]) return current_script -def interactive_mode( - findings_with_rules: List[Tuple[LintFinding, LintRule]], +def interactive_mode_for_rule( + findings: List[LintFinding], + rule: LintRule, script_content: str, linter: ScriptLinter, -) -> Tuple[str, bool]: - """Run interactive mode with cursor-based workflow. + finding_offset: int, + total_findings: int, +) -> Tuple[str, bool, bool, bool]: + """Run interactive mode for a single rule's findings. + + Args: + findings: List of findings for this rule + rule: The rule being processed + script_content: Current script content + linter: The linter instance + finding_offset: Offset for display numbering + total_findings: Total number of findings across all rules Returns: - Tuple of (modified_script, changes_made) + Tuple of (modified_script, changes_made, should_continue, auto_approve_remaining) + should_continue is False if user chose 's' (save) or 'q' (quit) + auto_approve_remaining is True if user chose 'u' (update all) """ - current_script = script_content - cursors: Dict[str, int] = {rule.name: 0 for rule in linter.rules} - changes_made = False - i = 0 - - while i < len(findings_with_rules): - finding, rule = findings_with_rules[i] - root = None + accepted_findings: List[LintFinding] = [] - if changes_made: - finding, root = linter.refresh_finding(current_script, rule, cursors[rule.name]) - findings_with_rules[i] = (finding, rule) - - display_finding(finding, i + 1, len(findings_with_rules), current_script) + for i, finding in enumerate(findings): + display_finding(finding, finding_offset + i + 1, total_findings, script_content) choice = get_user_choice( "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " ) if choice == "y": - current_script = linter.apply_single_fix(current_script, finding, root) - cursors[rule.name] = finding.edit.start_pos + len(finding.edit.inserted_text) - changes_made = True - i += 1 + accepted_findings.append(finding) elif choice == "n": - cursors[rule.name] = finding.edit.end_pos - i += 1 + pass # Skip this finding elif choice == "u": - current_script = apply_all_fixes(findings_with_rules, current_script, linter, i) - changes_made = True - break + # Accept this and all remaining findings for all rules + accepted_findings.extend(findings[i:]) + if accepted_findings: + script_content = linter.apply_fixes(script_content, accepted_findings) + return script_content, True, True, True elif choice == "s": - break + # Apply accepted findings and stop processing + if accepted_findings: + script_content = linter.apply_fixes(script_content, accepted_findings) + return script_content, len(accepted_findings) > 0, False, False elif choice == "q": print("Quit without saving.") sys.exit(0) - return current_script, changes_made + if accepted_findings: + script_content = linter.apply_fixes(script_content, accepted_findings) + return script_content, True, True, False + + return script_content, False, True, False def main(): @@ -203,12 +211,48 @@ def main(): return if args.interactive: - fixed_content, changes_made = interactive_mode(findings_with_rules, script_content, linter) - if not changes_made: + # Process one rule at a time, re-parsing between rules + current_script = script_content + any_changes = False + finding_offset = 0 + + # Calculate total findings for display + total_findings = len(findings_with_rules) + + for rule_index, rule in enumerate(linter.rules): + # Lint for this specific rule with current script state + rule_findings = linter.lint_for_rule(current_script, rule) + + if not rule_findings: + continue + + current_script, changes_made, should_continue, auto_approve = interactive_mode_for_rule( + rule_findings, rule, current_script, linter, finding_offset, total_findings + ) + + if changes_made: + any_changes = True + + finding_offset += len(rule_findings) + + if not should_continue: + break + + # If user chose 'u', auto-apply all remaining rules + if auto_approve: + for remaining_rule in linter.rules[rule_index + 1 :]: + remaining_findings = linter.lint_for_rule(current_script, remaining_rule) + if remaining_findings: + current_script = linter.apply_fixes(current_script, remaining_findings) + any_changes = True + break + + if not any_changes: print("No changes accepted.") return + output_path = Path(args.output) if args.output else script_path - output_path.write_text(fixed_content) + output_path.write_text(current_script) print(f"Fixed script written to: {output_path}") elif args.fix or args.output: fixed_content = apply_all_fixes(findings_with_rules, script_content, linter) diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index 8e95f9486fa8..00e53cae3cbf 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -23,41 +23,37 @@ def lint(self, script_content: str) -> List[Tuple[LintFinding, LintRule]]: findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) ) - def apply_single_fix( - self, script_content: str, finding: LintFinding, root: SgRoot | None = None - ) -> str: - """Apply a single fix to the script content. + def lint_for_rule(self, script_content: str, rule: LintRule) -> List[LintFinding]: + """Lint the script for a single rule. Args: - script_content: The script content to modify - finding: The finding with edit to apply - root: Optional pre-parsed SgRoot to reuse (avoids re-parsing) + script_content: The script content to lint + rule: The rule to check + + Returns: + List of findings for this rule, sorted by position """ - if root is None: - root = SgRoot(script_content, "bash") - node = root.root() - return node.commit_edits([finding.edit]) + root = SgRoot(script_content, "bash") + findings = rule.check(root) + return sorted(findings, key=lambda f: (f.edit.start_pos, f.edit.end_pos)) - def refresh_finding( - self, script_content: str, rule: LintRule, cursor_pos: int - ) -> Tuple[LintFinding, SgRoot]: - """Re-lint from cursor position and return the first finding and parsed root. + def apply_fixes(self, script_content: str, findings: List[LintFinding]) -> str: + """Apply multiple fixes for a single rule. - Returns: - Tuple of (finding, root). The root can be reused in apply_single_fix to - avoid re-parsing. + Args: + script_content: The script content to modify + findings: List of findings from a single rule to apply - Raises: - RuntimeError: If no finding is found after cursor_pos. This should never - happen as our rules are designed such that fixing one finding never - inadvertently resolves another. + Returns: + Modified script content """ - root = SgRoot(script_content, "bash") - findings = rule.check(root, start_pos=cursor_pos) if not findings: - raise RuntimeError( - f"Expected to find at least one issue for rule '{rule.name}' after position " - f"{cursor_pos}, but found none. This indicates a rule fix inadvertently " - f"resolved another finding, which should not happen." - ) - return (findings[0], root) + return script_content + + root = SgRoot(script_content, "bash") + node = root.root() + + # Collect all edits - they should be non-overlapping within a single rule + edits = [f.edit for f in findings] + + return node.commit_edits(edits) diff --git a/awsclilinter/awsclilinter/rules/base.py b/awsclilinter/awsclilinter/rules/base.py index e9d8efd94e03..2fe0d311f0e0 100644 --- a/awsclilinter/awsclilinter/rules/base.py +++ b/awsclilinter/awsclilinter/rules/base.py @@ -33,7 +33,7 @@ def description(self) -> str: pass @abstractmethod - def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: + def check(self, root: SgRoot) -> List[LintFinding]: """Check the AST root for violations and return findings. Args: diff --git a/awsclilinter/awsclilinter/rules/base64_rule.py b/awsclilinter/awsclilinter/rules/base64_rule.py index 193e00e69d04..980d275243e9 100644 --- a/awsclilinter/awsclilinter/rules/base64_rule.py +++ b/awsclilinter/awsclilinter/rules/base64_rule.py @@ -21,7 +21,7 @@ def description(self) -> str: "add `--cli-binary-format raw-in-base64-out`." ) - def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: + def check(self, root: SgRoot) -> List[LintFinding]: """Check for AWS CLI commands missing --cli-binary-format.""" node = root.root() base64_broken_nodes = node.find_all( @@ -34,8 +34,6 @@ def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: findings = [] for stmt in base64_broken_nodes: - if stmt.range().start.index < start_pos: - continue original = stmt.text() # To retain v1 behavior after migrating to v2, append # --cli-binary-format raw-in-base64-out diff --git a/awsclilinter/awsclilinter/rules/pagination_rule.py b/awsclilinter/awsclilinter/rules/pagination_rule.py index 58839ca2e705..f7416f0c4943 100644 --- a/awsclilinter/awsclilinter/rules/pagination_rule.py +++ b/awsclilinter/awsclilinter/rules/pagination_rule.py @@ -19,7 +19,7 @@ def description(self) -> str: "Add --no-cli-paginate to disable pagination and match v1 behavior." ) - def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: + def check(self, root: SgRoot) -> List[LintFinding]: """Check for AWS CLI commands missing --no-cli-paginate.""" node = root.root() nodes = node.find_all( @@ -32,8 +32,6 @@ def check(self, root: SgRoot, start_pos: int = 0) -> List[LintFinding]: findings = [] for stmt in nodes: - if stmt.range().start.index < start_pos: - continue original = stmt.text() suggested = original + " --no-cli-paginate" edit = stmt.replace(suggested) diff --git a/awsclilinter/tests/test_linter.py b/awsclilinter/tests/test_linter.py index 45b926a5fcdf..db66e45e0804 100644 --- a/awsclilinter/tests/test_linter.py +++ b/awsclilinter/tests/test_linter.py @@ -21,8 +21,8 @@ def test_apply_fixes(self): script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" linter = ScriptLinter([Base64BinaryFormatRule()]) findings_with_rules = linter.lint(script) - finding, _ = findings_with_rules[0] - fixed = linter.apply_single_fix(script, finding) + findings = [f for f, _ in findings_with_rules] + fixed = linter.apply_fixes(script, findings) assert "--cli-binary-format raw-in-base64-out" in fixed assert "file://data.json" in fixed From e496ac3155fcfc2dbe76949b45c89a5b5dc63548 Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 10:30:03 -0500 Subject: [PATCH 09/14] Finalize refactor for multiple linting rules. --- awsclilinter/awsclilinter/cli.py | 142 ++++++++++++++++------------ awsclilinter/awsclilinter/linter.py | 99 +++++++++---------- awsclilinter/tests/test_linter.py | 11 +-- 3 files changed, 129 insertions(+), 123 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index b31468c191be..b957aea41219 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -2,9 +2,12 @@ import difflib import sys from pathlib import Path -from typing import Dict, List, Tuple +from typing import Dict, List, Tuple, Optional -from awsclilinter.linter import ScriptLinter +from ast_grep_py.ast_grep_py import SgRoot + +from awsclilinter import linter +from awsclilinter.linter import parse from awsclilinter.rules import LintFinding, LintRule from awsclilinter.rules.base64_rule import Base64BinaryFormatRule from awsclilinter.rules.pagination_rule import PaginationRule @@ -19,10 +22,12 @@ CONTEXT_SIZE = 3 -def get_user_choice(prompt: str) -> str: +def prompt_user_choice_interactive_mode() -> str: """Get user input for interactive mode.""" while True: - choice = input(prompt).lower().strip() + choice = input( + "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " + ).lower().strip() if choice in ["y", "n", "u", "s", "q"]: return choice print("Invalid choice. Please enter y, n, u, s, or q.") @@ -78,23 +83,21 @@ def display_finding(finding: LintFinding, index: int, total: int, script_content def apply_all_fixes( findings_with_rules: List[Tuple[LintFinding, LintRule]], - script_content: str, - linter: ScriptLinter, + ast: SgRoot, ) -> str: """Apply all fixes using rule-by-rule processing. Since multiple rules can target the same command, we must process one rule - at a time and re-parse between rules to get fresh Edit objects. + at a time and re-parse the updated script between rules to get fresh Edit objects. Args: - findings_with_rules: List of findings and their rules - script_content: Current script content - linter: The linter instance + findings_with_rules: List of findings and their rules. + ast: Current script represented as an AST. Returns: Modified script content """ - current_script = script_content + current_ast = ast # Group findings by rule findings_by_rule: Dict[str, List[LintFinding]] = {} @@ -104,68 +107,69 @@ def apply_all_fixes( findings_by_rule[rule.name].append(finding) # Process one rule at a time, re-parsing between rules - for rule in linter.rules: - if rule.name in findings_by_rule: - current_script = linter.apply_fixes(current_script, findings_by_rule[rule.name]) - - return current_script + for rule in findings_by_rule: + updated_script = linter.apply_fixes(current_ast, findings_by_rule[rule]) + current_ast = parse(updated_script) + return current_ast.root().text() def interactive_mode_for_rule( findings: List[LintFinding], - rule: LintRule, - script_content: str, - linter: ScriptLinter, + ast: SgRoot, finding_offset: int, total_findings: int, -) -> Tuple[str, bool, bool, bool]: +) -> Tuple[SgRoot, bool, Optional[str]]: """Run interactive mode for a single rule's findings. Args: - findings: List of findings for this rule - rule: The rule being processed - script_content: Current script content - linter: The linter instance - finding_offset: Offset for display numbering - total_findings: Total number of findings across all rules + findings: List of findings for this rule. + ast: Current script content, represented as an AST. + finding_offset: Offset for display numbering. + total_findings: Total number of findings across all rules. Returns: - Tuple of (modified_script, changes_made, should_continue, auto_approve_remaining) - should_continue is False if user chose 's' (save) or 'q' (quit) - auto_approve_remaining is True if user chose 'u' (update all) + Tuple of (ast, changes_made, last_choice) + ast is the resulting AST from this interactive mode execution. + changes_made whether the AST was updated based on user choice. + last_choice is the last choice entered by the user. """ accepted_findings: List[LintFinding] = [] + last_choice: Optional[str] = None for i, finding in enumerate(findings): - display_finding(finding, finding_offset + i + 1, total_findings, script_content) - choice = get_user_choice( - "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " - ) + display_finding(finding, finding_offset + i + 1, total_findings, ast.root().text()) + last_choice = prompt_user_choice_interactive_mode() - if choice == "y": + if last_choice == "y": accepted_findings.append(finding) - elif choice == "n": + elif last_choice == "n": pass # Skip this finding - elif choice == "u": - # Accept this and all remaining findings for all rules + elif last_choice == "u": + # Accept this and all remaining findings for this rule. accepted_findings.extend(findings[i:]) if accepted_findings: - script_content = linter.apply_fixes(script_content, accepted_findings) - return script_content, True, True, True - elif choice == "s": + ast = parse( + linter.apply_fixes(ast, accepted_findings) + ) + return ast, True, last_choice + elif last_choice == "s": # Apply accepted findings and stop processing if accepted_findings: - script_content = linter.apply_fixes(script_content, accepted_findings) - return script_content, len(accepted_findings) > 0, False, False - elif choice == "q": + ast = parse( + linter.apply_fixes(ast, accepted_findings) + ) + return ast, len(accepted_findings) > 0, last_choice + elif last_choice == "q": print("Quit without saving.") sys.exit(0) if accepted_findings: - script_content = linter.apply_fixes(script_content, accepted_findings) - return script_content, True, True, False + ast = parse( + linter.apply_fixes(ast, accepted_findings) + ) + return ast, True, last_choice - return script_content, False, True, False + return ast, False, last_choice def main(): @@ -203,14 +207,17 @@ def main(): script_content = script_path.read_text() rules = [Base64BinaryFormatRule(), PaginationRule()] - linter = ScriptLinter(rules) - findings_with_rules = linter.lint(script_content) - - if not findings_with_rules: - print("No issues found.") - return if args.interactive: + # Do an initial parse-and-lint with all the rules simultaneously to compute the total + # number of findings for displaying progress in interactive mode. + current_ast = parse(script_content) + findings_with_rules = linter.lint(current_ast, rules) + + if not findings_with_rules: + print("No issues found.") + return + # Process one rule at a time, re-parsing between rules current_script = script_content any_changes = False @@ -219,15 +226,15 @@ def main(): # Calculate total findings for display total_findings = len(findings_with_rules) - for rule_index, rule in enumerate(linter.rules): + for rule_index, rule in enumerate(rules): # Lint for this specific rule with current script state - rule_findings = linter.lint_for_rule(current_script, rule) + rule_findings = linter.lint_for_rule(current_ast, rule) if not rule_findings: continue - current_script, changes_made, should_continue, auto_approve = interactive_mode_for_rule( - rule_findings, rule, current_script, linter, finding_offset, total_findings + current_ast, changes_made, last_choice = interactive_mode_for_rule( + rule_findings, current_ast, finding_offset, total_findings ) if changes_made: @@ -235,15 +242,15 @@ def main(): finding_offset += len(rule_findings) - if not should_continue: + if last_choice == 's': break # If user chose 'u', auto-apply all remaining rules - if auto_approve: - for remaining_rule in linter.rules[rule_index + 1 :]: - remaining_findings = linter.lint_for_rule(current_script, remaining_rule) + if last_choice == 'u': + for remaining_rule in rules[rule_index + 1 :]: + remaining_findings = linter.lint_for_rule(current_ast, remaining_rule) if remaining_findings: - current_script = linter.apply_fixes(current_script, remaining_findings) + current_script = linter.apply_fixes(current_ast, remaining_findings) any_changes = True break @@ -255,11 +262,20 @@ def main(): output_path.write_text(current_script) print(f"Fixed script written to: {output_path}") elif args.fix or args.output: - fixed_content = apply_all_fixes(findings_with_rules, script_content, linter) + current_ast = parse(script_content) + findings_with_rules = linter.lint(current_ast, rules) + updated_script = apply_all_fixes(findings_with_rules, current_ast) output_path = Path(args.output) if args.output else script_path - output_path.write_text(fixed_content) + output_path.write_text(updated_script) print(f"Fixed script written to: {output_path}") else: + current_ast = parse(script_content) + findings_with_rules = linter.lint(current_ast, rules) + + if not findings_with_rules: + print("No issues found.") + return + print(f"\nFound {len(findings_with_rules)} issue(s):\n") for i, (finding, _) in enumerate(findings_with_rules, 1): display_finding(finding, i, len(findings_with_rules), script_content) diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index 00e53cae3cbf..93bb22eae2da 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -1,59 +1,52 @@ from typing import List, Tuple -from ast_grep_py import SgRoot +from ast_grep_py.ast_grep_py import SgRoot from awsclilinter.rules import LintFinding, LintRule -class ScriptLinter: - """Linter for bash scripts to detect AWS CLI v1 to v2 migration issues.""" - - def __init__(self, rules: List[LintRule]): - self.rules = rules - - def lint(self, script_content: str) -> List[Tuple[LintFinding, LintRule]]: - """Lint the script and return all findings with their associated rules.""" - root = SgRoot(script_content, "bash") - findings_with_rules = [] - for rule in self.rules: - findings = rule.check(root) - for finding in findings: - findings_with_rules.append((finding, rule)) - return sorted( - findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) - ) - - def lint_for_rule(self, script_content: str, rule: LintRule) -> List[LintFinding]: - """Lint the script for a single rule. - - Args: - script_content: The script content to lint - rule: The rule to check - - Returns: - List of findings for this rule, sorted by position - """ - root = SgRoot(script_content, "bash") - findings = rule.check(root) - return sorted(findings, key=lambda f: (f.edit.start_pos, f.edit.end_pos)) - - def apply_fixes(self, script_content: str, findings: List[LintFinding]) -> str: - """Apply multiple fixes for a single rule. - - Args: - script_content: The script content to modify - findings: List of findings from a single rule to apply - - Returns: - Modified script content - """ - if not findings: - return script_content - - root = SgRoot(script_content, "bash") - node = root.root() - - # Collect all edits - they should be non-overlapping within a single rule - edits = [f.edit for f in findings] - - return node.commit_edits(edits) +def parse(script_content: str) -> SgRoot: + """Parse the bash script content into an AST.""" + return SgRoot(script_content, 'bash') + +def lint(ast: SgRoot, rules: List[LintRule]) -> List[Tuple[LintFinding, LintRule]]: + """Lint the AST and return all findings with their associated rules.""" + findings_with_rules = [] + for rule in rules: + findings = rule.check(ast) + for finding in findings: + findings_with_rules.append((finding, rule)) + return sorted( + findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) + ) + +def lint_for_rule(ast: SgRoot, rule: LintRule) -> List[LintFinding]: + """Lint the script for a single rule. + + Args: + ast: The AST to lint for the rule. + rule: The rule to check. + + Returns: + List of findings for this rule, sorted by position (ascending) + """ + findings = rule.check(ast) + return sorted(findings, key=lambda f: (f.edit.start_pos, f.edit.end_pos)) + +def apply_fixes(ast: SgRoot, findings: List[LintFinding]) -> str: + """Apply to the AST for a single rule. + + Args: + ast: The AST representation of the script to apply fixes to. + findings: List of findings from a single rule to apply. + + Returns: + Modified script content + """ + root = ast.root() + if not findings: + return root.text() + + # Collect all edits - they should be non-overlapping within a single rule + edits = [f.edit for f in findings] + return root.commit_edits(edits) diff --git a/awsclilinter/tests/test_linter.py b/awsclilinter/tests/test_linter.py index db66e45e0804..1e7b3ce8800c 100644 --- a/awsclilinter/tests/test_linter.py +++ b/awsclilinter/tests/test_linter.py @@ -1,4 +1,4 @@ -from awsclilinter.linter import ScriptLinter +from awsclilinter import linter from awsclilinter.rules.base64_rule import Base64BinaryFormatRule @@ -8,8 +8,7 @@ class TestScriptLinter: def test_lint_finds_issues(self): """Test that linter finds issues in script.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" - linter = ScriptLinter([Base64BinaryFormatRule()]) - findings_with_rules = linter.lint(script) + findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) assert len(findings_with_rules) == 1 finding, rule = findings_with_rules[0] @@ -19,8 +18,7 @@ def test_lint_finds_issues(self): def test_apply_fixes(self): """Test that fixes are applied correctly.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" - linter = ScriptLinter([Base64BinaryFormatRule()]) - findings_with_rules = linter.lint(script) + findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) findings = [f for f, _ in findings_with_rules] fixed = linter.apply_fixes(script, findings) @@ -35,8 +33,7 @@ def test_multiple_issues(self): " aws kinesis put-record --stream-name samplestream " "--data file://data --partition-key samplepartitionkey" ) - linter = ScriptLinter([Base64BinaryFormatRule()]) - findings_with_rules = linter.lint(script) + findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) # 2 commands, 1 rule = 2 findings assert len(findings_with_rules) == 2 From a46c0046d7a254a69d29a383f9a53df0d455e2b9 Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 10:32:47 -0500 Subject: [PATCH 10/14] Add LICENSE.txt to MANIFEST.in. --- awsclilinter/MANIFEST.in | 1 + 1 file changed, 1 insertion(+) diff --git a/awsclilinter/MANIFEST.in b/awsclilinter/MANIFEST.in index 5de61f385a65..8d19f2886e8e 100644 --- a/awsclilinter/MANIFEST.in +++ b/awsclilinter/MANIFEST.in @@ -1,4 +1,5 @@ include README.md +include LICENSE.txt recursive-include awsclilinter *.py recursive-exclude tests * recursive-exclude examples * From f2e36331f3fccef78909ee14be3f0fc9aae1b0e2 Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 10:48:01 -0500 Subject: [PATCH 11/14] Update MANIFEST and pyproject. --- awsclilinter/MANIFEST.in | 2 -- awsclilinter/pyproject.toml | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/awsclilinter/MANIFEST.in b/awsclilinter/MANIFEST.in index 8d19f2886e8e..a7aeb71e7d91 100644 --- a/awsclilinter/MANIFEST.in +++ b/awsclilinter/MANIFEST.in @@ -1,5 +1,3 @@ include README.md include LICENSE.txt -recursive-include awsclilinter *.py -recursive-exclude tests * recursive-exclude examples * diff --git a/awsclilinter/pyproject.toml b/awsclilinter/pyproject.toml index 3b070b8cfe53..bd891e3fe965 100644 --- a/awsclilinter/pyproject.toml +++ b/awsclilinter/pyproject.toml @@ -59,3 +59,6 @@ testpaths = ["tests"] python_files = ["test_*.py"] python_classes = ["Test*"] python_functions = ["test_*"] + +[tool.setuptools] +packages = ["awsclilinter"] \ No newline at end of file From a855a4a41f59d7d53e0b98a5c8375803281006f4 Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 11:12:29 -0500 Subject: [PATCH 12/14] Fix tests. --- awsclilinter/awsclilinter/cli.py | 27 ++++++++++++--------------- awsclilinter/awsclilinter/linter.py | 10 ++++++---- awsclilinter/tests/test_cli.py | 1 + awsclilinter/tests/test_linter.py | 15 +++++++++------ 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/awsclilinter/awsclilinter/cli.py b/awsclilinter/awsclilinter/cli.py index b957aea41219..96b4a6f5edb1 100644 --- a/awsclilinter/awsclilinter/cli.py +++ b/awsclilinter/awsclilinter/cli.py @@ -2,7 +2,7 @@ import difflib import sys from pathlib import Path -from typing import Dict, List, Tuple, Optional +from typing import Dict, List, Optional, Tuple from ast_grep_py.ast_grep_py import SgRoot @@ -25,9 +25,11 @@ def prompt_user_choice_interactive_mode() -> str: """Get user input for interactive mode.""" while True: - choice = input( - "\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: " - ).lower().strip() + choice = ( + input("\nApply this fix? [y]es, [n]o, [u]pdate all, [s]ave and exit, [q]uit: ") + .lower() + .strip() + ) if choice in ["y", "n", "u", "s", "q"]: return choice print("Invalid choice. Please enter y, n, u, s, or q.") @@ -148,25 +150,19 @@ def interactive_mode_for_rule( # Accept this and all remaining findings for this rule. accepted_findings.extend(findings[i:]) if accepted_findings: - ast = parse( - linter.apply_fixes(ast, accepted_findings) - ) + ast = parse(linter.apply_fixes(ast, accepted_findings)) return ast, True, last_choice elif last_choice == "s": # Apply accepted findings and stop processing if accepted_findings: - ast = parse( - linter.apply_fixes(ast, accepted_findings) - ) + ast = parse(linter.apply_fixes(ast, accepted_findings)) return ast, len(accepted_findings) > 0, last_choice elif last_choice == "q": print("Quit without saving.") sys.exit(0) if accepted_findings: - ast = parse( - linter.apply_fixes(ast, accepted_findings) - ) + ast = parse(linter.apply_fixes(ast, accepted_findings)) return ast, True, last_choice return ast, False, last_choice @@ -238,15 +234,16 @@ def main(): ) if changes_made: + current_script = current_ast.root().text() any_changes = True finding_offset += len(rule_findings) - if last_choice == 's': + if last_choice == "s": break # If user chose 'u', auto-apply all remaining rules - if last_choice == 'u': + if last_choice == "u": for remaining_rule in rules[rule_index + 1 :]: remaining_findings = linter.lint_for_rule(current_ast, remaining_rule) if remaining_findings: diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index 93bb22eae2da..de3938a0e1cb 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -7,7 +7,8 @@ def parse(script_content: str) -> SgRoot: """Parse the bash script content into an AST.""" - return SgRoot(script_content, 'bash') + return SgRoot(script_content, "bash") + def lint(ast: SgRoot, rules: List[LintRule]) -> List[Tuple[LintFinding, LintRule]]: """Lint the AST and return all findings with their associated rules.""" @@ -16,9 +17,8 @@ def lint(ast: SgRoot, rules: List[LintRule]) -> List[Tuple[LintFinding, LintRule findings = rule.check(ast) for finding in findings: findings_with_rules.append((finding, rule)) - return sorted( - findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos) - ) + return sorted(findings_with_rules, key=lambda fr: (fr[0].edit.start_pos, fr[0].edit.end_pos)) + def lint_for_rule(ast: SgRoot, rule: LintRule) -> List[LintFinding]: """Lint the script for a single rule. @@ -33,6 +33,7 @@ def lint_for_rule(ast: SgRoot, rule: LintRule) -> List[LintFinding]: findings = rule.check(ast) return sorted(findings, key=lambda f: (f.edit.start_pos, f.edit.end_pos)) + def apply_fixes(ast: SgRoot, findings: List[LintFinding]) -> str: """Apply to the AST for a single rule. @@ -49,4 +50,5 @@ def apply_fixes(ast: SgRoot, findings: List[LintFinding]) -> str: # Collect all edits - they should be non-overlapping within a single rule edits = [f.edit for f in findings] + print(edits) return root.commit_edits(edits) diff --git a/awsclilinter/tests/test_cli.py b/awsclilinter/tests/test_cli.py index e90f07c763bc..3e43d066a61a 100644 --- a/awsclilinter/tests/test_cli.py +++ b/awsclilinter/tests/test_cli.py @@ -113,6 +113,7 @@ def test_interactive_mode_accept_all(self, tmp_path): with patch("builtins.input", side_effect=["y", "y", "y", "y"]): main() fixed_content = output_file.read_text() + print(fixed_content) # 2 commands, 2 rules = 4 findings, so 2 of each flag assert fixed_content.count("--cli-binary-format") == 2 assert fixed_content.count("--no-cli-paginate") == 2 diff --git a/awsclilinter/tests/test_linter.py b/awsclilinter/tests/test_linter.py index 1e7b3ce8800c..86fb0fed0202 100644 --- a/awsclilinter/tests/test_linter.py +++ b/awsclilinter/tests/test_linter.py @@ -2,13 +2,14 @@ from awsclilinter.rules.base64_rule import Base64BinaryFormatRule -class TestScriptLinter: - """Test cases for ScriptLinter.""" +class TestLinter: + """Test cases for linter functions.""" def test_lint_finds_issues(self): """Test that linter finds issues in script.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" - findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) + ast = linter.parse(script) + findings_with_rules = linter.lint(ast, [Base64BinaryFormatRule()]) assert len(findings_with_rules) == 1 finding, rule = findings_with_rules[0] @@ -18,9 +19,10 @@ def test_lint_finds_issues(self): def test_apply_fixes(self): """Test that fixes are applied correctly.""" script = "aws secretsmanager put-secret-value --secret-id secret1213 --secret-binary file://data.json" - findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) + ast = linter.parse(script) + findings_with_rules = linter.lint(ast, [Base64BinaryFormatRule()]) findings = [f for f, _ in findings_with_rules] - fixed = linter.apply_fixes(script, findings) + fixed = linter.apply_fixes(ast, findings) assert "--cli-binary-format raw-in-base64-out" in fixed assert "file://data.json" in fixed @@ -33,7 +35,8 @@ def test_multiple_issues(self): " aws kinesis put-record --stream-name samplestream " "--data file://data --partition-key samplepartitionkey" ) - findings_with_rules = linter.lint(script, [Base64BinaryFormatRule()]) + ast = linter.parse(script) + findings_with_rules = linter.lint(ast, [Base64BinaryFormatRule()]) # 2 commands, 1 rule = 2 findings assert len(findings_with_rules) == 2 From 500117fd57cf6272e3d881b237a908dabd23ab0c Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 11:18:22 -0500 Subject: [PATCH 13/14] Update base.py docs. --- awsclilinter/awsclilinter/rules/base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/awsclilinter/awsclilinter/rules/base.py b/awsclilinter/awsclilinter/rules/base.py index 2fe0d311f0e0..4d512784a6b2 100644 --- a/awsclilinter/awsclilinter/rules/base.py +++ b/awsclilinter/awsclilinter/rules/base.py @@ -38,6 +38,5 @@ def check(self, root: SgRoot) -> List[LintFinding]: Args: root: The AST root to check - start_pos: Starting position in the script to begin checking from """ pass From 3f56a6328b1d5a0486d2fd92b23bde2133ef6eda Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 20 Nov 2025 11:24:00 -0500 Subject: [PATCH 14/14] Remove leftover print from testing. --- awsclilinter/awsclilinter/linter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/awsclilinter/awsclilinter/linter.py b/awsclilinter/awsclilinter/linter.py index de3938a0e1cb..8f28928465dc 100644 --- a/awsclilinter/awsclilinter/linter.py +++ b/awsclilinter/awsclilinter/linter.py @@ -50,5 +50,4 @@ def apply_fixes(ast: SgRoot, findings: List[LintFinding]) -> str: # Collect all edits - they should be non-overlapping within a single rule edits = [f.edit for f in findings] - print(edits) return root.commit_edits(edits)