Skip to content

Commit

Permalink
Match toolkit clippy to CI settings
Browse files Browse the repository at this point in the history
I've had a few PRs fail CI but pass `toolkit check pr` because there was
a settings mismatch.
  • Loading branch information
drbrain committed Nov 7, 2023
1 parent 1874082 commit 5fecf7f
Show file tree
Hide file tree
Showing 37 changed files with 589 additions and 430 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ name: continuous-integration
env:
NUSHELL_CARGO_PROFILE: ci
NU_LOG_LEVEL: DEBUG
# If changing these settings also change toolkit.nu
CLIPPY_OPTIONS: "-D warnings -D clippy::unwrap_used"

jobs:
Expand Down Expand Up @@ -47,9 +48,10 @@ jobs:
- name: cargo fmt
run: cargo fmt --all -- --check

# If changing these settings also change toolkit.nu
- name: Clippy
run: cargo clippy --workspace ${{ matrix.flags }} --exclude nu_plugin_* -- $CLIPPY_OPTIONS

# In tests we don't have to deny unwrap
- name: Clippy of tests
run: cargo clippy --tests --workspace ${{ matrix.flags }} --exclude nu_plugin_* -- -D warnings
Expand Down
9 changes: 3 additions & 6 deletions crates/nu-cli/src/completions/command_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,9 @@ mod command_completions_tests {
working_set.render()
};

let result = engine_state.merge_delta(delta);
assert!(
result.is_ok(),
"Merge delta has failed: {}",
result.err().unwrap()
);
if let Err(e) = engine_state.merge_delta(delta) {
unreachable!("Merge delta has failed: {e:?}");
}

let is_passthrough_command = is_passthrough_command(engine_state.get_file_contents());
assert_eq!(
Expand Down
9 changes: 3 additions & 6 deletions crates/nu-cli/src/completions/completer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,12 +514,9 @@ mod completer_tests {
working_set.render()
};

let result = engine_state.merge_delta(delta);
assert!(
result.is_ok(),
"Error merging delta: {:?}",
result.err().unwrap()
);
if let Err(e) = engine_state.merge_delta(delta) {
unreachable!("Error merging delta: {e:?}");
}

let mut completer = NuCompleter::new(engine_state.into(), Stack::new());
let dataset = vec![
Expand Down
18 changes: 9 additions & 9 deletions crates/nu-cli/src/reedline_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -987,13 +987,13 @@ mod test {
};

let span = Span::test_data();
let b = EventType::try_from_record(&event, span).unwrap();
let b = EventType::try_from_record(&event, span).expect("event type");
assert!(matches!(b, EventType::Send(_)));

let event = Value::test_record(event);
let config = Config::default();

let parsed_event = parse_event(&event, &config).unwrap();
let parsed_event = parse_event(&event, &config).expect("event");
assert_eq!(parsed_event, Some(ReedlineEvent::Enter));
}

Expand All @@ -1004,13 +1004,13 @@ mod test {
};

let span = Span::test_data();
let b = EventType::try_from_record(&event, span).unwrap();
let b = EventType::try_from_record(&event, span).expect("event type");
assert!(matches!(b, EventType::Edit(_)));

let event = Value::test_record(event);
let config = Config::default();

let parsed_event = parse_event(&event, &config).unwrap();
let parsed_event = parse_event(&event, &config).expect("event");
assert_eq!(
parsed_event,
Some(ReedlineEvent::Edit(vec![EditCommand::Clear]))
Expand All @@ -1025,13 +1025,13 @@ mod test {
};

let span = Span::test_data();
let b = EventType::try_from_record(&event, span).unwrap();
let b = EventType::try_from_record(&event, span).expect("event type");
assert!(matches!(b, EventType::Send(_)));

let event = Value::test_record(event);
let config = Config::default();

let parsed_event = parse_event(&event, &config).unwrap();
let parsed_event = parse_event(&event, &config).expect("event");
assert_eq!(
parsed_event,
Some(ReedlineEvent::Menu("history_menu".to_string()))
Expand All @@ -1055,13 +1055,13 @@ mod test {
};

let span = Span::test_data();
let b = EventType::try_from_record(&event, span).unwrap();
let b = EventType::try_from_record(&event, span).expect("event type");
assert!(matches!(b, EventType::Until(_)));

let event = Value::test_record(event);
let config = Config::default();

let parsed_event = parse_event(&event, &config).unwrap();
let parsed_event = parse_event(&event, &config).expect("event");
assert_eq!(
parsed_event,
Some(ReedlineEvent::UntilFound(vec![
Expand All @@ -1083,7 +1083,7 @@ mod test {
let event = Value::list(vec![menu_event, enter_event], Span::test_data());

let config = Config::default();
let parsed_event = parse_event(&event, &config).unwrap();
let parsed_event = parse_event(&event, &config).expect("event");
assert_eq!(
parsed_event,
Some(ReedlineEvent::Multiple(vec![
Expand Down
9 changes: 7 additions & 2 deletions crates/nu-cli/src/repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,13 +851,18 @@ fn are_session_ids_in_sync() {
let history_path_o =
crate::config_files::get_history_path("nushell", engine_state.config.history_file_format);
assert!(history_path_o.is_some());
let history_path = history_path_o.as_deref().unwrap();
let history_path = history_path_o.as_deref().expect("history path");
let line_editor = reedline::Reedline::create();
let history_session_id = reedline::Reedline::create_history_session_id();
let line_editor =
update_line_editor_history(engine_state, history_path, line_editor, history_session_id);
assert_eq!(
i64::from(line_editor.unwrap().get_history_session_id().unwrap()),
i64::from(
line_editor
.expect("reedline")
.get_history_session_id()
.expect("session id")
),
engine_state.history_session_id
);
}
26 changes: 16 additions & 10 deletions crates/nu-cli/tests/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,20 @@ fn dotnu_completions() {
let suggestions = completer.complete(&completion_str, completion_str.len());

assert_eq!(1, suggestions.len());
assert_eq!("custom_completion.nu", suggestions.get(0).unwrap().value);
assert_eq!(
"custom_completion.nu",
suggestions.get(0).expect("suggestion").value
);

// Test use completion
let completion_str = "use ".to_string();
let suggestions = completer.complete(&completion_str, completion_str.len());

assert_eq!(1, suggestions.len());
assert_eq!("custom_completion.nu", suggestions.get(0).unwrap().value);
assert_eq!(
"custom_completion.nu",
suggestions.get(0).expect("suggestion").value
);
}

#[test]
Expand All @@ -141,9 +147,9 @@ fn external_completer_trailing_space() {

let suggestions = run_external_completion(block, &input);
assert_eq!(3, suggestions.len());
assert_eq!("gh", suggestions.get(0).unwrap().value);
assert_eq!("alias", suggestions.get(1).unwrap().value);
assert_eq!("", suggestions.get(2).unwrap().value);
assert_eq!("gh", suggestions.get(0).expect("suggestion").value);
assert_eq!("alias", suggestions.get(1).expect("suggestion").value);
assert_eq!("", suggestions.get(2).expect("suggestion").value);
}

#[test]
Expand All @@ -153,8 +159,8 @@ fn external_completer_no_trailing_space() {

let suggestions = run_external_completion(block, &input);
assert_eq!(2, suggestions.len());
assert_eq!("gh", suggestions.get(0).unwrap().value);
assert_eq!("alias", suggestions.get(1).unwrap().value);
assert_eq!("gh", suggestions.get(0).expect("suggestion").value);
assert_eq!("alias", suggestions.get(1).expect("suggestion").value);
}

#[test]
Expand All @@ -164,9 +170,9 @@ fn external_completer_pass_flags() {

let suggestions = run_external_completion(block, &input);
assert_eq!(3, suggestions.len());
assert_eq!("gh", suggestions.get(0).unwrap().value);
assert_eq!("api", suggestions.get(1).unwrap().value);
assert_eq!("--", suggestions.get(2).unwrap().value);
assert_eq!("gh", suggestions.get(0).expect("suggestion").value);
assert_eq!("api", suggestions.get(1).expect("suggestion").value);
assert_eq!("--", suggestions.get(2).expect("suggestion").value);
}

#[test]
Expand Down
13 changes: 8 additions & 5 deletions crates/nu-cmd-base/src/arg_glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ mod test {
for f in act.expect("checked ok") {
match f {
Ok(p) => {
assert!(!p.to_str().unwrap().is_empty());
assert!(!p.to_str().expect("path").is_empty());
}
Err(e) => panic!("unexpected error {:?}", e),
};
Expand Down Expand Up @@ -136,9 +136,12 @@ mod test {
);

// expected behavior -- that directories are included in results (if name matches pattern)
let t = p
.iter()
.any(|i| i.as_ref().unwrap().to_string_lossy().contains(".children"));
let t = p.iter().any(|i| {
i.as_ref()
.expect("path")
.to_string_lossy()
.contains(".children")
});
assert!(t, "check for dir, case {case}");
})
}
Expand Down Expand Up @@ -172,7 +175,7 @@ mod test {
" case {case}: matches input, but count not 1? "
);
assert_eq!(
&res[0].as_ref().unwrap().to_string_lossy(),
&res[0].as_ref().expect("path").to_string_lossy(),
pat, // todo: is it OK for glob to return relative paths (not to current cwd, but to arg cwd of arg_glob)?
);
} else {
Expand Down
8 changes: 4 additions & 4 deletions crates/nu-command/src/charting/hashable_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,14 @@ mod test {
(
Value::date(
DateTime::<FixedOffset>::parse_from_rfc2822("Wed, 18 Feb 2015 23:16:09 GMT")
.unwrap(),
.expect("date"),
span,
),
HashableValue::Date {
val: DateTime::<FixedOffset>::parse_from_rfc2822(
"Wed, 18 Feb 2015 23:16:09 GMT",
)
.unwrap(),
.expect("date"),
span,
},
),
Expand All @@ -229,7 +229,7 @@ mod test {
];
for (val, expect_hashable_val) in values.into_iter() {
assert_eq!(
HashableValue::from_value(val, Span::unknown()).unwrap(),
HashableValue::from_value(val, Span::unknown()).expect("value"),
expect_hashable_val
);
}
Expand Down Expand Up @@ -280,7 +280,7 @@ mod test {
let expected_val = val.clone();
assert_eq!(
HashableValue::from_value(val, Span::unknown())
.unwrap()
.expect("value")
.into_value(),
expected_val
);
Expand Down
12 changes: 8 additions & 4 deletions crates/nu-command/src/conversions/into/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,8 @@ mod tests {
};
let actual = action(&date_str, &args, Span::test_data());
let expected = Value::date(
DateTime::parse_from_str("16.11.1984 8:00 am +0000", "%d.%m.%Y %H:%M %P %z").unwrap(),
DateTime::parse_from_str("16.11.1984 8:00 am +0000", "%d.%m.%Y %H:%M %P %z")
.expect("valid date"),
Span::test_data(),
);
assert_eq!(actual, expected)
Expand All @@ -396,7 +397,8 @@ mod tests {
};
let actual = action(&date_str, &args, Span::test_data());
let expected = Value::date(
DateTime::parse_from_str("2020-08-04T16:39:18+00:00", "%Y-%m-%dT%H:%M:%S%z").unwrap(),
DateTime::parse_from_str("2020-08-04T16:39:18+00:00", "%Y-%m-%dT%H:%M:%S%z")
.expect("valid date"),
Span::test_data(),
);
assert_eq!(actual, expected)
Expand All @@ -416,7 +418,8 @@ mod tests {
};
let actual = action(&date_str, &args, Span::test_data());
let expected = Value::date(
DateTime::parse_from_str("2021-02-27 21:55:40 +08:00", "%Y-%m-%d %H:%M:%S %z").unwrap(),
DateTime::parse_from_str("2021-02-27 21:55:40 +08:00", "%Y-%m-%d %H:%M:%S %z")
.expect("valid date"),
Span::test_data(),
);

Expand All @@ -437,7 +440,8 @@ mod tests {
};
let actual = action(&date_int, &args, Span::test_data());
let expected = Value::date(
DateTime::parse_from_str("2021-02-27 21:55:40 +08:00", "%Y-%m-%d %H:%M:%S %z").unwrap(),
DateTime::parse_from_str("2021-02-27 21:55:40 +08:00", "%Y-%m-%d %H:%M:%S %z")
.expect("valid date"),
Span::test_data(),
);

Expand Down
20 changes: 10 additions & 10 deletions crates/nu-command/src/database/values/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,8 @@ mod test {

#[test]
fn can_read_empty_db() {
let db = open_connection_in_memory().unwrap();
let converted_db = read_entire_sqlite_db(db, Span::test_data(), None).unwrap();
let db = open_connection_in_memory().expect("db");
let converted_db = read_entire_sqlite_db(db, Span::test_data(), None).expect("record");

let expected = Value::test_record(Record::new());

Expand All @@ -471,7 +471,7 @@ mod test {

#[test]
fn can_read_empty_table() {
let db = open_connection_in_memory().unwrap();
let db = open_connection_in_memory().expect("db");

db.execute(
"CREATE TABLE person (
Expand All @@ -481,8 +481,8 @@ mod test {
)",
[],
)
.unwrap();
let converted_db = read_entire_sqlite_db(db, Span::test_data(), None).unwrap();
.expect("query");
let converted_db = read_entire_sqlite_db(db, Span::test_data(), None).expect("record");

let expected = Value::test_record(record! {
"person" => Value::test_list(vec![]),
Expand All @@ -494,7 +494,7 @@ mod test {
#[test]
fn can_read_null_and_non_null_data() {
let span = Span::test_data();
let db = open_connection_in_memory().unwrap();
let db = open_connection_in_memory().expect("db");

db.execute(
"CREATE TABLE item (
Expand All @@ -503,15 +503,15 @@ mod test {
)",
[],
)
.unwrap();
.expect("query");

db.execute("INSERT INTO item (id, name) VALUES (123, NULL)", [])
.unwrap();
.expect("query");

db.execute("INSERT INTO item (id, name) VALUES (456, 'foo bar')", [])
.unwrap();
.expect("query");

let converted_db = read_entire_sqlite_db(db, span, None).unwrap();
let converted_db = read_entire_sqlite_db(db, span, None).expect("record");

let expected = Value::test_record(record! {
"item" => Value::test_list(
Expand Down

0 comments on commit 5fecf7f

Please sign in to comment.