Skip to content

Commit 66a5714

Browse files
committed
Address review comments
1 parent 7715656 commit 66a5714

File tree

3 files changed

+29
-63
lines changed

3 files changed

+29
-63
lines changed

src/tools/jsondocck/src/cache.rs

+8-14
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::error::CkError;
22
use serde_json::Value;
33
use std::collections::HashMap;
4-
use std::fs;
54
use std::path::{Path, PathBuf};
5+
use std::{fs, io};
66

77
#[derive(Debug)]
88
pub struct Cache {
@@ -15,28 +15,25 @@ pub struct Cache {
1515
impl Cache {
1616
pub fn new(doc_dir: &str) -> Cache {
1717
Cache {
18-
root: <str as AsRef<Path>>::as_ref(doc_dir).to_owned(),
18+
root: Path::new(doc_dir).to_owned(),
1919
files: HashMap::new(),
2020
values: HashMap::new(),
2121
last_path: None,
2222
}
2323
}
2424

25-
fn resolve_path(&mut self, path: &String) -> Result<PathBuf, CkError> {
25+
fn resolve_path(&mut self, path: &String) -> PathBuf {
2626
if path != "-" {
2727
let resolve = self.root.join(path);
2828
self.last_path = Some(resolve.clone());
29-
Ok(resolve)
29+
resolve
3030
} else {
31-
match &self.last_path {
32-
Some(p) => Ok(p.clone()),
33-
None => unreachable!(),
34-
}
31+
self.last_path.as_ref().unwrap().clone()
3532
}
3633
}
3734

38-
pub fn get_file(&mut self, path: &String) -> Result<String, CkError> {
39-
let path = self.resolve_path(path)?;
35+
pub fn get_file(&mut self, path: &String) -> Result<String, io::Error> {
36+
let path = self.resolve_path(path);
4037

4138
if let Some(f) = self.files.get(&path) {
4239
return Ok(f.clone());
@@ -47,24 +44,21 @@ impl Cache {
4744
self.files.insert(path, file.clone());
4845

4946
Ok(file)
50-
// Err(_) => Err(CkError::FailedCheck(format!("File {:?} does not exist / could not be opened", path)))
5147
}
5248

5349
pub fn get_value(&mut self, path: &String) -> Result<Value, CkError> {
54-
let path = self.resolve_path(path)?;
50+
let path = self.resolve_path(path);
5551

5652
if let Some(v) = self.values.get(&path) {
5753
return Ok(v.clone());
5854
}
5955

6056
let file = fs::File::open(&path)?;
61-
// Err(_) => return Err(CkError::FailedCheck(format!("File {:?} does not exist / could not be opened", path)))
6257

6358
let val = serde_json::from_reader::<_, Value>(file)?;
6459

6560
self.values.insert(path, val.clone());
6661

6762
Ok(val)
68-
// Err(_) => Err(CkError::FailedCheck(format!("File {:?} did not contain valid JSON", path)))
6963
}
7064
}

src/tools/jsondocck/src/config.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
2222
panic!()
2323
}
2424

25-
let matches = &match opts.parse(args_) {
26-
Ok(m) => m,
27-
Err(f) => panic!("{:?}", f),
28-
};
25+
let matches = opts.parse(args_).unwrap();
2926

3027
if matches.opt_present("h") || matches.opt_present("help") {
3128
let message = format!("Usage: {} <doc-dir> <template>", argv0);
3229
println!("{}", opts.usage(&message));
33-
println!();
34-
panic!()
30+
std::process::exit(1);
3531
}
3632

3733
Config {

src/tools/jsondocck/src/main.rs

+19-43
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,15 @@ impl CommandKind {
6262
return false;
6363
}
6464

65-
match self {
66-
CommandKind::Has => {
67-
if args[0] == "-" && command_num == 0 {
68-
print_err(
69-
&format!("Tried to use the previous path in the first command"),
70-
lineno,
71-
);
72-
return false;
73-
}
74-
}
75-
CommandKind::Count => {
76-
if args[0] == "-" && command_num == 0 {
77-
print_err(
78-
&format!("Tried to use the previous path in the first command"),
79-
lineno,
80-
);
81-
return false;
82-
}
83-
if args[2].parse::<usize>().is_err() {
84-
print_err(&format!("Third argument to @count must be a valid usize"), lineno);
85-
return false;
86-
}
65+
if args[0] == "-" && command_num == 0 {
66+
print_err(&format!("Tried to use the previous path in the first command"), lineno);
67+
return false;
68+
}
69+
70+
if let CommandKind::Count = self {
71+
if args[2].parse::<usize>().is_err() {
72+
print_err(&format!("Third argument to @count must be a valid usize"), lineno);
73+
return false;
8774
}
8875
}
8976

@@ -132,10 +119,7 @@ fn get_commands(template: &str) -> Result<Vec<Command>, ()> {
132119
None => continue,
133120
};
134121

135-
let negated = match cap.name("negated") {
136-
Some(m) => m.as_str() == "!",
137-
None => false,
138-
};
122+
let negated = cap.name("negated").unwrap().as_str() == "!";
139123
let cmd = cap.name("cmd").unwrap().as_str();
140124

141125
let cmd = match cmd {
@@ -209,26 +193,18 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
209193
Err(_) => false,
210194
}
211195
}
212-
_ => {
213-
unreachable!()
214-
}
196+
_ => unreachable!(),
215197
}
216198
}
217199
CommandKind::Count => {
218-
match command.args.len() {
219-
// @count <path> <jsonpath> <count> = Check that the jsonpath matches exactly [count] times
220-
3 => {
221-
let expected: usize = command.args[2].parse().unwrap();
222-
223-
let val = cache.get_value(&command.args[0])?;
224-
match select(&val, &command.args[1]) {
225-
Ok(results) => results.len() == expected,
226-
Err(_) => false,
227-
}
228-
}
229-
_ => {
230-
unreachable!()
231-
}
200+
// @count <path> <jsonpath> <count> = Check that the jsonpath matches exactly [count] times
201+
assert_eq!(command.args.len(), 3);
202+
let expected: usize = command.args[2].parse().unwrap();
203+
204+
let val = cache.get_value(&command.args[0])?;
205+
match select(&val, &command.args[1]) {
206+
Ok(results) => results.len() == expected,
207+
Err(_) => false,
232208
}
233209
}
234210
};

0 commit comments

Comments
 (0)