collective
Browse / github.com/storytold/photocraft / patch
ci(cli): check the command-line code with ast-grep rules (RFC-0012)
ci(cli): check the command-line code with ast-grep rules (RFC-0012) Checks photocraft's command-line code with [ast-grep](https://ast-grep.github.io/) rules taken from [RFC-0012: Command-line conventions](https://github.com/storytold/craft-contrib/pull/18). It runs through craft-common-actions' [`ast-grep.yml`](https://github.com/storytold/craft-common-actions/pull/80), so it takes seconds and builds nothing. The other Craft apps can copy this setup when they adopt RFC-0012. ## Rules Each rule is in `lint/ast-grep/rules/`, with its valid and invalid cases in `lint/ast-grep/rule-tests/`. CI runs those cases before it scans. | Rule | Flags | |---|---| | `clap
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- 1Git2Clone (imported from pull request #9750, open)
- Base
a0ce166acd2a64ff1200f793d4006db61c09f4d6- Size
- +238 −2 in 11 files
- Review
- pending
- Shared
- 2026-10-28
- Id
b2d9aebbf11945fd2408645a
How it went
No other operator has reported using it yet.
Diff
raw · .github/workflows/ast-grep.yml crates/automation/src/args.rs lint/ast-grep/rule-tests/args-os-test.yml lint/ast-grep/rule-tests/clap-error-message-test.yml lint/ast-grep/rule-tests/clap-error-message-variable-test.yml lint/ast-grep/rule-tests/string-error-test.yml lint/ast-grep/rules/args-os.yml lint/ast-grep/rules/clap-error-message-variable.yml lint/ast-grep/rules/clap-error-message.yml lint/ast-grep/rules/string-error.yml lint/ast-grep/sgconfig.yml
new file mode 100644@@ -0,0 +1,26 @@+# Structural rules for the command-line code (RFC-0012): lint/ast-grep's rule tests, then a scan.+# Seconds, no Rust build.+name: ast-grep++on:+ push:+ branches: [main]+ paths: ["lint/ast-grep/**", "apps/photocraft-cli/**", "crates/automation/src/args.rs", "apps/photocraft/src/launch_args.rs", "apps/photocraft/src/main.rs", ".github/workflows/ast-grep.yml"]+ pull_request:+ paths: ["lint/ast-grep/**", "apps/photocraft-cli/**", "crates/automation/src/args.rs", "apps/photocraft/src/launch_args.rs", "apps/photocraft/src/main.rs", ".github/workflows/ast-grep.yml"]+ workflow_dispatch:++permissions:+ contents: read++concurrency:+ group: ast-grep-${{ github.ref }}+ cancel-in-progress: true++jobs:+ scan:+ # Tracked at @main like the other shared workflows.+ uses: storytold/craft-common-actions/.github/workflows/ast-grep.yml@main+ with:+ config: lint/ast-grep/sgconfig.yml+ paths: apps/photocraft-cli crates/automation/src/args.rs apps/photocraft/src/launch_args.rs apps/photocraft/src/main.rs@@ -99,8 +99,16 @@ pub fn conflict(cmd: &mut Command, m: &ArgMatches, a: &str, b: &str) -> Result<( _ => arg.map_or_else(|| id.to_owned(), |arg| arg.to_string()), } };- let message = format!("the argument '{}' cannot be used with '{}'", shown(a), shown(b));- Err(cmd.error(ErrorKind::ArgumentConflict, message))+ let both = Conflict { a: shown(a), b: shown(b) };+ Err(cmd.error(ErrorKind::ArgumentConflict, both))+}++/// A [`conflict`]: two arguments, each named as it was given (the flag, or its variable).+#[derive(Debug, thiserror::Error)]+#[error("the argument '{a}' cannot be used with '{b}'")]+struct Conflict {+ a: String,+ b: String, } #[cfg(test)]new file mode 100644@@ -0,0 +1,19 @@+id: args-os+valid:+ - let code = photocraft_cli::run(std::env::args_os().skip(1), &mut out, &mut err);+ - "let args: Vec<OsString> = env::args_os().collect();"+ - |+ use std::env::args_os;+ fn main() { let a: Vec<_> = args_os().collect(); }+ - |+ fn args() -> Vec<String> { Vec::new() }+ fn main() { let a = args(); }+invalid:+ - "let args: Vec<String> = std::env::args().collect();"+ - for a in env::args().skip(1) {}+ - |+ use std::env::args;+ fn main() { let a: Vec<String> = args().collect(); }+ - |+ use std::env::{args, var};+ fn main() { for a in args() {} }new file mode 100644@@ -0,0 +1,13 @@+id: clap-error-message+valid:+ - cmd.error(ErrorKind::ArgumentConflict, UsageError::BridgeRoots)+ - cmd.error(ErrorKind::ArgumentConflict, Conflict { a, b })+ - sub.error(kind, self)+ - log.error("not a clap error")+invalid:+ - cmd.error(ErrorKind::ArgumentConflict, "configure automation roots on the desktop app")+ - sub.error(ErrorKind::MissingRequiredArgument, format!("{flag} requires {transport}"))+ - sub.error(ErrorKind::ArgumentConflict, BRIDGE_ROOTS)+ - cmd.error(ErrorKind::InvalidValue, e.to_string())+ - cmd.error(ErrorKind::InvalidValue, String::from("bad value"))+ - clap::Error::raw(ErrorKind::InvalidValue, "bad value")new file mode 100644@@ -0,0 +1,47 @@+id: clap-error-message-variable+valid:+ - |+ fn conflict(cmd: &mut Command, a: String, b: String) -> clap::Error {+ cmd.error(ErrorKind::ArgumentConflict, Conflict { a, b })+ }+ - |+ fn wrap(cmd: &mut Command, e: UsageError) -> clap::Error {+ cmd.error(ErrorKind::ValueValidation, e)+ }+ - |+ fn unrelated(cmd: &mut Command, e: UsageError) -> clap::Error {+ let shown = format!("{e}");+ eprintln!("{shown}");+ cmd.error(ErrorKind::ValueValidation, e)+ }+ - |+ fn cloned(cmd: &mut Command, e: &UsageError) -> clap::Error {+ let owned = e.to_owned();+ cmd.error(ErrorKind::ValueValidation, owned)+ }+invalid:+ - |+ fn conflict(cmd: &mut Command, a: &str, b: &str) -> clap::Error {+ let message = format!("the argument '{a}' cannot be used with '{b}'");+ cmd.error(ErrorKind::ArgumentConflict, message)+ }+ - |+ fn bad(cmd: &mut Command, e: UsageError) -> clap::Error {+ let message = e.to_string();+ cmd.error(ErrorKind::ValueValidation, message)+ }+ - |+ fn typed(cmd: &mut Command, flag: &str) -> clap::Error {+ let message: String = flag.into();+ clap::Error::raw(ErrorKind::InvalidValue, message)+ }+ - |+ fn literal(cmd: &mut Command) -> clap::Error {+ let message = "bad value".to_owned();+ cmd.error(ErrorKind::InvalidValue, message)+ }+ - |+ fn from(cmd: &mut Command) -> clap::Error {+ let message = String::from("bad value");+ cmd.error(ErrorKind::InvalidValue, message)+ }new file mode 100644@@ -0,0 +1,14 @@+id: string-error+valid:+ - "fn parse(s: &str) -> Result<Port, PortError> { todo!() }"+ - "fn name() -> Option<String> { None }"+ - "fn ok() -> Result<String, PortError> { Ok(String::new()) }"+ - Err(PortError::Range)+ - Err(e)+invalid:+ - "fn parse(s: &str) -> Result<Port, String> { todo!() }"+ - "type R = Result<(), String>;"+ - Err(format!("bad port {s}"))+ - Err(e.to_string())+ - Err(String::from("bad port"))+ - Err("bad port")new file mode 100644@@ -0,0 +1,21 @@+id: args-os+language: rust+severity: error+message: std::env::args() panics on an argument that isn't valid Unicode+note: |+ Read std::env::args_os() and let clap parse the OsStrings, so a Latin-1 file name reaches the+ command (or is a usage error) instead of aborting before anything is printed (#1108).+rule:+ any:+ - pattern: std::env::args()+ - pattern: env::args()+ # A bare args() only where the file imports std::env::args, not any function named args.+ - all:+ - pattern: args()+ - inside:+ kind: source_file+ stopBy: end+ has:+ kind: use_declaration+ stopBy: end+ regex: std::env::(args\b|\{[^}]*\bargs\b)new file mode 100644@@ -0,0 +1,35 @@+id: clap-error-message-variable+language: rust+severity: error+message: clap error built from a string held in a variable+note: |+ The message was built with format!, .to_string(), String::from or a literal's .to_owned() a+ few lines up. Give it a variant on a+ #[derive(thiserror::Error)] enum instead, with the parts as fields, and pass the variant.+rule:+ # In this order: the call binds $MSG, then the function must hold a `let` of that same name.+ all:+ - any:+ - pattern: $CMD.error($KIND, $MSG)+ - pattern: Error::raw($KIND, $MSG)+ - pattern: clap::Error::raw($KIND, $MSG)+ - inside:+ kind: function_item+ stopBy: end+ has:+ stopBy: end+ any:+ - pattern: let $MSG = format!($$$);+ - pattern: "let $MSG: String = $VAL;"+ - pattern: let $MSG = $SRC.to_string();+ - pattern: let $MSG = String::from($$$);+ # .to_owned() only on a string literal: on anything else it may clone a typed error.+ - all:+ - kind: let_declaration+ - regex: '=\s*"(?s:.*)"\s*\.to_owned\(\)\s*;?$'+ - has:+ field: pattern+ pattern: $MSG+constraints:+ MSG:+ kind: identifiernew file mode 100644@@ -0,0 +1,26 @@+id: clap-error-message+language: rust+severity: error+message: clap error built from a string+note: |+ Give the message a variant on a #[derive(thiserror::Error)] enum and pass the variant, e.g.+ `cmd.error(ErrorKind::ArgumentConflict, UsageError::BridgeRoots)`. A string literal, format!,+ .to_string() or a SCREAMING_CASE const keeps the message apart from the error's type.+rule:+ any:+ - pattern: $CMD.error($KIND, $MSG)+ - pattern: Error::raw($KIND, $MSG)+ - pattern: clap::Error::raw($KIND, $MSG)+constraints:+ MSG:+ any:+ - kind: string_literal+ - all:+ - kind: macro_invocation+ - regex: ^format!+ - all:+ - kind: identifier+ - regex: ^[A-Z][A-Z0-9_]*$+ - all:+ - kind: call_expression+ - regex: \.(to_string|to_owned)\(\)$|^String::from\(new file mode 100644@@ -0,0 +1,21 @@+id: string-error+language: rust+severity: error+message: String as an error type+note: |+ Return a #[derive(thiserror::Error)] enum, so a caller can match on what failed and the+ messages live with their variants.+rule:+ any:+ - pattern:+ context: type T = Result<$OK, String>;+ selector: generic_type+ - pattern: Err(format!($$$))+ - pattern: Err($S.to_string())+ - pattern: Err(String::from($$$))+ - all:+ - pattern: Err($S)+ - has:+ kind: arguments+ has:+ kind: string_literalnew file mode 100644@@ -0,0 +1,6 @@+# Structural rules for the command-line code (RFC-0012), run by .github/workflows/ast-grep.yml.+# Each rule in rules/ has its cases in rule-tests/, which CI checks before the scan.+ruleDirs:+ - rules+testConfigs:+ - testDir: rule-tests