diff --git a/src/tools/jsondocck/src/cache.rs b/src/tools/jsondocck/src/cache.rs index 9f95f9fb408e7..47512039740b2 100644 --- a/src/tools/jsondocck/src/cache.rs +++ b/src/tools/jsondocck/src/cache.rs @@ -28,7 +28,8 @@ impl Cache { } } - pub fn value(&self) -> &Value { - &self.value + // FIXME: Make this failible, so jsonpath syntax error has line number. + pub fn select(&self, path: &str) -> Vec<&Value> { + jsonpath_lib::select(&self.value, path).unwrap() } } diff --git a/src/tools/jsondocck/src/error.rs b/src/tools/jsondocck/src/error.rs index c4cd79a64fda1..0a3e085b405ba 100644 --- a/src/tools/jsondocck/src/error.rs +++ b/src/tools/jsondocck/src/error.rs @@ -1,29 +1,7 @@ -use std::error::Error; -use std::fmt; - use crate::Command; #[derive(Debug)] -pub enum CkError { - /// A check failed. File didn't exist or failed to match the command - FailedCheck(String, Command), - /// An error triggered by some other error - Induced(Box), -} - -impl fmt::Display for CkError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - CkError::FailedCheck(msg, cmd) => { - write!(f, "Failed check: {} on line {}", msg, cmd.lineno) - } - CkError::Induced(err) => write!(f, "Check failed: {}", err), - } - } -} - -impl From for CkError { - fn from(err: T) -> CkError { - CkError::Induced(Box::new(err)) - } +pub struct CkError { + pub message: String, + pub command: Command, } diff --git a/src/tools/jsondocck/src/main.rs b/src/tools/jsondocck/src/main.rs index c08bbbc769a0e..b6a1d7dfa7a3a 100644 --- a/src/tools/jsondocck/src/main.rs +++ b/src/tools/jsondocck/src/main.rs @@ -1,8 +1,8 @@ use std::borrow::Cow; +use std::process::ExitCode; use std::sync::OnceLock; -use std::{env, fmt, fs}; +use std::{env, fs}; -use jsonpath_lib::select; use regex::{Regex, RegexBuilder}; use serde_json::Value; @@ -14,90 +14,134 @@ use cache::Cache; use config::parse_config; use error::CkError; -fn main() -> Result<(), String> { +fn main() -> ExitCode { let config = parse_config(env::args().collect()); let mut failed = Vec::new(); let mut cache = Cache::new(&config); - let commands = get_commands(&config.template) - .map_err(|_| format!("Jsondocck failed for {}", &config.template))?; + let Ok(commands) = get_commands(&config.template) else { + eprintln!("Jsondocck failed for {}", &config.template); + return ExitCode::FAILURE; + }; for command in commands { - if let Err(e) = check_command(command, &mut cache) { - failed.push(e); + if let Err(message) = check_command(&command, &mut cache) { + failed.push(CkError { command, message }); } } if failed.is_empty() { - Ok(()) + ExitCode::SUCCESS } else { for i in failed { - eprintln!("{}", i); + eprintln!("{}:{}, command failed", config.template, i.command.lineno); + eprintln!("{}", i.message) } - Err(format!("Jsondocck failed for {}", &config.template)) + ExitCode::FAILURE } } #[derive(Debug)] pub struct Command { - negated: bool, kind: CommandKind, - args: Vec, + path: String, lineno: usize, } #[derive(Debug)] -pub enum CommandKind { - Has, - Count, - Is, - IsMany, - Set, +enum CommandKind { + /// `//@ has ` + /// + /// Checks the path exists. + HasPath, + + /// `//@ has ` + /// + /// Check one thing at the path is equal to the value. + HasValue { value: String }, + + /// `//@ !has ` + /// + /// Checks the path doesn't exist. + HasNotPath, + + /// `//@ is ` + /// + /// Check the path is the given value. + Is { value: String }, + + /// `//@ is ...` + /// + /// Check that the path matches to exactly every given value. + IsMany { values: Vec }, + + /// `//@ !is ` + /// + /// Check the path isn't the given value. + IsNot { value: String }, + + /// `//@ count ` + /// + /// Check the path has the expected number of matches. + CountIs { expected: usize }, + + /// `//@ set = ` + Set { variable: String }, } impl CommandKind { - fn validate(&self, args: &[String], lineno: usize) -> bool { - // FIXME(adotinthevoid): We should "parse, don't validate" here, so we avoid ad-hoc - // indexing in check_command. - let count = match self { - CommandKind::Has => (1..=2).contains(&args.len()), - CommandKind::IsMany => args.len() >= 2, - CommandKind::Count | CommandKind::Is => 2 == args.len(), - CommandKind::Set => 3 == args.len(), - }; + /// Returns both the kind and the path. + /// + /// Returns `None` if the command isn't from jsondocck (e.g. from compiletest). + fn parse<'a>(command_name: &str, negated: bool, args: &'a [String]) -> Option<(Self, &'a str)> { + let kind = match (command_name, negated) { + ("count", false) => { + assert_eq!(args.len(), 2); + let expected = args[1].parse().expect("invalid number for `count`"); + Self::CountIs { expected } + } - if !count { - print_err(&format!("Incorrect number of arguments to `{}`", self), lineno); - return false; - } + ("ismany", false) => { + // FIXME: Make this >= 3, and migrate len(values)==1 cases to @is + assert!(args.len() >= 2, "Not enough args to `ismany`"); + let values = args[1..].to_owned(); + Self::IsMany { values } + } - if let CommandKind::Count = self { - if args[1].parse::().is_err() { - print_err( - &format!( - "Second argument to `count` must be a valid usize (got `{}`)", - args[1] - ), - lineno, - ); - return false; + ("is", false) => { + assert_eq!(args.len(), 2); + Self::Is { value: args[1].clone() } + } + ("is", true) => { + assert_eq!(args.len(), 2); + Self::IsNot { value: args[1].clone() } } - } - true - } -} + ("set", false) => { + assert_eq!(args.len(), 3); + assert_eq!(args[1], "="); + return Some((Self::Set { variable: args[0].clone() }, &args[2])); + } -impl fmt::Display for CommandKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let text = match self { - CommandKind::Has => "has", - CommandKind::IsMany => "ismany", - CommandKind::Count => "count", - CommandKind::Is => "is", - CommandKind::Set => "set", + ("has", false) => match args { + [_path] => Self::HasPath, + [_path, value] => Self::HasValue { value: value.clone() }, + _ => panic!("`//@ has` must have 2 or 3 arguments, but got {args:?}"), + }, + ("has", true) => { + assert_eq!(args.len(), 1, "args={args:?}"); + Self::HasNotPath + } + + (_, false) if KNOWN_DIRECTIVE_NAMES.contains(&command_name) => { + return None; + } + _ => { + panic!("Invalid command `//@ {}{command_name}`", if negated { "!" } else { "" }) + } }; - write!(f, "{}", text) + + Some((kind, &args[0])) } } @@ -125,8 +169,7 @@ fn print_err(msg: &str, lineno: usize) { // See . include!(concat!(env!("CARGO_MANIFEST_DIR"), "/../compiletest/src/directive-list.rs")); -/// Get a list of commands from a file. Does the work of ensuring the commands -/// are syntactically valid. +/// Get a list of commands from a file. fn get_commands(template: &str) -> Result, ()> { let mut commands = Vec::new(); let mut errors = false; @@ -142,217 +185,102 @@ fn get_commands(template: &str) -> Result, ()> { let negated = cap.name("negated").unwrap().as_str() == "!"; - let cmd = match cap.name("cmd").unwrap().as_str() { - "has" => CommandKind::Has, - "count" => CommandKind::Count, - "is" => CommandKind::Is, - "ismany" => CommandKind::IsMany, - "set" => CommandKind::Set, - // FIXME: See the comment above the `include!(...)`. - cmd if KNOWN_DIRECTIVE_NAMES.contains(&cmd) => continue, - cmd => { - print_err(&format!("Unrecognized command name `{cmd}`"), lineno); - errors = true; - continue; - } - }; - - let args = cap.name("args").map_or(Some(vec![]), |m| shlex::split(m.as_str())); - - let args = match args { + let args_str = &cap["args"]; + let args = match shlex::split(args_str) { Some(args) => args, None => { - print_err( - &format!( - "Invalid arguments to shlex::split: `{}`", - cap.name("args").unwrap().as_str() - ), - lineno, - ); + print_err(&format!("Invalid arguments to shlex::split: `{args_str}`",), lineno); errors = true; continue; } }; - if !cmd.validate(&args, lineno) { - errors = true; - continue; + if let Some((kind, path)) = CommandKind::parse(&cap["cmd"], negated, &args) { + commands.push(Command { kind, lineno, path: path.to_owned() }) } - - commands.push(Command { negated, kind: cmd, args, lineno }) } if !errors { Ok(commands) } else { Err(()) } } -/// Performs the actual work of ensuring a command passes. Generally assumes the command -/// is syntactically valid. -fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> { - // FIXME: Be more granular about why, (e.g. syntax error, count not equal) - let result = match command.kind { - CommandKind::Has => { - match command.args.len() { - // `has `: Check that `jsonpath` exists. - 1 => { - let val = cache.value(); - let results = select(val, &command.args[0]).unwrap(); - !results.is_empty() - } - // `has `: Check *any* item matched by `jsonpath` equals `value`. - 2 => { - let val = cache.value().clone(); - let results = select(&val, &command.args[0]).unwrap(); - let pat = string_to_value(&command.args[1], cache); - let has = results.contains(&pat.as_ref()); - // Give better error for when `has` check fails. - if !command.negated && !has { - return Err(CkError::FailedCheck( - format!( - "{} matched to {:?} but didn't have {:?}", - &command.args[0], - results, - pat.as_ref() - ), - command, - )); - } else { - has - } - } - _ => unreachable!(), +/// Performs the actual work of ensuring a command passes. +fn check_command(command: &Command, cache: &mut Cache) -> Result<(), String> { + let matches = cache.select(&command.path); + match &command.kind { + CommandKind::HasPath => { + if matches.is_empty() { + return Err("matched to no values".to_owned()); + } + } + CommandKind::HasNotPath => { + if !matches.is_empty() { + return Err(format!("matched to {matches:?}, but wanted no matches")); + } + } + CommandKind::HasValue { value } => { + let want_value = string_to_value(value, cache); + if !matches.contains(&want_value.as_ref()) { + return Err(format!("matched to {matches:?}, which didn't contain {want_value:?}")); + } + } + CommandKind::Is { value } => { + let want_value = string_to_value(value, cache); + let matched = get_one(&matches)?; + if matched != want_value.as_ref() { + return Err(format!("matched to {matched:?} but want {want_value:?}")); + } + } + CommandKind::IsNot { value } => { + let wantnt_value = string_to_value(value, cache); + let matched = get_one(&matches)?; + if matched == wantnt_value.as_ref() { + return Err(format!("got value {wantnt_value:?}, but want anything else")); } } - // `ismany ` - CommandKind::IsMany => { - assert!(!command.negated, "`ismany` may not be negated"); - let (query, values) = if let [query, values @ ..] = &command.args[..] { - (query, values) - } else { - unreachable!("Checked in CommandKind::validate") - }; - let val = cache.value(); - let got_values = select(val, &query).unwrap(); + CommandKind::IsMany { values } => { // Serde json doesn't implement Ord or Hash for Value, so we must // use a Vec here. While in theory that makes setwize equality // O(n^2), in practice n will never be large enough to matter. let expected_values = values.iter().map(|v| string_to_value(v, cache)).collect::>(); - if expected_values.len() != got_values.len() { - return Err(CkError::FailedCheck( - format!( - "Expected {} values, but `{}` matched to {} values ({:?})", - expected_values.len(), - query, - got_values.len(), - got_values - ), - command, + if expected_values.len() != matches.len() { + return Err(format!( + "Expected {} values, but matched to {} values ({:?})", + expected_values.len(), + matches.len(), + matches )); }; - for got_value in got_values { + for got_value in matches { if !expected_values.iter().any(|exp| &**exp == got_value) { - return Err(CkError::FailedCheck( - format!("`{}` has match {:?}, which was not expected", query, got_value), - command, - )); + return Err(format!("has match {got_value:?}, which was not expected",)); } } - true - } - // `count `: Check that `jsonpath` matches exactly `count` times. - CommandKind::Count => { - assert_eq!(command.args.len(), 2); - let expected: usize = command.args[1].parse().unwrap(); - let val = cache.value(); - let results = select(val, &command.args[0]).unwrap(); - let eq = results.len() == expected; - if !command.negated && !eq { - return Err(CkError::FailedCheck( - format!( - "`{}` matched to `{:?}` with length {}, but expected length {}", - &command.args[0], - results, - results.len(), - expected - ), - command, - )); - } else { - eq - } } - // `has `: Check` *exactly one* item matched by `jsonpath`, and it equals `value`. - CommandKind::Is => { - assert_eq!(command.args.len(), 2); - let val = cache.value().clone(); - let results = select(&val, &command.args[0]).unwrap(); - let pat = string_to_value(&command.args[1], cache); - let is = results.len() == 1 && results[0] == pat.as_ref(); - if !command.negated && !is { - return Err(CkError::FailedCheck( - format!( - "{} matched to {:?}, but expected {:?}", - &command.args[0], - results, - pat.as_ref() - ), - command, + CommandKind::CountIs { expected } => { + if *expected != matches.len() { + return Err(format!( + "matched to `{matches:?}` with length {}, but expected length {expected}", + matches.len(), )); - } else { - is } } - // `set = ` - CommandKind::Set => { - assert!(!command.negated, "`set` may not be negated"); - assert_eq!(command.args.len(), 3); - assert_eq!(command.args[1], "=", "Expected an `=`"); - let val = cache.value().clone(); - let results = select(&val, &command.args[2]).unwrap(); - assert_eq!( - results.len(), - 1, - "Expected 1 match for `{}` (because of `set`): matched to {:?}", - command.args[2], - results - ); - match results.len() { - 0 => false, - 1 => { - let r = cache.variables.insert(command.args[0].clone(), results[0].clone()); - assert!(r.is_none(), "Name collision: {} is duplicated", command.args[0]); - true - } - _ => { - panic!( - "Got multiple results in `set` for `{}`: {:?}", - &command.args[2], results, - ); - } - } + CommandKind::Set { variable } => { + let value = get_one(&matches)?; + let r = cache.variables.insert(variable.to_owned(), value.clone()); + assert!(r.is_none(), "name collision: {variable:?} is duplicated"); } - }; + } - if result == command.negated { - if command.negated { - Err(CkError::FailedCheck( - format!("`!{} {}` matched when it shouldn't", command.kind, command.args.join(" ")), - command, - )) - } else { - // FIXME: In the future, try 'peeling back' each step, and see at what level the match failed - Err(CkError::FailedCheck( - format!( - "`{} {}` didn't match when it should", - command.kind, - command.args.join(" ") - ), - command, - )) - } - } else { - Ok(()) + Ok(()) +} + +fn get_one<'a>(matches: &[&'a Value]) -> Result<&'a Value, String> { + match matches { + [] => Err("matched to no values".to_owned()), + [matched] => Ok(matched), + _ => Err(format!("matched to multiple values {matches:?}, but want exactly 1")), } }