Skip to content

Commit 839e477

Browse files
style(clippy): idiom sweep across lib/common, mylang, and checker (refs #22) (#25)
Mechanical idiom upgrades flagged by clippy. No behaviour change in any path; all touched modules continue to test green. checker.rs (6 sites): `if let Err(_) = X { .. }` -> `if X.is_err() { .. }` for `define_effect`, `define_ai_model`, `define_prompt`, function symbol define, parameter symbol define, and variable symbol define. checker.rs:773: drop the unused `.enumerate()` whose index was already discarded via `_i`; iterate the zipped pair directly. lib/common/string.rs: * `radix < 2 || radix > 36` -> `!(2..=36).contains(&radix)` * two `std::iter::repeat(pad).take(N)` sites -> `std::iter::repeat_n(pad, N)` (stable since Rust 1.82). lib/common/types.rs: * `radix < 2 || radix > 36` -> `!(2..=36).contains(&radix)` * `assert_eq!(int_to_bool(0), false)` / `assert_eq!(.., true)` -> `assert!(!..)` / `assert!(..)`. lib/common/utils.rs: * `self.next_u64() % 2 == 0` -> `self.next_u64().is_multiple_of(2)` * two `r >= 0.0 && r < 1.0` -> `(0.0..1.0).contains(&r)` * `r >= 5 && r <= 10` -> `(5..=10).contains(&r)`. lib/common/array.rs:313: `slice(&vec![1,2,3,4,5], ..)` -> `slice(&[1,2,3,4,5], ..)` (clippy::useless_vec). lib/mylang/ai.rs:26 and lib/mylang/prompt.rs:139: keep `AiModelType:: from_str` and `PromptBuilder::add` as they are, but annotate each with `#[allow(clippy::should_implement_trait)]` plus a one-line rationale. Implementing `FromStr` would force callers to `.unwrap()` an `Infallible` (the classifier never fails). Implementing `std::ops::Add` would be wrong shape for a fluent builder (`+` ate two operands and consumes them; `add` here chains). Out of scope for this PR (kept clean): * parser.rs `errors.len() > 0` x6 -- in test code that overlaps with the depth-guard PR (#21); will land cleanly once #21 merges. * `approx_constant` errors (PR1, #23) and the two real correctness bugs (PR2, #24) -- handled in their own branches. Part 3 of 4 from #22. Off clean origin/main; does not depend on PR1 (#23) or PR2 (#24).
1 parent 888c779 commit 839e477

7 files changed

Lines changed: 32 additions & 22 deletions

File tree

crates/my-lang/lib/common/array.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ mod tests {
310310
let a = vec![1, 2];
311311
let b = vec![3, 4];
312312
assert_eq!(concat(&a, &b), vec![1, 2, 3, 4]);
313-
assert_eq!(slice(&vec![1, 2, 3, 4, 5], 1, 4), vec![2, 3, 4]);
313+
assert_eq!(slice(&[1, 2, 3, 4, 5], 1, 4), vec![2, 3, 4]);
314314
}
315315

316316
#[test]

crates/my-lang/lib/common/string.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ pub fn pad_start(s: &str, len: usize, pad: char) -> String {
135135
if s.len() >= len {
136136
s.to_string()
137137
} else {
138-
let padding: String = std::iter::repeat(pad).take(len - s.len()).collect();
138+
let padding: String = std::iter::repeat_n(pad, len - s.len()).collect();
139139
format!("{}{}", padding, s)
140140
}
141141
}
@@ -145,7 +145,7 @@ pub fn pad_end(s: &str, len: usize, pad: char) -> String {
145145
if s.len() >= len {
146146
s.to_string()
147147
} else {
148-
let padding: String = std::iter::repeat(pad).take(len - s.len()).collect();
148+
let padding: String = std::iter::repeat_n(pad, len - s.len()).collect();
149149
format!("{}{}", s, padding)
150150
}
151151
}
@@ -197,7 +197,7 @@ pub fn parse_float(s: &str) -> Option<f64> {
197197

198198
/// Format integer to string with radix
199199
pub fn int_to_string_radix(n: i64, radix: u32) -> Option<String> {
200-
if radix < 2 || radix > 36 {
200+
if !(2..=36).contains(&radix) {
201201
return None;
202202
}
203203

crates/my-lang/lib/common/types.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub fn int_to_str(n: i64) -> String {
9191

9292
/// Convert integer to string with radix
9393
pub fn int_to_str_radix(n: i64, radix: u32) -> Option<String> {
94-
if radix < 2 || radix > 36 {
94+
if !(2..=36).contains(&radix) {
9595
return None;
9696
}
9797

@@ -331,8 +331,8 @@ mod tests {
331331
assert_eq!(str_to_bool("false"), Some(false));
332332
assert_eq!(str_to_bool("yes"), Some(true));
333333
assert_eq!(str_to_bool("no"), Some(false));
334-
assert_eq!(int_to_bool(0), false);
335-
assert_eq!(int_to_bool(1), true);
334+
assert!(!int_to_bool(0));
335+
assert!(int_to_bool(1));
336336
}
337337

338338
#[test]

crates/my-lang/lib/common/utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl SimpleRng {
9595

9696
/// Generate random bool
9797
pub fn next_bool(&mut self) -> bool {
98-
self.next_u64() % 2 == 0
98+
self.next_u64().is_multiple_of(2)
9999
}
100100

101101
/// Generate random bool with probability p of being true
@@ -294,15 +294,15 @@ mod tests {
294294
let r1 = random();
295295
let r2 = random();
296296
// Random numbers should be in [0, 1)
297-
assert!(r1 >= 0.0 && r1 < 1.0);
298-
assert!(r2 >= 0.0 && r2 < 1.0);
297+
assert!((0.0..1.0).contains(&r1));
298+
assert!((0.0..1.0).contains(&r2));
299299
}
300300

301301
#[test]
302302
fn test_random_int() {
303303
for _ in 0..100 {
304304
let r = random_int(5, 10);
305-
assert!(r >= 5 && r <= 10);
305+
assert!((5..=10).contains(&r));
306306
}
307307
}
308308

crates/my-lang/lib/mylang/ai.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ pub enum AiModelType {
2323
}
2424

2525
impl AiModelType {
26+
// Deliberately not implementing `std::str::FromStr`: this classifier is
27+
// infallible (every string maps to *some* variant -- unknown names fall
28+
// through to `Local`), so the trait's `Result<Self, Self::Err>` shape
29+
// would just force callers to `.unwrap()` an `Infallible` everywhere.
30+
#[allow(clippy::should_implement_trait)]
2631
pub fn from_str(s: &str) -> Self {
2732
if s.starts_with("gpt-") || s.starts_with("o1") {
2833
AiModelType::OpenAI(s.to_string())

crates/my-lang/lib/mylang/prompt.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,12 @@ impl PromptBuilder {
135135
}
136136
}
137137

138-
/// Add a part to the prompt
138+
/// Add a part to the prompt.
139+
///
140+
/// This is a fluent-builder method, not `std::ops::Add::add` (which would
141+
/// be the wrong shape -- `Add` is for the `+` operator and consumes two
142+
/// values, while this returns `Self` for chaining).
143+
#[allow(clippy::should_implement_trait)]
139144
pub fn add(mut self, text: &str) -> Self {
140145
self.parts.push(text.to_string());
141146
self

crates/my-lang/src/checker.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ impl Checker {
416416
span: e.span,
417417
};
418418

419-
if let Err(_) = self.types.define_effect(def) {
419+
if self.types.define_effect(def).is_err() {
420420
self.errors.push(CheckError::DuplicateDefinition {
421421
name: e.name.name.clone(),
422422
line: e.span.line,
@@ -445,7 +445,7 @@ impl Checker {
445445
span: m.span,
446446
};
447447

448-
if let Err(_) = self.types.define_ai_model(def) {
448+
if self.types.define_ai_model(def).is_err() {
449449
self.errors.push(CheckError::DuplicateDefinition {
450450
name: m.name.name.clone(),
451451
line: m.span.line,
@@ -469,7 +469,7 @@ impl Checker {
469469
span: p.span,
470470
};
471471

472-
if let Err(_) = self.types.define_prompt(def) {
472+
if self.types.define_prompt(def).is_err() {
473473
self.errors.push(CheckError::DuplicateDefinition {
474474
name: p.name.name.clone(),
475475
line: p.span.line,
@@ -506,13 +506,13 @@ impl Checker {
506506
result: Box::new(return_type),
507507
};
508508

509-
if let Err(_) = self.symbols.define(Symbol {
509+
if self.symbols.define(Symbol {
510510
name: f.name.name.clone(),
511511
kind: SymbolKind::Function,
512512
ty: fn_type,
513513
span: f.span,
514514
mutable: false,
515-
}) {
515+
}).is_err() {
516516
self.errors.push(CheckError::DuplicateDefinition {
517517
name: f.name.name.clone(),
518518
line: f.span.line,
@@ -541,13 +541,13 @@ impl Checker {
541541
// Add parameters to scope
542542
for param in &f.params {
543543
let ty = ast_type_to_ty(&param.ty);
544-
if let Err(_) = self.symbols.define(Symbol {
544+
if self.symbols.define(Symbol {
545545
name: param.name.name.clone(),
546546
kind: SymbolKind::Parameter,
547547
ty,
548548
span: param.span,
549549
mutable: false,
550-
}) {
550+
}).is_err() {
551551
self.errors.push(CheckError::DuplicateDefinition {
552552
name: param.name.name.clone(),
553553
line: param.span.line,
@@ -610,13 +610,13 @@ impl Checker {
610610
value_ty
611611
};
612612

613-
if let Err(_) = self.symbols.define(Symbol {
613+
if self.symbols.define(Symbol {
614614
name: name.name.clone(),
615615
kind: SymbolKind::Variable,
616616
ty: final_ty,
617617
span: *span,
618618
mutable: *mutable,
619-
}) {
619+
}).is_err() {
620620
self.errors.push(CheckError::DuplicateDefinition {
621621
name: name.name.clone(),
622622
line: span.line,
@@ -770,7 +770,7 @@ impl Checker {
770770
column: span.column,
771771
});
772772
} else {
773-
for (_i, (param, arg)) in params.iter().zip(arg_types.iter()).enumerate() {
773+
for (param, arg) in params.iter().zip(arg_types.iter()) {
774774
if !param.is_assignable_from(arg) && !arg.is_error_or_unknown() {
775775
self.errors.push(CheckError::TypeMismatch {
776776
expected: param.to_string(),

0 commit comments

Comments
 (0)