Skip to content

Commit 2b11536

Browse files
CI(rust): Tighten clippy rules except for tests where we relax (#112)
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
1 parent 3c92c5a commit 2b11536

16 files changed

Lines changed: 118 additions & 116 deletions

File tree

Cargo.toml

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,32 +27,32 @@ lto = true
2727

2828
[workspace.lints.rust]
2929
unsafe_code = "forbid"
30-
missing_docs = "warn"
31-
missing_debug_implementations = "warn"
32-
unreachable_pub = "warn"
33-
unused_qualifications = "warn"
34-
deprecated = "warn"
30+
missing_docs = "deny"
31+
missing_debug_implementations = "deny"
32+
unreachable_pub = "deny"
33+
unused_qualifications = "deny"
34+
deprecated = "deny"
3535
semicolon_in_expressions_from_macros = "deny"
36-
rust_2024_compatibility = { level = "warn", priority = -1 }
36+
rust_2024_compatibility = { level = "deny", priority = -1 }
3737

3838
[workspace.lints.clippy]
3939
# Pedantic group
40-
pedantic = { level = "warn", priority = -1 }
40+
pedantic = { level = "deny", priority = -1 }
4141
must_use_candidate = "allow"
4242
missing_errors_doc = "allow"
4343
missing_docs_in_private_items = "allow"
4444

4545
# Restriction group
46-
allow_attributes_without_reason = "warn"
47-
as_conversions = "warn"
46+
allow_attributes_without_reason = "deny"
47+
as_conversions = "deny"
4848
create_dir = "deny"
4949
dbg_macro = "deny"
5050
empty_drop = "deny"
5151
empty_enum_variants_with_brackets = "deny"
5252
empty_structs_with_brackets = "deny"
5353
error_impl_error = "deny"
5454
exit = "deny"
55-
expect_used = "warn"
55+
expect_used = "deny"
5656
field_scoped_visibility_modifiers = "deny"
5757
filetype_is_file = "deny"
5858
float_cmp_const = "deny"
@@ -63,9 +63,9 @@ if_then_some_else_none = "deny"
6363
impl_trait_in_params = "deny"
6464
indexing_slicing = "deny"
6565
infinite_loop = "deny"
66-
integer_division_remainder_used = "warn"
67-
integer_division = "warn"
68-
iter_over_hash_type = "warn"
66+
integer_division_remainder_used = "deny"
67+
integer_division = "deny"
68+
iter_over_hash_type = "deny"
6969
lossy_float_literal = "deny"
7070
map_err_ignore = "deny"
7171
map_with_unused_argument_over_ranges = "deny"
@@ -76,43 +76,44 @@ module_name_repetitions = "deny"
7676
multiple_inherent_impl = "deny"
7777
multiple_unsafe_ops_per_block = "deny"
7878
needless_raw_strings = "deny"
79-
panic = "warn"
79+
panic = "deny"
8080
panic_in_result_fn = "deny"
8181
partial_pub_fields = "deny"
8282
pathbuf_init_then_push = "deny"
8383
print_stderr = "deny"
84-
print_stdout = "warn"
84+
print_stdout = "deny"
8585
pub_without_shorthand = "deny"
86-
redundant_type_annotations = "warn"
87-
ref_patterns = "allow" # TODO: discuss
86+
redundant_type_annotations = "deny"
87+
ref_patterns = "deny"
8888
renamed_function_params = "deny"
8989
rest_pat_in_fully_bound_structs = "deny"
9090
same_name_method = "deny"
9191
self_named_module_files = "deny"
92-
semicolon_inside_block = "allow" # TODO: discuss
93-
semicolon_outside_block = "allow" # TODO: discuss
94-
separated_literal_suffix = "allow" # TODO: discuss
95-
shadow_reuse = "warn"
92+
semicolon_inside_block = "deny"
93+
semicolon_outside_block = "allow"
94+
separated_literal_suffix = "allow"
95+
shadow_same = "allow"
96+
shadow_reuse = "allow"
9697
shadow_unrelated = "deny"
9798
str_to_string = "deny"
9899
string_add = "allow" # TODO: discuss
99-
string_lit_chars_any = "warn"
100-
string_slice = "warn"
101-
suspicious_xor_used_as_pow = "warn"
100+
string_lit_chars_any = "deny"
101+
string_slice = "deny"
102+
suspicious_xor_used_as_pow = "deny"
102103
tests_outside_test_module = "deny"
103104
todo = "deny"
104105
try_err = "deny"
105106
undocumented_unsafe_blocks = "deny"
106-
unimplemented = "warn"
107+
unimplemented = "deny"
107108
unnecessary_safety_comment = "deny"
108109
unnecessary_safety_doc = "deny"
109110
unnecessary_self_imports = "deny"
110-
unneeded_field_pattern = "warn"
111+
unneeded_field_pattern = "deny"
111112
unreachable = "deny"
112-
unseparated_literal_suffix = "allow" # TODO: discuss
113+
unseparated_literal_suffix = "deny"
113114
unused_result_ok = "deny"
114115
unused_trait_names = "deny"
115-
unwrap_in_result = "warn"
116-
unwrap_used = "allow" # TODO: discuss
117-
use_debug = "warn"
116+
unwrap_in_result = "deny"
117+
unwrap_used = "deny"
118+
use_debug = "deny"
118119
verbose_file_reads = "deny"

clippy.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
allow-dbg-in-tests = true
2+
allow-expect-in-tests = true
3+
allow-indexing-slicing-in-tests = true
4+
allow-panic-in-tests = true
5+
allow-print-in-tests = true
6+
allow-unwrap-in-tests = true

rust/yaml-parser/benches/parser_bench.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,21 @@ fn bench_saphyr_marked_parse(bench: &mut criterion::Bencher<'_>, input: &str) {
9696
});
9797
}
9898

99+
#[expect(
100+
clippy::unwrap_used,
101+
reason = "benchmark corpora must remain valid serde_yaml input; panic preserves that precondition"
102+
)]
99103
fn bench_serde_yaml_value_parse(bench: &mut criterion::Bencher<'_>, input: &str) {
100104
bench.iter(|| {
101105
let value: serde_yaml::Value = serde_yaml::from_str(black_box(input)).unwrap();
102106
black_box(value);
103107
});
104108
}
105109

110+
fn input_throughput(input: &str) -> Throughput {
111+
Throughput::Bytes(u64::try_from(input.len()).unwrap_or(u64::MAX))
112+
}
113+
106114
/// Benchmark parsing throughput for different document types.
107115
fn bench_parse_throughput(criterion: &mut Criterion) {
108116
let test_cases: &[(&str, &str)] = &[
@@ -118,7 +126,7 @@ fn bench_parse_throughput(criterion: &mut Criterion) {
118126
let mut group = criterion.benchmark_group("parse_throughput");
119127

120128
for (name, input) in test_cases {
121-
group.throughput(Throughput::Bytes(u64::try_from(input.len()).unwrap()));
129+
group.throughput(input_throughput(input));
122130

123131
// Benchmark yaml-parser (always includes spans)
124132
group.bench_with_input(
@@ -226,6 +234,10 @@ key3: "tab\there"
226234
/// Benchmark serde-based deserialization throughput for different document
227235
/// types when the `serde` feature is enabled.
228236
#[cfg(feature = "serde")]
237+
#[expect(
238+
clippy::unwrap_used,
239+
reason = "benchmark corpora must deserialize successfully; panic preserves that precondition"
240+
)]
229241
fn bench_serde_deserialize_throughput(criterion: &mut Criterion) {
230242
let test_cases: &[(&str, &str)] = &[
231243
("large_mapping", LARGE_MAPPING),
@@ -243,7 +255,7 @@ fn bench_serde_deserialize_throughput(criterion: &mut Criterion) {
243255
// We expect both libraries to successfully deserialize all benchmark
244256
// corpora. If either panics here, that's a behavioural regression we
245257
// want to see rather than silently skipping the dataset.
246-
group.throughput(Throughput::Bytes(u64::try_from(input.len()).unwrap()));
258+
group.throughput(input_throughput(input));
247259

248260
// Benchmark yaml-parser's serde-based deserialization into our own
249261
// generic `OwnedYamlValue` tree. This uses the crate's only serde

rust/yaml-parser/examples/profile_cases.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,15 @@ fn main() -> ExitCode {
116116
#[cfg(feature = "serde")]
117117
Mode::Serde => {
118118
for _ in 0..iterations {
119-
let value: OwnedYamlValue = yaml_parser::serde::from_str(black_box(input)).unwrap();
120-
black_box(value);
119+
match yaml_parser::serde::from_str::<OwnedYamlValue>(black_box(input)) {
120+
Ok(value) => {
121+
black_box(value);
122+
}
123+
Err(error) => {
124+
eprintln!("serde deserialization failed for {dataset_arg}: {error}");
125+
return ExitCode::FAILURE;
126+
}
127+
}
121128
}
122129
}
123130
}

rust/yaml-parser/src/emitter/document.rs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -129,20 +129,14 @@ impl Emitter<'_> {
129129
self.tag_handles.insert("!!", "tag:yaml.org,2002:");
130130

131131
let mut idx = self.pos;
132-
loop {
133-
let Some(continue_scan) = self.cursor.peek_with(idx, |token, _| match token {
134-
Token::TagDirective(handle, prefix) => {
135-
self.tag_handles.insert(handle, prefix);
136-
true
137-
}
138-
Token::DocStart | Token::DocEnd => false,
139-
_ => true,
140-
}) else {
141-
break;
142-
};
143-
if !continue_scan {
144-
break;
132+
while let Some(true) = self.cursor.peek_with(idx, |token, _| match token {
133+
Token::TagDirective(handle, prefix) => {
134+
self.tag_handles.insert(handle, prefix);
135+
true
145136
}
137+
Token::DocStart | Token::DocEnd => false,
138+
_ => true,
139+
}) {
146140
idx += 1;
147141
}
148142
}

rust/yaml-parser/src/emitter/trivia.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ impl Emitter<'_> {
5353
loop {
5454
match self.peek_kind() {
5555
Some(TokenKind::LineStart) => {
56-
let (token, span) = self.take_current().unwrap();
57-
let Token::LineStart(indent) = token else {
56+
let Some((Token::LineStart(indent), span)) = self.take_current() else {
5857
debug_assert!(false, "expected LineStart token");
5958
break;
6059
};

rust/yaml-parser/src/lexer/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@ mod trivia;
3030

3131
#[cfg(test)]
3232
#[allow(
33-
clippy::indexing_slicing,
3433
clippy::min_ident_chars,
3534
clippy::shadow_reuse,
36-
reason = "Tests benefit from direct indexing, short identifiers, and variable shadowing for readability"
35+
reason = "Tests benefit from short identifiers and variable shadowing for readability"
3736
)]
3837
mod tests;
3938

rust/yaml-parser/src/lexer/properties.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl<'input> Lexer<'input> {
9191
// Colon ends plain scalar if followed by whitespace (or EOF)
9292
if peek_ch == ':' {
9393
let next = self.peek_n(1);
94-
if next.is_none() || next.is_some_and(char::is_whitespace) {
94+
if next.is_none_or(char::is_whitespace) {
9595
break;
9696
}
9797
}

rust/yaml-parser/src/lexer/quoted.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ impl<'input> Lexer<'input> {
4242
fn finish_quoted_newline(&mut self) -> usize {
4343
// Consume newline
4444
let newline_start = self.byte_pos;
45-
let ch = self.advance().unwrap();
45+
let Some(ch) = self.advance() else {
46+
debug_assert!(false, "finish_quoted_newline called at EOF");
47+
return self.byte_pos;
48+
};
4649
if ch == '\r' && self.peek() == Some('\n') {
4750
self.advance();
4851
}
@@ -220,7 +223,7 @@ impl<'input> Lexer<'input> {
220223
// Track the content length after the last non-escape character.
221224
// When we see a newline, we trim trailing whitespace but only up to this position.
222225
// This preserves escaped whitespace like \t while trimming literal trailing whitespace.
223-
let mut escape_protected_len = 0usize;
226+
let mut escape_protected_len = 0_usize;
224227

225228
loop {
226229
match self.peek() {
@@ -310,6 +313,15 @@ impl<'input> Lexer<'input> {
310313
Some(result)
311314
}
312315

316+
fn ascii_hex_digit_value(ch: char) -> Option<u32> {
317+
match ch {
318+
'0'..='9' => Some(u32::from(ch) - u32::from('0')),
319+
'a'..='f' => Some(u32::from(ch) - u32::from('a') + 10),
320+
'A'..='F' => Some(u32::from(ch) - u32::from('A') + 10),
321+
_ => None,
322+
}
323+
}
324+
313325
#[allow(
314326
clippy::string_slice,
315327
reason = "self.byte_pos is always at proper UTF-8 boundaries"
@@ -320,12 +332,11 @@ impl<'input> Lexer<'input> {
320332
'u' => 4,
321333
_ => 8, // 'U' case
322334
};
323-
let mut value = 0u32;
324-
let mut consumed = 0u8;
335+
let mut value = 0_u32;
336+
let mut consumed = 0_u8;
325337
for _ in 0..digits {
326338
if let Some(peek_ch) = self.peek() {
327-
if peek_ch.is_ascii_hexdigit() {
328-
let digit = peek_ch.to_digit(16).unwrap();
339+
if let Some(digit) = Self::ascii_hex_digit_value(peek_ch) {
329340
value = value * 16 + digit;
330341
consumed += 1;
331342
self.advance();

rust/yaml-parser/src/parser/tests.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
#![allow(clippy::indexing_slicing, reason = "panics are acceptable in tests")]
2-
#![allow(clippy::panic, reason = "panic is acceptable in tests")]
3-
#![allow(
41
// Copyright (c) 2026 Arista Networks, Inc.
52
// Use of this source code is governed by the Apache License 2.0
63
// that can be found in the LICENSE file.
74

5+
#![allow(
86
clippy::min_ident_chars,
97
reason = "single-char closure params are fine in tests"
108
)]
@@ -300,7 +298,6 @@ fn parse_via_events(input: &str) -> Vec<Node<'static>> {
300298

301299
/// Test that parsing through collected events matches the public AST path.
302300
#[test]
303-
#[allow(clippy::print_stderr, reason = "Debug output on failure")]
304301
fn test_event_parser_matches_public_parse_pipeline() {
305302
let test_cases = [
306303
"hello",
@@ -522,7 +519,7 @@ fn test_invalid_explicit_builtin_tags_report_error_and_recover_to_string() {
522519
);
523520
let expected_text = input.split_once(' ').map_or("", |(_, text)| text);
524521
assert!(
525-
matches!(docs[0].value, Value::String(ref text) if text.as_ref() == expected_text),
522+
matches!(&docs[0].value, Value::String(text) if text == expected_text),
526523
"expected string recovery for {input:?}, got {:?}",
527524
docs[0].value
528525
);
@@ -557,7 +554,7 @@ fn test_non_specific_and_custom_tags_disable_implicit_resolution() {
557554
non_specific_errors.is_empty(),
558555
"unexpected errors: {non_specific_errors:?}"
559556
);
560-
assert!(matches!(non_specific_docs[0].value, Value::String(ref text) if text == "42"));
557+
assert!(matches!(&non_specific_docs[0].value, Value::String(text) if text == "42"));
561558
assert_eq!(
562559
non_specific_docs[0].tag().map(|tag| tag.value.as_ref()),
563560
Some("!")
@@ -568,7 +565,7 @@ fn test_non_specific_and_custom_tags_disable_implicit_resolution() {
568565
custom_errors.is_empty(),
569566
"unexpected errors: {custom_errors:?}"
570567
);
571-
assert!(matches!(custom_docs[0].value, Value::String(ref text) if text == "42"));
568+
assert!(matches!(&custom_docs[0].value, Value::String(text) if text == "42"));
572569
assert_eq!(
573570
custom_docs[0].tag().map(|tag| tag.value.as_ref()),
574571
Some("!custom")
@@ -579,7 +576,7 @@ fn test_non_specific_and_custom_tags_disable_implicit_resolution() {
579576
explicit_str_errors.is_empty(),
580577
"unexpected errors: {explicit_str_errors:?}"
581578
);
582-
assert!(matches!(explicit_str_docs[0].value, Value::String(ref text) if text == "42"));
579+
assert!(matches!(&explicit_str_docs[0].value, Value::String(text) if text == "42"));
583580

584581
let (explicit_int_docs, explicit_int_errors) = parse("!<tag:yaml.org,2002:int> 0o52");
585582
assert!(

0 commit comments

Comments
 (0)