From f6467db21be5e21903a4ca2d8a018cfa2803d4fe Mon Sep 17 00:00:00 2001 From: Mathias Peters Date: Mon, 14 Oct 2019 11:04:56 +0200 Subject: [PATCH] Fix clippy warnings (#153) * Fixed clippy warnings option_unwrap_used and result_unwrap_used. * Fixed float_cmp clippy error and the option_unwrap_used it was hiding. * Fixed shadowing issues. * Disabled some clippy lints in tests. * Fixed clippy string_add issues. * Added allow else_if_without_else since an else-block would not make sense. * Fixed integer arithmetic issues. * Fixed all clippy get_unwrap issues. * Fixed usage of unimplemented and an occurrence of variable name shadowing. * Fixed indexing may panic issues. * Fixed a slicing may panic issue. * Changed some unnecessary uses of match that I introduced for expect. --- .../declarative_environment_record.rs | 32 +++++---- .../function_environment_record.rs | 32 +++++---- .../environment/global_environment_record.rs | 3 +- src/lib/environment/lexical_environment.rs | 18 +++-- src/lib/exec.rs | 24 +++---- src/lib/js/array.rs | 49 +++++++------- src/lib/js/boolean.rs | 1 + src/lib/js/console.rs | 14 ++-- src/lib/js/function.rs | 6 +- src/lib/js/math.rs | 66 +++++++++---------- src/lib/js/regexp.rs | 6 +- src/lib/js/string.rs | 37 ++++++----- src/lib/js/value.rs | 14 ++-- src/lib/syntax/lexer.rs | 39 +++++------ src/lib/syntax/parser.rs | 19 +++--- 15 files changed, 190 insertions(+), 170 deletions(-) diff --git a/src/lib/environment/declarative_environment_record.rs b/src/lib/environment/declarative_environment_record.rs index 5ae0a8bb45..70cdce4eca 100644 --- a/src/lib/environment/declarative_environment_record.rs +++ b/src/lib/environment/declarative_environment_record.rs @@ -90,6 +90,7 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } } + #[allow(clippy::else_if_without_else)] fn set_mutable_binding(&mut self, name: &str, value: Value, mut strict: bool) { if self.env_rec.get(name).is_none() { if strict { @@ -120,25 +121,30 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } fn get_binding_value(&self, name: &str, _strict: bool) -> Value { - if self.env_rec.get(name).is_some() && self.env_rec.get(name).unwrap().value.is_some() { - let record: &DeclarativeEnvironmentRecordBinding = self.env_rec.get(name).unwrap(); - record.value.as_ref().unwrap().clone() - } else { - // TODO: change this when error handling comes into play - panic!("ReferenceError: Cannot get binding value for {}", name); + match self.env_rec.get(name) { + Some(binding) => { + binding.value.as_ref().expect("Could not get record as reference").clone() + }, + None => { + // TODO: change this when error handling comes into play + panic!("ReferenceError: Cannot get binding value for {}", name); + } } } fn delete_binding(&mut self, name: &str) -> bool { - if self.env_rec.get(name).is_some() { - if self.env_rec.get(name).unwrap().can_delete { - self.env_rec.remove(name); - true - } else { + match self.env_rec.get(name) { + Some(binding) => { + if binding.can_delete { + self.env_rec.remove(name); + true + } else { + false + } + }, + None => { false } - } else { - false } } diff --git a/src/lib/environment/function_environment_record.rs b/src/lib/environment/function_environment_record.rs index 072b9b31a4..5e4498dd05 100644 --- a/src/lib/environment/function_environment_record.rs +++ b/src/lib/environment/function_environment_record.rs @@ -147,6 +147,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } } + #[allow(clippy::else_if_without_else)] fn set_mutable_binding(&mut self, name: &str, value: Value, mut strict: bool) { if self.env_rec.get(name).is_none() { if strict { @@ -178,25 +179,30 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } fn get_binding_value(&self, name: &str, _strict: bool) -> Value { - if self.env_rec.get(name).is_some() && self.env_rec.get(name).unwrap().value.is_some() { - let record: &DeclarativeEnvironmentRecordBinding = self.env_rec.get(name).unwrap(); - record.value.as_ref().unwrap().clone() - } else { - // TODO: change this when error handling comes into play - panic!("ReferenceError: Cannot get binding value for {}", name); + match self.env_rec.get(name) { + Some(binding) => { + binding.value.as_ref().expect("Could not get record as reference").clone() + }, + None => { + // TODO: change this when error handling comes into play + panic!("ReferenceError: Cannot get binding value for {}", name); + } } } fn delete_binding(&mut self, name: &str) -> bool { - if self.env_rec.get(name).is_some() { - if self.env_rec.get(name).unwrap().can_delete { - self.env_rec.remove(name); - true - } else { + match self.env_rec.get(name) { + Some(binding) => { + if binding.can_delete { + self.env_rec.remove(name); + true + } else { + false + } + }, + None => { false } - } else { - false } } diff --git a/src/lib/environment/global_environment_record.rs b/src/lib/environment/global_environment_record.rs index 8c3aa8a82a..e018d467ad 100644 --- a/src/lib/environment/global_environment_record.rs +++ b/src/lib/environment/global_environment_record.rs @@ -179,7 +179,8 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn set_outer_environment(&mut self, _env: Environment) { - unimplemented!(); + // TODO: Implement + panic!("Not implemented yet") } fn get_environment_type(&self) -> EnvironmentType { diff --git a/src/lib/environment/lexical_environment.rs b/src/lib/environment/lexical_environment.rs index 4069157617..82dae14191 100644 --- a/src/lib/environment/lexical_environment.rs +++ b/src/lib/environment/lexical_environment.rs @@ -91,8 +91,8 @@ impl LexicalEnvironment { } pub fn get_global_object(&self) -> Option { - let global = &self.environment_stack[0]; - global.borrow().get_global_object() + self.environment_stack.get(0).expect("").borrow().get_global_object() + } pub fn create_mutable_binding(&mut self, name: String, deletion: bool) { @@ -120,16 +120,14 @@ impl LexicalEnvironment { /// get_current_environment_ref is used when you only need to borrow the environment /// (you only need to add a new variable binding, or you want to fetch a value) pub fn get_current_environment_ref(&self) -> &Environment { - &self - .environment_stack - .get(self.environment_stack.len() - 1) - .unwrap() + let index = self.environment_stack.len().wrapping_sub(1); + &self.environment_stack.get(index).expect("Could not get current environment") } /// When neededing to clone an environment (linking it with another environnment) /// cloning is more suited. The GC will remove the env once nothing is linking to it anymore pub fn get_current_environment(&mut self) -> &mut Environment { - self.environment_stack.back_mut().unwrap() + self.environment_stack.back_mut().expect("Could not get mutable reference to back object") } pub fn get_binding_value(&mut self, name: &str) -> Value { @@ -144,10 +142,10 @@ impl LexicalEnvironment { if borrowed_env.get_outer_environment().is_some() { let mut outer: Option = borrowed_env.get_outer_environment(); while outer.is_some() { - if outer.as_ref().unwrap().borrow().has_binding(&name) { - return outer.unwrap().borrow().get_binding_value(name, false); + if outer.as_ref().expect("Could not get outer as reference").borrow().has_binding(&name) { + return outer.expect("Outer was None").borrow().get_binding_value(name, false); } - outer = outer.unwrap().borrow().get_outer_environment(); + outer = outer.expect("Outer was None").borrow().get_outer_environment(); } } diff --git a/src/lib/exec.rs b/src/lib/exec.rs index fd1382cc38..436aa82e3b 100644 --- a/src/lib/exec.rs +++ b/src/lib/exec.rs @@ -154,7 +154,7 @@ impl Executor for Interpreter { let block = &tup.1; if val == self.run(cond)? { matched = true; - let last_expr = block.last().unwrap(); + let last_expr = block.last().expect("Block has no expressions"); for expr in block.iter() { let e_result = self.run(expr)?; if expr == last_expr { @@ -164,12 +164,12 @@ impl Executor for Interpreter { } } if !matched && default.is_some() { - result = self.run(default.as_ref().unwrap())?; + result = self.run(default.as_ref().expect("Could not get default as reference"))?; } Ok(result) } ExprDef::ObjectDecl(ref map) => { - let global_val = &self.realm.environment.get_global_object().unwrap(); + let global_val = &self.realm.environment.get_global_object().expect("Could not get the global object"); let obj = ValueData::new_obj(Some(global_val)); for (key, val) in map.iter() { obj.borrow().set_field(key.clone(), self.run(val)?); @@ -177,7 +177,7 @@ impl Executor for Interpreter { Ok(obj) } ExprDef::ArrayDecl(ref arr) => { - let global_val = &self.realm.environment.get_global_object().unwrap(); + let global_val = &self.realm.environment.get_global_object().expect("Could not get the global object"); let arr_map = ValueData::new_obj(Some(global_val)); // Note that this object is an Array arr_map.set_kind(ObjectKind::Array); @@ -205,10 +205,10 @@ impl Executor for Interpreter { if name.is_some() { self.realm .environment - .create_mutable_binding(name.clone().unwrap(), false); + .create_mutable_binding(name.clone().expect("No name was supplied"), false); self.realm .environment - .initialize_binding(name.as_ref().unwrap(), val.clone()) + .initialize_binding(name.as_ref().expect("Could not get name as reference"), val.clone()) } Ok(val) } @@ -277,8 +277,8 @@ impl Executor for Interpreter { })) } ExprDef::BinOp(BinOp::Log(ref op), ref a, ref b) => { - let v_a = from_value::(self.run(a)?).unwrap(); - let v_b = from_value::(self.run(b)?).unwrap(); + let v_a = from_value::(self.run(a)?).expect("Could not convert JS value to bool"); + let v_b = from_value::(self.run(b)?).expect("Could not convert JS value to bool"); Ok(match *op { LogOp::And => to_value(v_a && v_b), LogOp::Or => to_value(v_a || v_b), @@ -338,8 +338,8 @@ impl Executor for Interpreter { )); for i in 0..data.args.len() { - let name = data.args.get(i).unwrap(); - let expr = v_args.get(i).unwrap(); + let name = data.args.get(i).expect("Could not get data argument"); + let expr = v_args.get(i).expect("Could not get argument"); env.create_mutable_binding(name.clone(), false); env.initialize_binding(name, expr.to_owned()); } @@ -468,8 +468,8 @@ impl Interpreter { Some(env.get_current_environment_ref().clone()), )); for i in 0..data.args.len() { - let name = data.args.get(i).unwrap(); - let expr: &Value = arguments_list.get(i).unwrap(); + let name = data.args.get(i).expect("Could not get data argument"); + let expr: &Value = arguments_list.get(i).expect("Could not get argument"); self.realm .environment .create_mutable_binding(name.clone(), false); diff --git a/src/lib/js/array.rs b/src/lib/js/array.rs index b4db761580..d6d891d9c1 100644 --- a/src/lib/js/array.rs +++ b/src/lib/js/array.rs @@ -35,11 +35,11 @@ fn add_to_array_object(array_ptr: &Value, add_values: &[Value]) -> ResultValue { from_value(array_ptr.get_field_slice("length")).expect("failed to conveert lenth to i32"); for (n, value) in add_values.iter().enumerate() { - let new_index = orig_length + (n as i32); + let new_index = orig_length.wrapping_add(n as i32); array_ptr.set_field(new_index.to_string(), value.clone()); } - array_ptr.set_field_slice("length", to_value(orig_length + add_values.len() as i32)); + array_ptr.set_field_slice("length", to_value(orig_length.wrapping_add(add_values.len() as i32))); Ok(array_ptr.clone()) } @@ -55,8 +55,8 @@ pub fn make_array(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultVa match args.len() { 0 => construct_array(this, &[]), 1 => { - let array = construct_array(this, &[]).unwrap(); - let size: i32 = from_value(args[0].clone()).unwrap(); + let array = construct_array(this, &[]).expect("Could not construct array"); + let size: i32 = from_value(args.get(0).expect("Could not get argument").clone()).expect("Could not convert argument to i32"); array.set_field_slice("length", to_value(size)); Ok(array) } @@ -87,13 +87,13 @@ pub fn concat(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue // one) let mut new_values: Vec = Vec::new(); - let this_length: i32 = from_value(this.get_field_slice("length")).unwrap(); + let this_length: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); for n in 0..this_length { new_values.push(this.get_field(&n.to_string())); } for concat_array in args { - let concat_length: i32 = from_value(concat_array.get_field_slice("length")).unwrap(); + let concat_length: i32 = from_value(concat_array.get_field_slice("length")).expect("Could not convert argument to i32"); for n in 0..concat_length { new_values.push(concat_array.get_field(&n.to_string())); } @@ -118,13 +118,13 @@ pub fn push(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { /// The last element of the array is removed from the array and returned. /// pub fn pop(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - let curr_length: i32 = from_value(this.get_field_slice("length")).unwrap(); + let curr_length: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); if curr_length < 1 { return Err(to_value( "Cannot pop() on an array with zero length".to_string(), )); } - let pop_index = curr_length - 1; + let pop_index = curr_length.wrapping_sub(1); let pop_value: Value = this.get_field(&pop_index.to_string()); this.remove_prop(&pop_index.to_string()); this.set_field_slice("length", to_value(pop_index)); @@ -141,11 +141,11 @@ pub fn join(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { let separator = if args.is_empty() { String::from(",") } else { - args[0].to_string() + args.get(0).expect("Could not get argument").to_string() }; let mut elem_strs: Vec = Vec::new(); - let length: i32 = from_value(this.get_field_slice("length")).unwrap(); + let length: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); for n in 0..length { let elem_str: String = this.get_field(&n.to_string()).to_string(); elem_strs.push(elem_str); @@ -159,12 +159,13 @@ pub fn join(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { /// The elements of the array are rearranged so as to reverse their order. /// The object is returned as the result of the call. /// +#[allow(clippy::else_if_without_else)] pub fn reverse(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - let len: i32 = from_value(this.get_field_slice("length")).unwrap(); - let middle: i32 = len / 2; + let len: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); + let middle: i32 = len.wrapping_div(2); for lower in 0..middle { - let upper = len - lower - 1; + let upper = len.wrapping_sub(lower).wrapping_sub(1); let upper_exists = this.has_field(&upper.to_string()); let lower_exists = this.has_field(&lower.to_string()); @@ -192,7 +193,7 @@ pub fn reverse(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { /// The first element of the array is removed from the array and returned. /// pub fn shift(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - let len: i32 = from_value(this.get_field_slice("length")).unwrap(); + let len: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); if len == 0 { this.set_field_slice("length", to_value(0_i32)); @@ -204,7 +205,7 @@ pub fn shift(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { for k in 1..len { let from = k.to_string(); - let to = (k - 1).to_string(); + let to = (k.wrapping_sub(1)).to_string(); let from_value = this.get_field(&from); if from_value == Gc::new(ValueData::Undefined) { @@ -214,8 +215,9 @@ pub fn shift(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { } } - this.remove_prop(&(len - 1).to_string()); - this.set_field_slice("length", to_value(len - 1)); + let final_index = len.wrapping_sub(1); + this.remove_prop(&(final_index).to_string()); + this.set_field_slice("length", to_value(final_index)); Ok(first) } @@ -227,13 +229,13 @@ pub fn shift(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { /// argument list. /// pub fn unshift(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { - let len: i32 = from_value(this.get_field_slice("length")).unwrap(); + let len: i32 = from_value(this.get_field_slice("length")).expect("Could not convert argument to i32"); let arg_c: i32 = args.len() as i32; if arg_c > 0 { for k in (1..=len).rev() { - let from = (k - 1).to_string(); - let to = (k + arg_c - 1).to_string(); + let from = (k.wrapping_sub(1)).to_string(); + let to = (k.wrapping_add(arg_c).wrapping_sub(1)).to_string(); let from_value = this.get_field(&from); if from_value == Gc::new(ValueData::Undefined) { @@ -243,12 +245,13 @@ pub fn unshift(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue } } for j in 0..arg_c { - this.set_field_slice(&j.to_string(), args[j as usize].clone()); + this.set_field_slice(&j.to_string(), args.get(j as usize).expect("Could not get argument").clone()); } } - this.set_field_slice("length", to_value(len + arg_c)); - Ok(to_value(len + arg_c)) + let temp = len.wrapping_add(arg_c); + this.set_field_slice("length", to_value(temp)); + Ok(to_value(temp)) } /// Create a new `Array` object diff --git a/src/lib/js/boolean.rs b/src/lib/js/boolean.rs index dee87e616c..83a980bd09 100644 --- a/src/lib/js/boolean.rs +++ b/src/lib/js/boolean.rs @@ -101,6 +101,7 @@ mod tests { assert_eq!(boolean_constructor.is_function(), true); } + #[allow(clippy::result_unwrap_used)] #[test] /// Test the correct type is returned from call and construct fn construct_and_call() { diff --git a/src/lib/js/console.rs b/src/lib/js/console.rs index a257c28e06..a2b74f8925 100644 --- a/src/lib/js/console.rs +++ b/src/lib/js/console.rs @@ -41,10 +41,10 @@ fn log_string_from(x: Value) -> String { .unwrap() .value .clone() - .unwrap() + .expect("Could not borrow value") .clone(), ) - .unwrap(); + .expect("Could not convert JS value to i32"); for i in 0..len { // Introduce recursive call to stringify any objects // which are part of the Array @@ -55,11 +55,11 @@ fn log_string_from(x: Value) -> String { .unwrap() .value .clone() - .unwrap() + .expect("Could not borrow value") .clone(), ); write!(s, "{}", arr_str).unwrap(); - if i != len - 1 { + if i != len.wrapping_sub(1) { write!(s, ", ").unwrap(); } } @@ -79,7 +79,7 @@ fn log_string_from(x: Value) -> String { s, "{}: {}", key, - log_string_from(val.value.clone().unwrap().clone()) + log_string_from(val.value.clone().expect("Could not read value").clone()) ) .unwrap(); if key != last_key { @@ -93,7 +93,7 @@ fn log_string_from(x: Value) -> String { s } - _ => from_value::(x.clone()).unwrap(), + _ => from_value::(x.clone()).expect("Could not convert value to String"), } } @@ -113,7 +113,7 @@ pub fn log(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { pub fn error(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { let args: Vec = FromIterator::from_iter( args.iter() - .map(|x| from_value::(x.clone()).unwrap()), + .map(|x| from_value::(x.clone()).expect("Could not convert value to String")), ); println!("{}", args.join(" ")); Ok(Gc::new(ValueData::Undefined)) diff --git a/src/lib/js/function.rs b/src/lib/js/function.rs index 937880d0c9..00af71bf02 100644 --- a/src/lib/js/function.rs +++ b/src/lib/js/function.rs @@ -115,7 +115,7 @@ pub fn create_unmapped_arguments_object(arguments_list: Vec) -> Value { obj.define_own_property("length".to_string(), length); let mut index: usize = 0; while index < len { - let val = arguments_list.get(index).unwrap(); + let val = arguments_list.get(index).expect("Could not get argument"); let mut prop = Property::default(); prop = prop .value(val.clone()) @@ -136,6 +136,7 @@ mod tests { use crate::realm::Realm; use crate::{forward, forward_val, js::value::from_value}; + #[allow(clippy::float_cmp)] #[test] fn check_arguments_object() { let realm = Realm::create(); @@ -148,8 +149,9 @@ mod tests { "#; forward(&mut engine, init); + let expected_return_val: f64 = 100.0; let return_val = forward_val(&mut engine, "val").expect("value expected"); assert_eq!(return_val.is_double(), true); - assert_eq!(from_value::(return_val).unwrap(), 100.0); + assert_eq!(from_value::(return_val).expect("Could not convert value to f64"), expected_return_val); } } diff --git a/src/lib/js/math.rs b/src/lib/js/math.rs index 0dd92cd270..78e21f6003 100644 --- a/src/lib/js/math.rs +++ b/src/lib/js/math.rs @@ -13,8 +13,8 @@ pub fn abs(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .abs() })) } @@ -23,8 +23,8 @@ pub fn acos(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .acos() })) } @@ -33,8 +33,8 @@ pub fn asin(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .asin() })) } @@ -43,8 +43,8 @@ pub fn atan(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .atan() })) } @@ -53,9 +53,9 @@ pub fn atan2(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() - .atan2(args.get(1).unwrap().to_num()) + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") + .atan2(args.get(1).expect("Could not get argument").to_num()) })) } /// Get the cubic root of a number @@ -63,8 +63,8 @@ pub fn cbrt(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .cbrt() })) } @@ -73,8 +73,8 @@ pub fn ceil(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .ceil() })) } @@ -83,8 +83,8 @@ pub fn cos(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .cos() })) } @@ -93,8 +93,8 @@ pub fn exp(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .exp() })) } @@ -103,8 +103,8 @@ pub fn floor(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .floor() })) } @@ -113,8 +113,8 @@ pub fn log(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .log(f64::consts::E) })) } @@ -139,8 +139,8 @@ pub fn min(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { /// Raise a number to a power pub fn pow(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.len() >= 2 { - let num: f64 = from_value(args.get(0).unwrap().clone()).unwrap(); - let power: f64 = from_value(args.get(1).unwrap().clone()).unwrap(); + let num: f64 = from_value(args.get(0).expect("Could not get argument").clone()).expect("Could not convert argument to f64"); + let power: f64 = from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert argument to f64"); num.powf(power) } else { f64::NAN @@ -155,8 +155,8 @@ pub fn round(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .round() })) } @@ -165,8 +165,8 @@ pub fn sin(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .sin() })) } @@ -175,8 +175,8 @@ pub fn sqrt(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .sqrt() })) } @@ -185,8 +185,8 @@ pub fn tan(_: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { Ok(to_value(if args.is_empty() { f64::NAN } else { - from_value::(args.get(0).unwrap().clone()) - .unwrap() + from_value::(args.get(0).expect("Could not get argument").clone()) + .expect("Could not convert argument to f64") .tan() })) } diff --git a/src/lib/js/regexp.rs b/src/lib/js/regexp.rs index f95475cbb3..18f123fcf7 100644 --- a/src/lib/js/regexp.rs +++ b/src/lib/js/regexp.rs @@ -62,10 +62,10 @@ pub fn make_regexp(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultV if slots.get("RegExpMatcher").is_some() { // first argument is another `RegExp` object, so copy its pattern and flags if let Some(body) = slots.get("OriginalSource") { - regex_body = from_value(body.clone()).unwrap(); + regex_body = from_value(body.clone()).expect("Could not convert value to String"); } if let Some(flags) = slots.get("OriginalFlags") { - regex_flags = from_value(flags.clone()).unwrap(); + regex_flags = from_value(flags.clone()).expect("Could not convert value to String"); } } } @@ -227,7 +227,7 @@ pub fn exec(this: &Value, args: &[Value], _: &mut Interpreter) -> ResultValue { let mut result = Vec::with_capacity(locations.len()); for i in 0..locations.len() { if let Some((start, end)) = locations.get(i) { - result.push(to_value(&arg_str[start..end])); + result.push(to_value(arg_str.get(start..end).expect("Could not get slice"))); } else { result.push(Gc::new(ValueData::Undefined)); } diff --git a/src/lib/js/string.rs b/src/lib/js/string.rs index 010a69098f..3db8d208b3 100644 --- a/src/lib/js/string.rs +++ b/src/lib/js/string.rs @@ -199,11 +199,11 @@ pub fn slice(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue min(end, length) }; - let span = max(to - from, 0); + let span = max(to.wrapping_sub(from), 0); let mut new_str = String::new(); - for i in from..from + span { - new_str.push(primitive_val.chars().nth(i as usize).unwrap()); + for i in from..from.wrapping_add(span) { + new_str.push(primitive_val.chars().nth(i as usize).expect("Could not get nth char")); } Ok(to_value(new_str)) } @@ -272,11 +272,11 @@ pub fn ends_with(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultV let end_position: i32 = if args.len() < 2 { length } else { - from_value(args[1].clone()).unwrap() + from_value(args.get(1).expect("Could not get argumetn").clone()).expect("Could not convert value to i32") }; let end = min(max(end_position, 0), length); - let start = end - search_length; + let start = end.wrapping_sub(search_length); if start < 0 { Ok(to_value(false)) @@ -311,7 +311,7 @@ pub fn includes(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultVa let position: i32 = if args.len() < 2 { 0 } else { - from_value(args[1].clone()).unwrap() + from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert value to i32") }; let start = min(max(position, 0), length); @@ -347,7 +347,7 @@ pub fn index_of(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultVa let position: i32 = if args.len() < 2 { 0 } else { - from_value(args[1].clone()).unwrap() + from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert value to i32") }; let start = min(max(position, 0), length); @@ -392,7 +392,7 @@ pub fn last_index_of(this: &Value, args: &[Value], ctx: &mut Interpreter) -> Res let position: i32 = if args.len() < 2 { 0 } else { - from_value(args[1].clone()).unwrap() + from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert value to i32") }; let start = min(max(position, 0), length); @@ -437,7 +437,7 @@ fn string_pad( return Ok(to_value(primitive)); } - let fill_len = max_length - primitive_length; + let fill_len = max_length.wrapping_sub(primitive_length); let mut fill_str = String::new(); while fill_str.len() < fill_len as usize { @@ -447,9 +447,9 @@ fn string_pad( let concat_fill_str: String = fill_str.chars().take(fill_len as usize).collect(); if at_start { - Ok(to_value(concat_fill_str + &primitive)) + Ok(to_value(format!("{}{}", concat_fill_str, &primitive))) } else { - Ok(to_value(primitive + &concat_fill_str)) + Ok(to_value(format!("{}{}", primitive, &concat_fill_str))) } } @@ -471,7 +471,7 @@ pub fn pad_end(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultVal .expect("failed to parse argument for String method"); let fill_string: Option = match args.len() { 1 => None, - _ => Some(from_value(args[1].clone()).unwrap()), + _ => Some(from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert value to Option")), }; string_pad(primitive_val, max_length, fill_string, false) @@ -495,7 +495,7 @@ pub fn pad_start(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultV .expect("failed to parse argument for String method"); let fill_string: Option = match args.len() { 1 => None, - _ => Some(from_value(args[1].clone()).unwrap()), + _ => Some(from_value(args.get(1).expect("Could not get argument").clone()).expect("Could not convert value to Option")), }; string_pad(primitive_val, max_length, fill_string, true) @@ -587,7 +587,7 @@ pub fn substring(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultV let end = if args.len() < 2 { length } else { - from_value(args[1].clone()).expect("failed to parse argument for String method") + from_value(args.get(1).expect("Could not get argument").clone()).expect("failed to parse argument for String method") }; // Both start and end args replaced by 0 if they were negative // or by the length of the String if they were greater @@ -598,7 +598,7 @@ pub fn substring(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultV let to = max(final_start, final_end) as usize; // Extract the part of the string contained between the start index and the end index // where start is guaranteed to be smaller or equals to end - let extracted_string: String = primitive_val.chars().skip(from).take(to - from).collect(); + let extracted_string: String = primitive_val.chars().skip(from).take(to.wrapping_sub(from)).collect(); Ok(to_value(extracted_string)) } @@ -631,15 +631,15 @@ pub fn substr(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValu let end = if args.len() < 2 { i32::max_value() } else { - from_value(args[1].clone()).expect("failed to parse argument for String method") + from_value(args.get(1).expect("Could not get argument").clone()).expect("failed to parse argument for String method") }; // If start is negative it become the number of code units from the end of the string if start < 0 { - start = max(length + start, 0); + start = max(length.wrapping_add(start), 0); } // length replaced by 0 if it was negative // or by the number of code units from start to the end of the string if it was greater - let result_length = min(max(end, 0), length - start); + let result_length = min(max(end, 0), length.wrapping_sub(start)); // If length is negative we return an empty string // otherwise we extract the part of the string from start and is length code units long if result_length <= 0 { @@ -765,6 +765,7 @@ mod tests { //assert_eq!(b, String::from("Hello, world! Have a nice day.")); } + #[allow(clippy::result_unwrap_used)] #[test] /// Test the correct type is returned from call and construct fn construct_and_call() { diff --git a/src/lib/js/value.rs b/src/lib/js/value.rs index 53a7d4bdec..5871b41b3b 100644 --- a/src/lib/js/value.rs +++ b/src/lib/js/value.rs @@ -307,7 +307,7 @@ impl ValueData { match prop_getter { Some(val) => val, None => { - let val = prop.value.as_ref().unwrap(); + let val = prop.value.as_ref().expect("Could not get property as reference"); val.clone() } } @@ -481,7 +481,7 @@ impl ValueData { /// Convert from a JSON value to a JS value pub fn from_json(json: JSONValue) -> Self { match json { - JSONValue::Number(v) => ValueData::Number(v.as_f64().unwrap()), + JSONValue::Number(v) => ValueData::Number(v.as_f64().expect("Could not convert value to f64")), JSONValue::String(v) => ValueData::String(v), JSONValue::Bool(v) => ValueData::Boolean(v), JSONValue::Array(vs) => { @@ -527,7 +527,7 @@ impl ValueData { JSONValue::Object(new_obj) } ValueData::String(ref str) => JSONValue::String(str.clone()), - ValueData::Number(num) => JSONValue::Number(JSONNumber::from_f64(num).unwrap()), + ValueData::Number(num) => JSONValue::Number(JSONNumber::from_f64(num).expect("Could not convert to JSONNumber")), ValueData::Integer(val) => JSONValue::Number(JSONNumber::from(val)), } } @@ -619,8 +619,8 @@ impl Add for ValueData { type Output = Self; fn add(self, other: Self) -> Self { match (self, other) { - (ValueData::String(ref s), ref o) => ValueData::String(s.clone() + &o.to_string()), - (ref s, ValueData::String(ref o)) => ValueData::String(s.to_string() + o), + (ValueData::String(ref s), ref o) => ValueData::String(format!("{}{}", s.clone(), &o.to_string())), + (ref s, ValueData::String(ref o)) => ValueData::String(format!("{}{}", s.to_string(), o)), (ref s, ref o) => ValueData::Number(s.to_num() + o.to_num()), } } @@ -725,7 +725,7 @@ impl FromValue for String { impl<'s> ToValue for &'s str { fn to_value(&self) -> Value { - Gc::new(ValueData::String(String::from_str(*self).unwrap())) + Gc::new(ValueData::String(String::from_str(*self).expect("Could not convert string to self to String"))) } } @@ -736,7 +736,7 @@ impl ToValue for char { } impl FromValue for char { fn from_value(v: Value) -> Result { - Ok(v.to_string().chars().next().unwrap()) + Ok(v.to_string().chars().next().expect("Could not get next char")) } } diff --git a/src/lib/syntax/lexer.rs b/src/lib/syntax/lexer.rs index 9d4bbe550b..334185732d 100644 --- a/src/lib/syntax/lexer.rs +++ b/src/lib/syntax/lexer.rs @@ -180,7 +180,7 @@ impl<'a> Lexer<'a> { F: FnMut(char) -> bool, { let mut s = String::new(); - while self.buffer.peek().is_some() && f(self.preview_next().unwrap()) { + while self.buffer.peek().is_some() && f(self.preview_next().expect("Could not preview next value")) { s.push(self.next()?); } @@ -253,7 +253,7 @@ impl<'a> Lexer<'a> { if self.next_is('{') { let s = self .take_char_while(char::is_alphanumeric) - .unwrap(); + .expect("Could not read chars"); // We know this is a single unicode codepoint, convert to u32 let as_num = match u32::from_str_radix(&s, 16) { @@ -264,7 +264,7 @@ impl<'a> Lexer<'a> { .expect("Invalid Unicode escape sequence"); self.next()?; // '}' - self.column_number += s.len() as u64 + 3; + self.column_number += (s.len() as u64).wrapping_add(3); c } else { let mut codepoints: Vec = vec![]; @@ -272,7 +272,7 @@ impl<'a> Lexer<'a> { // Collect each character after \u e.g \uD83D will give "D83D" let s = self .take_char_while(char::is_alphanumeric) - .unwrap(); + .expect("Could not read chars"); // Convert to u16 let as_num = match u16::from_str_radix(&s, 16) { @@ -281,7 +281,7 @@ impl<'a> Lexer<'a> { }; codepoints.push(as_num); - self.column_number += s.len() as u64 + 2; + self.column_number += (s.len() as u64).wrapping_add(2); // Check for another UTF-16 codepoint if self.next_is('\\') && self.next_is('u') { @@ -294,8 +294,8 @@ impl<'a> Lexer<'a> { // Rust's decode_utf16 will deal with it regardless decode_utf16(codepoints.iter().cloned()) .next() - .unwrap() - .unwrap() + .expect("Could not get next codepoint") + .expect("Could not get next codepoint") } } '\'' | '"' | '\\' => escape, @@ -315,7 +315,7 @@ impl<'a> Lexer<'a> { // Why +1? Quotation marks are not included, // So technically it would be +2, (for both " ") but we want to be 1 less // to compensate for the incrementing at the top - self.column_number += str_length + 1; + self.column_number += str_length.wrapping_add(1); } '0' => { let mut buf = String::new(); @@ -327,7 +327,7 @@ impl<'a> Lexer<'a> { break; } } - u64::from_str_radix(&buf, 16).unwrap() + u64::from_str_radix(&buf, 16).expect("Could not convert value to u64") } else if self.next_is('b') { while let Some(ch) = self.preview_next() { if ch.is_digit(2) { @@ -336,14 +336,14 @@ impl<'a> Lexer<'a> { break; } } - u64::from_str_radix(&buf, 2).unwrap() + u64::from_str_radix(&buf, 2).expect("Could not convert value to u64") } else { let mut gone_decimal = false; loop { let next_ch = self.preview_next().unwrap_or('_'); match next_ch { - next_ch if next_ch.is_digit(8) => { - buf.push(next_ch); + c if next_ch.is_digit(8) => { + buf.push(c); self.next()?; } 'O' | 'o' => { @@ -358,11 +358,11 @@ impl<'a> Lexer<'a> { } } if gone_decimal { - u64::from_str(&buf).unwrap() + u64::from_str(&buf).expect("Could not convert value to u64r") } else if buf.is_empty() { 0 } else { - u64::from_str_radix(&buf, 8).unwrap() + u64::from_str_radix(&buf, 8).expect("Could not convert value to u64") } }; self.push_token(TokenData::NumericLiteral(num as f64)) @@ -374,12 +374,12 @@ impl<'a> Lexer<'a> { '.' => loop { buf.push(self.next()?); - let ch = match self.preview_next() { + let c = match self.preview_next() { Some(ch) => ch, None => break, }; - if !ch.is_digit(10) { + if !c.is_digit(10) { break 'digitloop; } }, @@ -393,7 +393,7 @@ impl<'a> Lexer<'a> { } } // TODO make this a bit more safe -------------------------------VVVV - self.push_token(TokenData::NumericLiteral(f64::from_str(&buf).unwrap())) + self.push_token(TokenData::NumericLiteral(f64::from_str(&buf).expect("Could not convert value to f64"))) } _ if ch.is_alphabetic() || ch == '$' || ch == '_' => { let mut buf = ch.to_string(); @@ -419,7 +419,7 @@ impl<'a> Lexer<'a> { } }); // Move position forward the length of keyword - self.column_number += (buf_compare.len() - 1) as u64; + self.column_number += (buf_compare.len().wrapping_sub(1)) as u64; } ';' => self.push_punc(Punctuator::Semicolon), ':' => self.push_punc(Punctuator::Colon), @@ -573,7 +573,7 @@ impl<'a> Lexer<'a> { self.column_number = 0; } ' ' => (), - ch => panic!( + _ => panic!( "{}:{}: Unexpected '{}'", self.line_number, self.column_number, ch ), @@ -582,6 +582,7 @@ impl<'a> Lexer<'a> { } } +#[allow(clippy::indexing_slicing)] #[cfg(test)] mod tests { use super::*; diff --git a/src/lib/syntax/parser.rs b/src/lib/syntax/parser.rs index 4c94e523c6..a4628f9227 100644 --- a/src/lib/syntax/parser.rs +++ b/src/lib/syntax/parser.rs @@ -201,7 +201,7 @@ impl Parser { ExprDef::If( Box::new(cond), Box::new(expr), - if next.is_ok() && next.unwrap().data == TokenData::Keyword(Keyword::Else) { + if next.is_ok() && next.expect("Could not get next value").data == TokenData::Keyword(Keyword::Else) { self.pos += 1; Some(Box::new(self.parse()?)) } else { @@ -227,7 +227,7 @@ impl Parser { self.expect_punc(Punctuator::OpenBlock, "switch block")?; let mut cases = Vec::new(); let mut default = None; - while self.pos + 1 < self.tokens.len() { + while self.pos.wrapping_add(1) < self.tokens.len() { let tok = self.get_token(self.pos)?; self.pos += 1; match tok.data { @@ -243,7 +243,7 @@ impl Parser { _ => block.push(self.parse()?), } } - cases.push((cond.unwrap(), block)); + cases.push((cond.expect("No condition supplied"), block)); } TokenData::Keyword(Keyword::Default) => { let mut block = Vec::new(); @@ -276,7 +276,7 @@ impl Parser { Ok(mk!( self, ExprDef::Switch( - Box::new(value.unwrap()), + Box::new(value.expect("Could not get value")), cases, match default { Some(v) => Some(Box::new(v)), @@ -369,7 +369,7 @@ impl Parser { TokenData::Punctuator(Punctuator::OpenParen) => { match self.get_token(self.pos)?.data { TokenData::Punctuator(Punctuator::CloseParen) - if self.get_token(self.pos + 1)?.data + if self.get_token(self.pos.wrapping_add(1))?.data == TokenData::Punctuator(Punctuator::Arrow) => { self.pos += 2; @@ -503,11 +503,11 @@ impl Parser { mk!(self, ExprDef::ObjectDecl(Box::new(BTreeMap::new())), token) } TokenData::Punctuator(Punctuator::OpenBlock) - if self.get_token(self.pos + 1)?.data + if self.get_token(self.pos.wrapping_add(1))?.data == TokenData::Punctuator(Punctuator::Colon) => { let mut map = Box::new(BTreeMap::new()); - while self.get_token(self.pos - 1)?.data == TokenData::Punctuator(Punctuator::Comma) + while self.get_token(self.pos.wrapping_sub(1))?.data == TokenData::Punctuator(Punctuator::Comma) || map.len() == 0 { let tk = self.get_token(self.pos)?; @@ -603,7 +603,7 @@ impl Parser { } TokenData::Punctuator(Punctuator::OpenParen) => { let mut args = Vec::new(); - let mut expect_comma_or_end = self.get_token(self.pos + 1)?.data + let mut expect_comma_or_end = self.get_token(self.pos.wrapping_add(1))?.data == TokenData::Punctuator(Punctuator::CloseParen); loop { self.pos += 1; @@ -822,7 +822,7 @@ impl Parser { /// Returns an error if the next symbol is not `tk` fn expect(&mut self, tk: TokenData, routine: &'static str) -> Result<(), ParseError> { self.pos += 1; - let curr_tk = self.get_token(self.pos - 1)?; + let curr_tk = self.get_token(self.pos.wrapping_sub(1))?; if curr_tk.data == tk { Ok(()) } else { @@ -845,6 +845,7 @@ mod tests { lexer::Lexer, }; + #[allow(clippy::result_unwrap_used)] fn check_parser(js: &str, expr: &[Expr]) { let mut lexer = Lexer::new(js); lexer.lex().expect("failed to lex");