diff --git a/.ci/build.sh b/.ci/build.sh index 65b853d5..877b0154 100755 --- a/.ci/build.sh +++ b/.ci/build.sh @@ -11,4 +11,7 @@ case "$1" in doc) cargo doc --all ;; -esac \ No newline at end of file + clippy) + cargo clippy --all-targets --all-features -- -D warnings + ;; +esac diff --git a/.ci/setup.sh b/.ci/setup.sh index 4a86379d..1e382e0e 100755 --- a/.ci/setup.sh +++ b/.ci/setup.sh @@ -1,9 +1,14 @@ #!/bin/bash -if [ "$1" == "format" ]; then - echo "Installing rustfmt..." - rustup toolchain install nightly - rustup component add --toolchain nightly rustfmt-preview - which rustfmt || cargo install --force rustfmt-nightly - cargo +nightly fmt -- --version -fi +case "$1" in + format) + echo "Installing rustfmt..." + rustup component add --toolchain nightly rustfmt-preview + which rustfmt || cargo install --force rustfmt-nightly + cargo +nightly fmt -- --version + ;; + clippy) + echo "Installing clippy..." + rustup component add clippy --toolchain=nightly || cargo install --git https://github.com/rust-lang/rust-clippy/ --force clippy + ;; +esac diff --git a/.travis.yml b/.travis.yml index 8b01e031..144c7c0c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,9 @@ matrix: - name: "Rust: format" env: ACTION=format rust: nightly + - name: "Rust: clippy" + env: ACTION=clippy + rust: nightly - name: "Rust: doc" env: ACTION=doc rust: stable diff --git a/starlark-repl/Cargo.toml b/starlark-repl/Cargo.toml index 42f15ace..a9959056 100644 --- a/starlark-repl/Cargo.toml +++ b/starlark-repl/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "starlark-repl" -version = "0.2.0" +version = "0.3.0-pre" authors = ["Damien Martin-Guillerez "] description = "A REPL for the implementation in Rust of the Starlark language." @@ -21,7 +21,7 @@ codemap = "0.1.1" codemap-diagnostic = "0.1" getopts = "0.2" linefeed = "0.5.3" -starlark = { version = "0.2", path = "../starlark" } +starlark = { path = "../starlark" } [lib] diff --git a/starlark-repl/src/lib.rs b/starlark-repl/src/lib.rs index a91c5a0d..4eb9f678 100644 --- a/starlark-repl/src/lib.rs +++ b/starlark-repl/src/lib.rs @@ -109,7 +109,7 @@ pub fn repl(global_environment: &Environment, dialect: Dialect) { reader.set_prompt(">>> ").unwrap(); while let Ok(ReadResult::Input(input)) = reader.read_line() { - if input.len() != 0 { + if !input.is_empty() { reader.set_prompt("... ").unwrap(); n += 1; let input = input + "\n"; diff --git a/starlark/Cargo.toml b/starlark/Cargo.toml index 697b06a0..67ee32a9 100644 --- a/starlark/Cargo.toml +++ b/starlark/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "starlark" -version = "0.2.0" +version = "0.3.0-pre" authors = ["Damien Martin-Guillerez "] build = "build.rs" diff --git a/starlark/src/environment/mod.rs b/starlark/src/environment/mod.rs index 84ebe031..cae09f94 100644 --- a/starlark/src/environment/mod.rs +++ b/starlark/src/environment/mod.rs @@ -24,9 +24,9 @@ use values::*; // TODO: move that code in some common error code list? // CM prefix = Critical Module -const FROZEN_ENV_ERROR_CODE: &'static str = "CM00"; -const NOT_FOUND_ERROR_CODE: &'static str = "CM01"; -const CANNOT_IMPORT_ERROR_CODE: &'static str = "CE02"; +const FROZEN_ENV_ERROR_CODE: &str = "CM00"; +const NOT_FOUND_ERROR_CODE: &str = "CM01"; +const CANNOT_IMPORT_ERROR_CODE: &str = "CE02"; #[derive(Debug)] #[doc(hidden)] @@ -207,7 +207,7 @@ impl EnvironmentContent { /// Get the value of the variable `name` pub fn get(&self, name: &str) -> Result { if self.variables.contains_key(name) { - Ok(self.variables.get(name).unwrap().clone()) + Ok(self.variables[name].clone()) } else { match self.parent { Some(ref p) => p.get(name), @@ -233,7 +233,7 @@ impl EnvironmentContent { pub fn get_type_value(&self, obj: &Value, id: &str) -> Option { match self.type_objs.get(obj.get_type()) { Some(ref d) => match d.get(id) { - Some(&ref v) => Some(v.clone()), + Some(v) => Some(v.clone()), None => match self.parent { Some(ref p) => p.get_type_value(obj, id), None => None, @@ -254,12 +254,10 @@ impl EnvironmentContent { r.push(k.clone()); } r + } else if let Some(ref p) = self.parent { + p.list_type_value(obj) } else { - if let Some(ref p) = self.parent { - p.list_type_value(obj) - } else { - Vec::new() - } + Vec::new() } } diff --git a/starlark/src/eval/mod.rs b/starlark/src/eval/mod.rs index da57fe05..f86009e2 100644 --- a/starlark/src/eval/mod.rs +++ b/starlark/src/eval/mod.rs @@ -47,17 +47,17 @@ macro_rules! eval_vector { // TODO: move that code in some common error code list? // CE prefix = Critical Evaluation #[doc(hidden)] -pub const BREAK_ERROR_CODE: &'static str = "CE00"; +pub const BREAK_ERROR_CODE: &str = "CE00"; #[doc(hidden)] -pub const CONTINUE_ERROR_CODE: &'static str = "CE01"; +pub const CONTINUE_ERROR_CODE: &str = "CE01"; #[doc(hidden)] -pub const RETURN_ERROR_CODE: &'static str = "CE02"; +pub const RETURN_ERROR_CODE: &str = "CE02"; #[doc(hidden)] -pub const INCORRECT_LEFT_VALUE_ERROR_CODE: &'static str = "CE03"; +pub const INCORRECT_LEFT_VALUE_ERROR_CODE: &str = "CE03"; #[doc(hidden)] -pub const INCORRECT_UNPACK_ERROR_CODE: &'static str = "CE04"; +pub const INCORRECT_UNPACK_ERROR_CODE: &str = "CE04"; #[doc(hidden)] -pub const RECURSION_ERROR_CODE: &'static str = "CE05"; +pub const RECURSION_ERROR_CODE: &str = "CE05"; #[doc(hidden)] #[derive(Debug, Clone)] @@ -247,14 +247,14 @@ impl Evaluate for AstString { impl Evaluate for AstInt { fn eval(&self, _context: &mut EvaluationContext) -> EvalResult { - Ok(Value::new(self.node.clone())) + Ok(Value::new(self.node)) } fn transform( &self, _context: &mut EvaluationContext, ) -> Result>, EvalException> { - Ok(Box::new(self.clone())) + Ok(Box::new(*self)) } fn set(&self, _context: &mut EvaluationContext, _new_value: Value) -> EvalResult { @@ -262,7 +262,7 @@ impl Evaluate for AstInt { } } -fn eval_comprehension_clause<'a, T: FileLoader + 'static>( +fn eval_comprehension_clause( context: &mut EvaluationContext, e: &AstExpr, clauses: &[AstClause], @@ -283,7 +283,7 @@ fn eval_comprehension_clause<'a, T: FileLoader + 'static>( Clause::For(ref k, ref v) => { let mut iterable = v.eval(context)?; iterable.freeze_for_iteration(); - for i in t!(iterable.into_iter(), c)? { + for i in t!(iterable.iter(), c)? { k.set(context, i)?; if tl.is_empty() { result.push(e.eval(context)?); @@ -299,6 +299,152 @@ fn eval_comprehension_clause<'a, T: FileLoader + 'static>( Ok(result) } +fn eval_compare( + this: &AstExpr, + left: &AstExpr, + right: &AstExpr, + cmp: F, + context: &mut EvaluationContext, +) -> EvalResult +where + F: Fn(Ordering) -> bool, +{ + let l = left.eval(context)?; + let r = right.eval(context)?; + Ok(Value::new(cmp(t!(l.compare(&r, 0), this)?))) +} + +fn eval_slice( + this: &AstExpr, + a: &AstExpr, + start: &Option, + stop: &Option, + stride: &Option, + context: &mut EvaluationContext, +) -> EvalResult { + let a = a.eval(context)?; + let start = match start { + Some(ref e) => Some(e.eval(context)?), + None => None, + }; + let stop = match stop { + Some(ref e) => Some(e.eval(context)?), + None => None, + }; + let stride = match stride { + Some(ref e) => Some(e.eval(context)?), + None => None, + }; + t!(a.slice(start, stop, stride), this) +} + +fn eval_call( + this: &AstExpr, + e: &AstExpr, + pos: &[AstExpr], + named: &[(AstString, AstExpr)], + args: &Option, + kwargs: &Option, + context: &mut EvaluationContext, +) -> EvalResult { + let npos = eval_vector!(pos, context); + let mut nnamed = HashMap::new(); + for &(ref k, ref v) in named.iter() { + nnamed.insert(k.eval(context)?.to_str(), v.eval(context)?); + } + let nargs = if let Some(ref x) = args { + Some(x.eval(context)?) + } else { + None + }; + let nkwargs = if let Some(ref x) = kwargs { + Some(x.eval(context)?) + } else { + None + }; + let f = e.eval(context)?; + let fname = f.to_repr(); + let descr = f.to_str(); + let mut new_stack = context.call_stack.clone(); + if context.call_stack.iter().any(|x| x.0 == fname) { + Err(EvalException::Recursion(this.span, fname, new_stack)) + } else { + let loc = { context.map.lock().unwrap().look_up_pos(this.span.low()) }; + new_stack.push(( + fname, + format!( + "call to {} at {}:{}", + descr, + loc.file.name(), + loc.position.line + 1, // line 1 is 0, so add 1 for human readable. + ), + )); + t!( + e.eval(context,)?.call( + &new_stack, + context.env.clone(), + npos, + nnamed, + nargs, + nkwargs, + ), + this + ) + } +} + +fn eval_dot( + this: &AstExpr, + e: &AstExpr, + s: &AstString, + context: &mut EvaluationContext, +) -> EvalResult { + let left = e.eval(context)?; + if let Some(v) = context.env.get_type_value(&left, &s.node) { + if v.get_type() == "function" { + // Insert self so the method see the object it is acting on + Ok(function::Function::new_self_call(left.clone(), v)) + } else { + Ok(v) + } + } else { + t!(left.get_attr(&s.node), this) + } +} + +fn eval_dict_comprehension( + k: &AstExpr, + v: &AstExpr, + clauses: &[AstClause], + context: &mut EvaluationContext, +) -> EvalResult { + let mut r = LinkedHashMap::new(); + let tuple = Box::new(Spanned { + span: k.span.merge(v.span), + node: Expr::Tuple(vec![k.clone(), v.clone()]), + }); + let mut context = context.child("dict_comprehension"); + for e in eval_comprehension_clause(&mut context, &tuple, clauses)? { + let k = t!(e.at(Value::from(0)), tuple)?; + let v = t!(e.at(Value::from(1)), tuple)?; + r.insert(k, v); + } + Ok(Value::from(r)) +} + +fn eval_list_comprehension( + e: &AstExpr, + clauses: &[AstClause], + context: &mut EvaluationContext, +) -> EvalResult { + let mut r = Vec::new(); + let mut context = context.child("list_comprehension"); + for v in eval_comprehension_clause(&mut context, e, clauses)? { + r.push(v.clone()); + } + Ok(Value::from(r)) +} + enum TransformedExpr { Dot(Value, String, Span), ArrayIndirection(Value, Value, Span), @@ -317,31 +463,28 @@ impl Evaluate for TransformedExpr { fn set(&self, context: &mut EvaluationContext, new_value: Value) -> EvalResult { let ok = Ok(Value::new(None)); match self { - &TransformedExpr::List(ref v, ref span) | &TransformedExpr::Tuple(ref v, ref span) => { - let span = span.clone(); + TransformedExpr::List(ref v, ref span) | &TransformedExpr::Tuple(ref v, ref span) => { let l = v.len() as i64; - let nvl = t!(new_value.length(), span span)?; + let nvl = t!(new_value.length(), span * span)?; if nvl != l { - Err(EvalException::IncorrectNumberOfValueToUnpack(span, l, nvl)) + Err(EvalException::IncorrectNumberOfValueToUnpack(*span, l, nvl)) } else { let mut r = Vec::new(); let mut it1 = v.iter(); // TODO: the span here should probably include the rvalue - let mut it2 = t!(new_value.into_iter(), span span)?; + let mut it2 = t!(new_value.iter(), span * span)?; for _ in 0..l { r.push(it1.next().unwrap().set(context, it2.next().unwrap())?) } ok } } - &TransformedExpr::Dot(ref e, ref s, ref span) => { - let span = span.clone(); - t!(e.clone().set_attr(&s, new_value), span span)?; + TransformedExpr::Dot(ref e, ref s, ref span) => { + t!(e.clone().set_attr(&s, new_value), span * span)?; ok } - &TransformedExpr::ArrayIndirection(ref e, ref idx, ref span) => { - let span = span.clone(); - t!(e.clone().set_at(idx.clone(), new_value), span span)?; + TransformedExpr::ArrayIndirection(ref e, ref idx, ref span) => { + t!(e.clone().set_at(idx.clone(), new_value), span * span)?; ok } } @@ -349,15 +492,15 @@ impl Evaluate for TransformedExpr { fn eval(&self, context: &mut EvaluationContext) -> EvalResult { match self { - &TransformedExpr::Tuple(ref v, ..) => { + TransformedExpr::Tuple(ref v, ..) => { let r = eval_vector!(v, context); Ok(Value::new(tuple::Tuple::new(r.as_slice()))) } - &TransformedExpr::List(ref v, ..) => { + TransformedExpr::List(ref v, ..) => { let r = eval_vector!(v, context); Ok(Value::from(r)) } - &TransformedExpr::Dot(ref left, ref s, ref span) => { + TransformedExpr::Dot(ref left, ref s, ref span) => { if let Some(v) = context.env.get_type_value(left, &s) { if v.get_type() == "function" { // Insert self so the method see the object it is acting on @@ -369,7 +512,7 @@ impl Evaluate for TransformedExpr { t!(left.get_attr(&s), span span.clone()) } } - &TransformedExpr::ArrayIndirection(ref e, ref idx, ref span) => { + TransformedExpr::ArrayIndirection(ref e, ref idx, ref span) => { t!(e.at(idx.clone()), span span.clone()) } } @@ -415,84 +558,16 @@ impl Evaluate for AstExpr { let r = eval_vector!(v, context); Ok(Value::new(tuple::Tuple::new(r.as_slice()))) } - Expr::Dot(ref e, ref s) => { - let left = e.eval(context)?; - if let Some(v) = context.env.get_type_value(&left, &s.node) { - if v.get_type() == "function" { - // Insert self so the method see the object it is acting on - Ok(function::Function::new_self_call(left.clone(), v)) - } else { - Ok(v) - } - } else { - t!(left.get_attr(&s.node), self) - } - } + Expr::Dot(ref e, ref s) => eval_dot(self, e, s, context), Expr::Call(ref e, ref pos, ref named, ref args, ref kwargs) => { - let npos = eval_vector!(pos, context); - let mut nnamed = HashMap::new(); - for &(ref k, ref v) in named.iter() { - nnamed.insert(k.eval(context)?.to_str(), v.eval(context)?); - } - let nargs = if let &Some(ref x) = args { - Some(x.eval(context)?) - } else { - None - }; - let nkwargs = if let &Some(ref x) = kwargs { - Some(x.eval(context)?) - } else { - None - }; - let f = e.eval(context)?; - let fname = f.to_repr(); - let descr = f.to_str(); - let mut new_stack = context.call_stack.clone(); - if context.call_stack.iter().any(|x| x.0 == fname) { - Err(EvalException::Recursion(self.span, fname, new_stack)) - } else { - let loc = { context.map.lock().unwrap().look_up_pos(self.span.low()) }; - new_stack.push(( - fname, - format!( - "call to {} at {}:{}", - descr, - loc.file.name(), - loc.position.line + 1, // line 1 is 0, so add 1 for human readable. - ), - )); - t!( - e.eval(context,)?.call( - &new_stack, - context.env.clone(), - npos, - nnamed, - nargs, - nkwargs, - ), - self - ) - } + eval_call(self, e, pos, named, args, kwargs, context) } Expr::ArrayIndirection(ref e, ref idx) => { let idx = idx.eval(context)?; t!(e.eval(context)?.at(idx), self) } Expr::Slice(ref a, ref start, ref stop, ref stride) => { - let a = a.eval(context)?; - let start = match start { - &Some(ref e) => Some(e.eval(context)?), - &None => None, - }; - let stop = match stop { - &Some(ref e) => Some(e.eval(context)?), - &None => None, - }; - let stride = match stride { - &Some(ref e) => Some(e.eval(context)?), - &None => None, - }; - t!(a.slice(start, stop, stride), self) + eval_slice(self, a, start, stop, stride, context) } Expr::Identifier(ref i) => t!(context.env.get(&i.node), i), Expr::IntLiteral(ref i) => Ok(Value::new(i.node)), @@ -509,34 +584,22 @@ impl Evaluate for AstExpr { Ok(if !l.to_bool() { l } else { r.eval(context)? }) } Expr::Op(BinOp::EqualsTo, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? == Ordering::Equal)) + eval_compare(self, l, r, |x| x == Ordering::Equal, context) } Expr::Op(BinOp::Different, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? != Ordering::Equal)) + eval_compare(self, l, r, |x| x != Ordering::Equal, context) } Expr::Op(BinOp::LowerThan, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? == Ordering::Less)) + eval_compare(self, l, r, |x| x == Ordering::Less, context) } Expr::Op(BinOp::GreaterThan, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? == Ordering::Greater)) + eval_compare(self, l, r, |x| x == Ordering::Greater, context) } Expr::Op(BinOp::LowerOrEqual, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? != Ordering::Greater)) + eval_compare(self, l, r, |x| x != Ordering::Greater, context) } Expr::Op(BinOp::GreaterOrEqual, ref l, ref r) => { - let l = l.eval(context)?; - let r = r.eval(context)?; - Ok(Value::new(t!(l.compare(&r, 0), self)? != Ordering::Less)) + eval_compare(self, l, r, |x| x != Ordering::Less, context) } Expr::Op(BinOp::In, ref l, ref r) => { t!(r.eval(context)?.is_in(&l.eval(context)?), self) @@ -584,26 +647,10 @@ impl Evaluate for AstExpr { Ok(r) } Expr::ListComprehension(ref e, ref clauses) => { - let mut r = Vec::new(); - let mut context = context.child("list_comprehension"); - for v in eval_comprehension_clause(&mut context, e, clauses.as_slice())? { - r.push(v.clone()); - } - Ok(Value::from(r)) + eval_list_comprehension(e, clauses, context) } Expr::DictComprehension((ref k, ref v), ref clauses) => { - let mut r = LinkedHashMap::new(); - let tuple = Box::new(Spanned { - span: k.span.merge(v.span), - node: Expr::Tuple(vec![k.clone(), v.clone()]), - }); - let mut context = context.child("dict_comprehension"); - for e in eval_comprehension_clause(&mut context, &tuple, clauses.as_slice())? { - let k = t!(e.at(Value::from(0)), tuple)?; - let v = t!(e.at(Value::from(1)), tuple)?; - r.insert(k, v); - } - Ok(Value::from(r)) + eval_dict_comprehension(k, v, clauses, context) } } } @@ -622,7 +669,7 @@ impl Evaluate for AstExpr { let mut r = Vec::new(); let mut it1 = v.iter(); // TODO: the span here should probably include the rvalue - let mut it2 = t!(new_value.into_iter(), self)?; + let mut it2 = t!(new_value.iter(), self)?; for _ in 0..l { r.push(it1.next().unwrap().set(context, it2.next().unwrap())?) } @@ -721,7 +768,7 @@ impl Evaluate for AstStatement { let mut iterable = e2.eval(context)?; let mut result = Ok(Value::new(None)); iterable.freeze_for_iteration(); - for v in t!(iterable.into_iter(), span * span)? { + for v in t!(iterable.iter(), span * span)? { e1.set(context, v)?; match st.eval(context) { Err(EvalException::Break(..)) => break, @@ -745,7 +792,7 @@ impl Evaluate for AstStatement { ) => panic!("The parser returned an invalid for clause: {:?}", node), Statement::Def(ref name, ref params, ref stmts) => { let mut p = Vec::new(); - for ref x in params.iter() { + for x in params.iter() { p.push(match x.node { Parameter::Normal(ref n) => FunctionParameter::Normal(n.node.clone()), Parameter::WithDefaultValue(ref n, ref v) => { @@ -798,8 +845,8 @@ impl Evaluate for AstStatement { /// A method for consumption by def funcitons #[doc(hidden)] pub fn eval_def( - call_stack: &Vec<(String, String)>, - signature: &Vec, + call_stack: &[(String, String)], + signature: &[FunctionParameter], stmts: &AstStatement, env: Environment, args: Vec, @@ -809,19 +856,18 @@ pub fn eval_def( let mut it2 = args.iter(); for s in signature.iter() { match s { - &FunctionParameter::Normal(ref v) - | &FunctionParameter::WithDefaultValue(ref v, ..) - | &FunctionParameter::ArgsArray(ref v) - | &FunctionParameter::KWArgsDict(ref v) => { - match env.set(v, it2.next().unwrap().clone()) { - Err(x) => return Err(x.into()), - _ => (), + FunctionParameter::Normal(ref v) + | FunctionParameter::WithDefaultValue(ref v, ..) + | FunctionParameter::ArgsArray(ref v) + | FunctionParameter::KWArgsDict(ref v) => { + if let Err(x) = env.set(v, it2.next().unwrap().clone()) { + return Err(x.into()); } } } } let mut ctx = EvaluationContext { - call_stack: call_stack.clone(), + call_stack: call_stack.to_owned(), env, loader: (), map: map.clone(), diff --git a/starlark/src/eval/tests.rs b/starlark/src/eval/tests.rs index ac1f4279..ad9fb7e0 100644 --- a/starlark/src/eval/tests.rs +++ b/starlark/src/eval/tests.rs @@ -48,7 +48,7 @@ cyclic[1] = cyclic #[test] fn funcall_test() { - const F: &'static str = " + const F: &str = " def f1(): return 1 diff --git a/starlark/src/stdlib/dict.rs b/starlark/src/stdlib/dict.rs index a4a8e87d..f7eef919 100644 --- a/starlark/src/stdlib/dict.rs +++ b/starlark/src/stdlib/dict.rs @@ -17,8 +17,8 @@ use linked_hash_map::LinkedHashMap; use values::*; -pub const DICT_KEY_NOT_FOUND_ERROR_CODE: &'static str = "UF20"; -pub const POP_ON_EMPTY_DICT_ERROR_CODE: &'static str = "UF21"; +pub const DICT_KEY_NOT_FOUND_ERROR_CODE: &str = "UF20"; +pub const POP_ON_EMPTY_DICT_ERROR_CODE: &str = "UF21"; macro_rules! ok { ($e:expr) => { @@ -48,7 +48,7 @@ starlark_module! {global => /// ``` dict.clear(this) { dict::Dictionary::mutate( - &mut this, + &this, &|x: &mut LinkedHashMap| -> ValueResult { x.clear(); ok!(None) @@ -133,7 +133,7 @@ starlark_module! {global => /// # )"#).unwrap()); /// ``` dict.keys(this) { - let v : Vec = this.into_iter()?.collect(); + let v : Vec = this.iter()?.collect(); ok!(v) } @@ -172,7 +172,7 @@ starlark_module! {global => key.get_hash()?; // ensure the key is hashable let key_error = format!("Key '{}' not found in '{}'", key.to_repr(), this.to_repr()); dict::Dictionary::mutate( - &mut this, + &this, &|x: &mut LinkedHashMap| -> ValueResult { match x.remove(&key) { Some(x) => Ok(x), @@ -218,7 +218,7 @@ starlark_module! {global => /// ``` dict.popitem(this) { dict::Dictionary::mutate( - &mut this, + &this, &|x: &mut LinkedHashMap| -> ValueResult { match x.pop_front() { Some(x) => ok!(x), @@ -265,7 +265,7 @@ starlark_module! {global => key.get_hash()?; // Ensure the key is hashable let cloned_default = default.clone_for_container_value(&this); dict::Dictionary::mutate( - &mut this, + &this, &|x: &mut LinkedHashMap| -> ValueResult { if let Some(r) = x.get(&key) { return Ok(r.clone()) @@ -311,7 +311,7 @@ starlark_module! {global => dict.update(this, #pairs=None, **kwargs) { match pairs.get_type() { "NoneType" => (), - "list" => for v in pairs.into_iter()? { + "list" => for v in pairs.iter()? { if v.length()? != 2 { starlark_err!( INCORRECT_PARAMETER_TYPE_ERROR_CODE, @@ -324,7 +324,7 @@ starlark_module! {global => } this.set_at(v.at(Value::new(0))?, v.at(Value::new(1))?)?; }, - "dict" => for k in pairs.into_iter()? { + "dict" => for k in pairs.iter()? { this.set_at(k.clone(), pairs.at(k)?)? }, x => starlark_err!( @@ -340,7 +340,7 @@ starlark_module! {global => ) } - for k in kwargs.into_iter()? { + for k in kwargs.iter()? { this.set_at(k.clone(), kwargs.at(k)?)? } ok!(None) diff --git a/starlark/src/stdlib/list.rs b/starlark/src/stdlib/list.rs index 97a93fe8..299c97c0 100644 --- a/starlark/src/stdlib/list.rs +++ b/starlark/src/stdlib/list.rs @@ -17,8 +17,8 @@ use values::*; // Errors -- UF = User Failure -- Failure that should be expected by the user (e.g. from a fail()). -pub const LIST_INDEX_FAILED_ERROR_CODE: &'static str = "UF10"; -pub const LIST_REMOVE_ELEMENT_NOT_FOUND_ERROR_CODE: &'static str = "UF11"; +pub const LIST_INDEX_FAILED_ERROR_CODE: &str = "UF10"; +pub const LIST_REMOVE_ELEMENT_NOT_FOUND_ERROR_CODE: &str = "UF11"; macro_rules! ok { ($e:expr) => { @@ -109,7 +109,7 @@ starlark_module! {global => /// ``` list.extend(this, #other) { let this_cloned = this.clone(); - let other_cloned: Result, _> = other.into_iter()?.map(|v| v.clone_for_container_value(&this_cloned)).collect(); + let other_cloned: Result, _> = other.iter()?.map(|v| v.clone_for_container_value(&this_cloned)).collect(); list::List::mutate(&this, &|x| { x.extend(other_cloned.clone()?); ok!(None) @@ -147,7 +147,7 @@ starlark_module! {global => /// ``` list.index(this, #needle, #start = 0, #end = None) { convert_indices!(this, start, end); - let mut it = this.into_iter()?.skip(start).take(end - start); + let mut it = this.iter()?.skip(start).take(end - start); if let Some(offset) = it.position(|x| x == needle) { ok!((offset + start) as i64) } else { @@ -260,7 +260,7 @@ starlark_module! {global => /// ``` list.remove(this, #needle) { let for_it = this.clone(); - let mut it = for_it.into_iter()?; + let mut it = for_it.iter()?; if let Some(offset) = it.position(|x| x == needle) { list::List::mutate(&this, &|x| { x.remove(offset); diff --git a/starlark/src/stdlib/macros.rs b/starlark/src/stdlib/macros.rs index c53a3abe..22b55e4a 100644 --- a/starlark/src/stdlib/macros.rs +++ b/starlark/src/stdlib/macros.rs @@ -151,7 +151,7 @@ macro_rules! starlark_fun { ($(#[$attr:meta])* $fn:ident ( $($signature:tt)* ) { $($content:tt)* }) => { $(#[$attr])* pub fn $fn( - __call_stack: &Vec<(String, String)>, + __call_stack: &[(String, String)], __env: $crate::environment::Environment, args: Vec<$crate::values::Value> ) -> $crate::values::ValueResult { @@ -163,7 +163,7 @@ macro_rules! starlark_fun { ($(#[$attr:meta])* $fn:ident ( $($signature:tt)* ) { $($content:tt)* } $($rest:tt)*) => { $(#[$attr])* pub fn $fn( - __call_stack: &Vec<(String, String)>, + __call_stack: &[(String, String)], __env: $crate::environment::Environment, args: Vec<$crate::values::Value> ) -> $crate::values::ValueResult { @@ -178,7 +178,7 @@ macro_rules! starlark_fun { ($(#[$attr:meta])* $ty:ident . $fn:ident ( $($signature:tt)* ) { $($content:tt)* }) => { $(#[$attr])* pub fn $fn( - __call_stack: &Vec<(String, String)>, + __call_stack: &[(String, String)], __env: $crate::environment::Environment, args: Vec<$crate::values::Value> ) -> $crate::values::ValueResult { @@ -191,7 +191,7 @@ macro_rules! starlark_fun { $($rest:tt)*) => { $(#[$attr])* pub fn $fn( - __call_stack: &Vec<(String, String)>, + __call_stack: &[(String, String)], __env: $crate::environment::Environment, args: Vec<$crate::values::Value> ) -> $crate::values::ValueResult { diff --git a/starlark/src/stdlib/mod.rs b/starlark/src/stdlib/mod.rs index 41b02888..9ca77c88 100644 --- a/starlark/src/stdlib/mod.rs +++ b/starlark/src/stdlib/mod.rs @@ -27,14 +27,14 @@ use values::dict::Dictionary; use values::*; // Errors -- CR = Critical Runtime -const CHR_NOT_UTF8_CODEPOINT_ERROR_CODE: &'static str = "CR00"; -const DICT_ITERABLE_NOT_PAIRS_ERROR_CODE: &'static str = "CR01"; -const ATTR_NAME_NOT_STRING_ERROR_CODE: &'static str = "CR02"; -const INT_CONVERSION_FAILED_ERROR_CODE: &'static str = "CR03"; -const ORD_EXPECT_ONE_CHAR_ERROR_CODE: &'static str = "CR04"; -const EMPTY_ITERABLE_ERROR_CODE: &'static str = "CR05"; -const NUL_RANGE_STEP_ERROR_CODE: &'static str = "CR06"; -const USER_FAILURE_ERROR_CODE: &'static str = "CR99"; +const CHR_NOT_UTF8_CODEPOINT_ERROR_CODE: &str = "CR00"; +const DICT_ITERABLE_NOT_PAIRS_ERROR_CODE: &str = "CR01"; +const ATTR_NAME_NOT_STRING_ERROR_CODE: &str = "CR02"; +const INT_CONVERSION_FAILED_ERROR_CODE: &str = "CR03"; +const ORD_EXPECT_ONE_CHAR_ERROR_CODE: &str = "CR04"; +const EMPTY_ITERABLE_ERROR_CODE: &str = "CR05"; +const NUL_RANGE_STEP_ERROR_CODE: &str = "CR06"; +const USER_FAILURE_ERROR_CODE: &str = "CR99"; #[macro_use] pub mod macros; @@ -55,7 +55,7 @@ starlark_module! {global_functions => format!( "fail(): {}{}", msg.to_str(), - st.into_iter().rev().fold(String::new(), |a,s| format!("{}\n {}", a, s.1)) + st.iter().rev().fold(String::new(), |a,s| format!("{}\n {}", a, s.1)) ), msg.to_str() ) @@ -86,7 +86,7 @@ starlark_module! {global_functions => /// # )").unwrap()); /// ``` any(#x) { - for i in x.into_iter()? { + for i in x.iter()? { if i.to_bool() { return Ok(Value::new(true)); } @@ -119,7 +119,7 @@ starlark_module! {global_functions => /// # )").unwrap()); /// ``` all(x) { - for i in x.into_iter()? { + for i in x.iter()? { if !i.to_bool() { return Ok(Value::new(false)); } @@ -233,14 +233,14 @@ starlark_module! {global_functions => match a.get_type() { "NoneType" => (), "dict" => { - for k in a.into_iter()? { + for k in a.iter()? { let v = a.at(k.clone())?; map.set_at(k, v)?; } }, _ => { - for el in a.into_iter()? { - match el.into_iter() { + for el in a.iter()? { + match el.iter() { Ok(mut it) => { let first = it.next(); let second = it.next(); @@ -269,7 +269,7 @@ starlark_module! {global_functions => } } } - for el in kwargs.into_iter()? { + for el in kwargs.iter()? { map.set_at(el.clone(), kwargs.at(el)?)?; } Ok(map) @@ -291,10 +291,11 @@ starlark_module! {global_functions => /// # ))).unwrap()); /// ``` dir(env env, x) { - match x.dir_attr() { - Ok(v) => Ok(Value::from(env.list_type_value(&x).extend(v))), - _ => Ok(Value::from(env.list_type_value(&x))), + let mut result = env.list_type_value(&x); + if let Ok(v) = x.dir_attr() { + result.extend(v); } + Ok(Value::from(result)) } @@ -322,7 +323,7 @@ starlark_module! {global_functions => let v2 = offset.to_int()?; let v : Vec = it - .into_iter()? + .iter()? .enumerate() .map(|(k, v)| Value::from((Value::new(k as i64 + v2), v))) .collect(); @@ -469,12 +470,10 @@ starlark_module! {global_functions => } else { s }, 8 => if s.starts_with("0o") || s.starts_with("0O") { s.get(2..).unwrap().to_string() + } else if s.starts_with('0') { + s.get(1..).unwrap().to_string() } else { - if s.starts_with("0") { - s.get(1..).unwrap().to_string() - } else { - s - } + s }, 2 => if s.starts_with("0b") || s.starts_with("0B") { s.get(2..).unwrap().to_string() @@ -528,7 +527,7 @@ starlark_module! {global_functions => list(#a = None) { let mut l = Vec::new(); if a.get_type() != "NoneType" { - for x in a.into_iter()? { + for x in a.iter()? { l.push(x.clone()) } } @@ -563,7 +562,7 @@ starlark_module! {global_functions => } else { args }; - let mut it = args.into_iter()?; + let mut it = args.iter()?; let mut max = match it.next() { Some(x) => x, None => starlark_err!( @@ -616,7 +615,7 @@ starlark_module! {global_functions => } else { args }; - let mut it = args.into_iter()?; + let mut it = args.iter()?; let mut min = match it.next() { Some(x) => x, None => starlark_err!( @@ -679,7 +678,7 @@ starlark_module! {global_functions => "Not a one character string".to_owned() ) } else { - Ok(Value::new(u32::from(a.to_string().chars().next().unwrap()) as i64)) + Ok(Value::new(i64::from(u32::from(a.to_string().chars().next().unwrap())))) } } @@ -781,7 +780,7 @@ starlark_module! {global_functions => /// # )"#).unwrap()); /// ``` reversed(#a) { - let v : Vec = a.into_iter()?.collect(); + let v : Vec = a.iter()?.collect(); let v : Vec = v.into_iter().rev().collect(); Ok(Value::from(v)) } @@ -814,7 +813,7 @@ starlark_module! {global_functions => /// # )"#).unwrap()); /// ``` sorted(call_stack cs, env e, #x, key = None, reverse = false) { - let x = x.into_iter()?; + let x = x.iter()?; let mut it : Vec<(Value, Value)> = if key.get_type() == "NoneType" { x.map(|x| (x.clone(), x)).collect() } else { @@ -870,7 +869,7 @@ starlark_module! {global_functions => tuple(#a = None) { let mut l = Vec::new(); if a.get_type() != "NoneType" { - for x in a.into_iter()? { + for x in a.iter()? { l.push(x.clone()) } } @@ -917,18 +916,16 @@ starlark_module! {global_functions => zip(*args) { let mut v = Vec::new(); - for arg in args.into_iter()? { + for arg in args.iter()? { let first = v.is_empty(); let mut idx = 0; - for e in arg.into_iter()? { + for e in arg.iter()? { if first { v.push(Value::from((e.clone(),))); idx += 1; - } else { - if idx < v.len() { - v[idx] = v[idx].add(Value::from((e.clone(),)))?; - idx += 1; - } + } else if idx < v.len() { + v[idx] = v[idx].add(Value::from((e.clone(),)))?; + idx += 1; } } v.truncate(idx); diff --git a/starlark/src/stdlib/string.rs b/starlark/src/stdlib/string.rs index d86e2311..ae2356bb 100644 --- a/starlark/src/stdlib/string.rs +++ b/starlark/src/stdlib/string.rs @@ -18,11 +18,11 @@ use std::str::FromStr; use values::*; // Errors -- UF = User Failure -- Failure that should be expected by the user (e.g. from a fail()). -pub const SUBSTRING_INDEX_FAILED_ERROR_CODE: &'static str = "UF00"; -pub const FORMAT_STRING_UNMATCHED_BRACKET_ERROR_CODE: &'static str = "UF01"; -pub const FORMAT_STRING_ORDER_INDEX_MIX_ERROR_CODE: &'static str = "UF02"; -pub const FORMAT_STRING_INVALID_SPECIFIER_ERROR_CODE: &'static str = "UF03"; -pub const FORMAT_STRING_INVALID_CHARACTER_ERROR_CODE: &'static str = "UF04"; +pub const SUBSTRING_INDEX_FAILED_ERROR_CODE: &str = "UF00"; +pub const FORMAT_STRING_UNMATCHED_BRACKET_ERROR_CODE: &str = "UF01"; +pub const FORMAT_STRING_ORDER_INDEX_MIX_ERROR_CODE: &str = "UF02"; +pub const FORMAT_STRING_INVALID_SPECIFIER_ERROR_CODE: &str = "UF03"; +pub const FORMAT_STRING_INVALID_CHARACTER_ERROR_CODE: &str = "UF04"; macro_rules! ok { ($e:expr) => { @@ -91,35 +91,33 @@ fn format_capture>( ) } } - } else { - if n.chars().all(|c| c.is_ascii_digit()) { - if *captured_by_order { - starlark_err!( - FORMAT_STRING_ORDER_INDEX_MIX_ERROR_CODE, - concat!( - "Cannot mix manual field specification and ", - "automatic field numbering in format string" - ) - .to_owned(), - "Mixed manual and automatic field numbering".to_owned() + } else if n.chars().all(|c| c.is_ascii_digit()) { + if *captured_by_order { + starlark_err!( + FORMAT_STRING_ORDER_INDEX_MIX_ERROR_CODE, + concat!( + "Cannot mix manual field specification and ", + "automatic field numbering in format string" ) - } else { - *captured_by_index = true; - return Ok(conv(args.at(Value::from(i64::from_str(n).unwrap()))?)); - } + .to_owned(), + "Mixed manual and automatic field numbering".to_owned() + ) } else { - if let Some(x) = n.chars().find(|c| match c { - &'.' | &',' | &'[' | &']' => true, - _ => false, - }) { - starlark_err!( - FORMAT_STRING_INVALID_CHARACTER_ERROR_CODE, - format!("Invalid character '{}' inside replacement field", x), - format!("Invalid character '{}'", x) - ) - } - return Ok(conv(kwargs.at(Value::from(n))?)); + *captured_by_index = true; + Ok(conv(args.at(Value::from(i64::from_str(n).unwrap()))?)) + } + } else { + if let Some(x) = n.chars().find(|c| match c { + '.' | ',' | '[' | ']' => true, + _ => false, + }) { + starlark_err!( + FORMAT_STRING_INVALID_CHARACTER_ERROR_CODE, + format!("Invalid character '{}' inside replacement field", x), + format!("Invalid character '{}'", x) + ) } + Ok(conv(kwargs.at(Value::from(n))?)) } } @@ -133,18 +131,16 @@ fn splitn_whitespace(s: &str, maxsplit: usize) -> Vec { for c in s.chars() { if split >= maxsplit && !eat_ws { cur.push(c) - } else { - if c.is_whitespace() { - if !cur.is_empty() { - v.push(cur); - cur = String::new(); - split += 1; - eat_ws = true; - } - } else { - eat_ws = false; - cur.push(c) + } else if c.is_whitespace() { + if !cur.is_empty() { + v.push(cur); + cur = String::new(); + split += 1; + eat_ws = true; } + } else { + eat_ws = false; + cur.push(c) } } if !cur.is_empty() { @@ -161,18 +157,16 @@ fn rsplitn_whitespace(s: &str, maxsplit: usize) -> Vec { for c in s.chars().rev() { if split >= maxsplit && !eat_ws { cur.push(c) - } else { - if c.is_whitespace() { - if !cur.is_empty() { - v.push(cur.chars().rev().collect()); - cur = String::new(); - split += 1; - eat_ws = true; - } - } else { - eat_ws = false; - cur.push(c) + } else if c.is_whitespace() { + if !cur.is_empty() { + v.push(cur.chars().rev().collect()); + cur = String::new(); + split += 1; + eat_ws = true; } + } else { + eat_ws = false; + cur.push(c) } } if !cur.is_empty() { @@ -269,7 +263,7 @@ starlark_module! {global => /// ``` string.codepoints(this) { // Note that we return a list here... Which is not equivalent to the go implementation. - let v : Vec = this.to_str().chars().map(|x| u32::from(x) as i64).collect(); + let v : Vec = this.to_str().chars().map(|x| i64::from(u32::from(x))).collect(); ok!(v) } @@ -436,7 +430,7 @@ starlark_module! {global => /// # )"#).unwrap()); /// ``` string.format(this, *args, **kwargs) { - let mut it = args.into_iter()?; + let mut it = args.iter()?; let this = this.to_str(); let mut captured_by_index = false; let mut captured_by_order = false; @@ -735,10 +729,8 @@ starlark_module! {global => if c.is_lowercase() { ok!(false); } - } else { - if c.is_uppercase() { - ok!(false); - } + } else if c.is_uppercase() { + ok!(false); } if c.is_alphabetic() { result = true; @@ -805,7 +797,7 @@ starlark_module! {global => string.join(this, #to_join) { let this = this.to_str(); ok!( - to_join.into_iter()?.fold( + to_join.iter()?.fold( Ok(String::new()), |a, x| { check_string!(x, join); @@ -1383,7 +1375,7 @@ mod tests { fn test_format_capture() { let args = Value::from(vec!["1", "2", "3"]); let mut kwargs = dict::Dictionary::new(); - let mut it = args.into_iter().unwrap(); + let mut it = args.iter().unwrap(); let mut captured_by_index = false; let mut captured_by_order = false; @@ -1460,7 +1452,7 @@ mod tests { ) .is_err()); captured_by_order = false; - it = args.into_iter().unwrap(); + it = args.iter().unwrap(); assert_eq!( format_capture( "{1", diff --git a/starlark/src/syntax/ast.rs b/starlark/src/syntax/ast.rs index 9d68c1a7..69732663 100644 --- a/starlark/src/syntax/ast.rs +++ b/starlark/src/syntax/ast.rs @@ -18,6 +18,7 @@ use super::lexer; use codemap::{Span, Spanned}; use std::collections::HashSet; use std::fmt; +use std::fmt::{Display, Formatter}; extern crate lalrpop_util; // Boxed types used for storing information from the parsing will be used especially for the @@ -270,7 +271,7 @@ impl Statement { { let mut stage = 0; let mut argset = HashSet::new(); - for ref arg in parameters.iter() { + for arg in parameters.iter() { match arg.node { Parameter::Normal(ref n) => { if stage > 0 { @@ -334,139 +335,138 @@ impl Statement { } } -impl fmt::Display for BinOp { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for BinOp { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { - BinOp::Or => write!(f, " or "), - BinOp::And => write!(f, " and "), - BinOp::EqualsTo => write!(f, " == "), - BinOp::Different => write!(f, " != "), - BinOp::LowerThan => write!(f, " < "), - BinOp::GreaterThan => write!(f, " > "), - BinOp::LowerOrEqual => write!(f, " <= "), - BinOp::GreaterOrEqual => write!(f, " >= "), - BinOp::In => write!(f, " in "), - BinOp::NotIn => write!(f, " not in "), - BinOp::Substraction => write!(f, " - "), - BinOp::Addition => write!(f, " + "), - BinOp::Multiplication => write!(f, " * "), - BinOp::Percent => write!(f, " % "), - BinOp::Division => write!(f, " / "), - BinOp::FloorDivision => write!(f, " // "), - BinOp::Pipe => write!(f, " | "), + BinOp::Or => f.write_str(" or "), + BinOp::And => f.write_str(" and "), + BinOp::EqualsTo => f.write_str(" == "), + BinOp::Different => f.write_str(" != "), + BinOp::LowerThan => f.write_str(" < "), + BinOp::GreaterThan => f.write_str(" > "), + BinOp::LowerOrEqual => f.write_str(" <= "), + BinOp::GreaterOrEqual => f.write_str(" >= "), + BinOp::In => f.write_str(" in "), + BinOp::NotIn => f.write_str(" not in "), + BinOp::Substraction => f.write_str(" - "), + BinOp::Addition => f.write_str(" + "), + BinOp::Multiplication => f.write_str(" * "), + BinOp::Percent => f.write_str(" % "), + BinOp::Division => f.write_str(" / "), + BinOp::FloorDivision => f.write_str(" // "), + BinOp::Pipe => f.write_str(" | "), } } } -impl fmt::Display for AssignOp { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for AssignOp { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { - AssignOp::Assign => write!(f, " = "), - AssignOp::Increment => write!(f, " += "), - AssignOp::Decrement => write!(f, " += "), - AssignOp::Multiplier => write!(f, " *= "), - AssignOp::Divider => write!(f, " /= "), - AssignOp::FloorDivider => write!(f, " //= "), - AssignOp::Percent => write!(f, " %= "), + AssignOp::Assign => f.write_str(" = "), + AssignOp::Increment => f.write_str(" += "), + AssignOp::Decrement => f.write_str(" += "), + AssignOp::Multiplier => f.write_str(" *= "), + AssignOp::Divider => f.write_str(" /= "), + AssignOp::FloorDivider => f.write_str(" //= "), + AssignOp::Percent => f.write_str(" %= "), } } } fn comma_separated_fmt( - f: &mut fmt::Formatter, - v: &Vec, + f: &mut Formatter, + v: &[I], converter: F, for_tuple: bool, ) -> fmt::Result where - F: Fn(&I, &mut fmt::Formatter) -> fmt::Result, + F: Fn(&I, &mut Formatter) -> fmt::Result, { for (i, e) in v.iter().enumerate() { - let sep = if i == 0 { "" } else { ", " }; - write!(f, "{}", sep)?; + f.write_str(if i == 0 { "" } else { ", " })?; converter(e, f)?; } if v.len() == 1 && for_tuple { - write!(f, ",")?; + f.write_str(",")?; } Ok(()) } -fn fmt_string_literal(f: &mut fmt::Formatter, s: &str) -> fmt::Result { - write!(f, "\"")?; +fn fmt_string_literal(f: &mut Formatter, s: &str) -> fmt::Result { + f.write_str("\"")?; for c in s.chars() { match c { - '\n' => write!(f, "\\n")?, - '\t' => write!(f, "\\t")?, - '\r' => write!(f, "\\r")?, - '\0' => write!(f, "\\0")?, - '"' => write!(f, "\\\"")?, - '\\' => write!(f, "\\\\")?, - x => write!(f, "{}", x)?, + '\n' => f.write_str("\\n")?, + '\t' => f.write_str("\\t")?, + '\r' => f.write_str("\\r")?, + '\0' => f.write_str("\\0")?, + '"' => f.write_str("\\\"")?, + '\\' => f.write_str("\\\\")?, + x => f.write_str(&x.to_string())?, } } - write!(f, "\"") + f.write_str("\"") } -impl fmt::Display for Expr { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for Expr { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { Expr::Tuple(ref e) => { - write!(f, "(")?; - comma_separated_fmt(f, e, |x, f| write!(f, "{}", x.node), true)?; - write!(f, ")") + f.write_str("(")?; + comma_separated_fmt(f, e, |x, f| x.node.fmt(f), true)?; + f.write_str(")") } Expr::Dot(ref e, ref s) => write!(f, "{}.{}", e.node, s.node), Expr::Call(ref e, ref pos, ref named, ref args, ref kwargs) => { write!(f, "{}(", e.node)?; let mut first = true; - for ref a in pos { + for a in pos { if !first { - write!(f, ", ")?; + f.write_str(", ")?; } first = false; - write!(f, "{}", a.node)?; + a.node.fmt(f)?; } for &(ref k, ref v) in named { if !first { - write!(f, ", ")?; + f.write_str(", ")?; } first = false; write!(f, "{} = {}", k.node, v.node)?; } - if let &Some(ref x) = args { + if let Some(ref x) = args { if !first { - write!(f, ", ")?; + f.write_str(", ")?; } first = false; write!(f, "*{}", x.node)?; } - if let &Some(ref x) = kwargs { + if let Some(ref x) = kwargs { if !first { - write!(f, ", ")?; + f.write_str(", ")?; } write!(f, "**{}", x.node)?; } - write!(f, ")") + f.write_str(")") } Expr::ArrayIndirection(ref e, ref i) => write!(f, "{}[{}]", e.node, i.node), Expr::Slice(ref e, ref i1, ref i2, ref i3) => { write!(f, "{}[]", e.node)?; - if let &Some(ref x) = i1 { + if let Some(ref x) = i1 { write!(f, "{}:", x.node)? } else { - write!(f, ":")? + f.write_str(":")? } - if let &Some(ref x) = i2 { - write!(f, "{}", x.node)? + if let Some(ref x) = i2 { + x.node.fmt(f)? } - if let &Some(ref x) = i3 { + if let Some(ref x) = i3 { write!(f, ":{}", x.node)? } Ok(()) } - Expr::Identifier(ref s) => write!(f, "{}", s.node), - Expr::IntLiteral(ref i) => write!(f, "{}", i.node), + Expr::Identifier(ref s) => s.node.fmt(f), + Expr::IntLiteral(ref i) => i.node.fmt(f), Expr::Not(ref e) => write!(f, "(not {})", e.node), Expr::Minus(ref e) => write!(f, "-{}", e.node), Expr::Plus(ref e) => write!(f, "+{}", e.node), @@ -475,34 +475,34 @@ impl fmt::Display for Expr { write!(f, "({} if {} else {})", v1.node, cond.node, v2.node) } Expr::List(ref v) => { - write!(f, "[")?; - comma_separated_fmt(f, v, |x, f| write!(f, "{}", x.node), false)?; - write!(f, "]") + f.write_str("[")?; + comma_separated_fmt(f, v, |x, f| x.node.fmt(f), false)?; + f.write_str("]") } Expr::Dict(ref v) => { - write!(f, "{{")?; + f.write_str("{")?; comma_separated_fmt(f, v, |x, f| write!(f, "{}: {}", x.0.node, x.1.node), false)?; - write!(f, "}}") + f.write_str("}") } Expr::ListComprehension(ref e, ref v) => { write!(f, "[{}", e.node)?; - comma_separated_fmt(f, v, |x, f| write!(f, "{}", x.node), false)?; - write!(f, "]") + comma_separated_fmt(f, v, |x, f| x.node.fmt(f), false)?; + f.write_str("]") } Expr::DictComprehension((ref k, ref v), ref c) => { write!(f, "{{{}: {}", k.node, v.node)?; - comma_separated_fmt(f, c, |x, f| write!(f, "{}", x.node), false)?; - write!(f, "}}") + comma_separated_fmt(f, c, |x, f| x.node.fmt(f), false)?; + f.write_str("}}") } Expr::StringLiteral(ref s) => fmt_string_literal(f, &s.node), } } } -impl fmt::Display for Argument { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for Argument { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { - Argument::Positional(ref s) => write!(f, "{}", s.node), + Argument::Positional(ref s) => s.node.fmt(f), Argument::Named(ref s, ref e) => write!(f, "{} = {}", s.node, e.node), Argument::ArgsArray(ref s) => write!(f, "*{}", s.node), Argument::KWArgsDict(ref s) => write!(f, "**{}", s.node), @@ -510,10 +510,10 @@ impl fmt::Display for Argument { } } -impl fmt::Display for Parameter { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for Parameter { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { - Parameter::Normal(ref s) => write!(f, "{}", s.node), + Parameter::Normal(ref s) => s.node.fmt(f), Parameter::WithDefaultValue(ref s, ref e) => write!(f, "{} = {}", s.node, e.node), Parameter::Args(ref s) => write!(f, "*{}", s.node), Parameter::KWArgs(ref s) => write!(f, "**{}", s.node), @@ -521,8 +521,8 @@ impl fmt::Display for Parameter { } } -impl fmt::Display for Clause { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for Clause { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { Clause::For(ref t, ref e) => write!(f, " for {} in {}", t.node, e.node), Clause::If(ref t) => write!(f, " if {}", t.node), @@ -531,16 +531,16 @@ impl fmt::Display for Clause { } impl Statement { - fn fmt_with_tab(&self, f: &mut fmt::Formatter, tab: String) -> fmt::Result { + fn fmt_with_tab(&self, f: &mut Formatter, tab: String) -> fmt::Result { match *self { - Statement::Break => write!(f, "{}break\n", tab), - Statement::Continue => write!(f, "{}continue\n", tab), - Statement::Pass => write!(f, "{}pass\n", tab), - Statement::Return(Some(ref e)) => write!(f, "{}return {}\n", tab, e.node), - Statement::Return(None) => write!(f, "{}return\n", tab), - Statement::Expression(ref e) => write!(f, "{}{}\n", tab, e.node), + Statement::Break => writeln!(f, "{}break", tab), + Statement::Continue => writeln!(f, "{}continue", tab), + Statement::Pass => writeln!(f, "{}pass", tab), + Statement::Return(Some(ref e)) => writeln!(f, "{}return {}", tab, e.node), + Statement::Return(None) => writeln!(f, "{}return", tab), + Statement::Expression(ref e) => writeln!(f, "{}{}", tab, e.node), Statement::Assign(ref l, ref op, ref r) => { - write!(f, "{}{}{}{}\n", tab, l.node, op, r.node) + writeln!(f, "{}{}{}{}", tab, l.node, op, r.node) } Statement::Statements(ref v) => { for s in v { @@ -549,23 +549,23 @@ impl Statement { Ok(()) } Statement::If(ref cond, ref suite) => { - write!(f, "{}if {}:\n", tab, cond.node)?; + writeln!(f, "{}if {}:", tab, cond.node)?; suite.node.fmt_with_tab(f, tab + " ") } Statement::IfElse(ref cond, ref suite1, ref suite2) => { - write!(f, "{}if {}:\n", tab, cond.node)?; + writeln!(f, "{}if {}:", tab, cond.node)?; suite1.node.fmt_with_tab(f, tab.clone() + " ")?; - write!(f, "{}else:\n", tab)?; + writeln!(f, "{}else:", tab)?; suite2.node.fmt_with_tab(f, tab + " ") } Statement::For(ref clause, ref suite) => { - write!(f, "{}for {}:\n", tab, clause.node)?; + writeln!(f, "{}for {}:", tab, clause.node)?; suite.node.fmt_with_tab(f, tab + " ") } Statement::Def(ref name, ref params, ref suite) => { write!(f, "{}def {}(", tab, name.node)?; - comma_separated_fmt(f, params, |x, f| write!(f, "{}", x.node), false)?; - write!(f, "):\n")?; + comma_separated_fmt(f, params, |x, f| x.node.fmt(f), false)?; + f.write_str("):\n")?; suite.node.fmt_with_tab(f, tab + " ") } Statement::Load(ref filename, ref v) => { @@ -580,14 +580,14 @@ impl Statement { }, false, )?; - write!(f, ")\n") + f.write_str(")\n") } } } } -impl fmt::Display for Statement { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl Display for Statement { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { self.fmt_with_tab(f, "".to_owned()) } } diff --git a/starlark/src/syntax/grammar.tests.rs b/starlark/src/syntax/grammar.tests.rs index b5be29ff..77c3fa60 100644 --- a/starlark/src/syntax/grammar.tests.rs +++ b/starlark/src/syntax/grammar.tests.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::StarlarkParser; +use syntax::grammar::StarlarkParser; use std::sync::{Arc, Mutex}; use syntax::ast::Statement; use syntax::dialect::Dialect; diff --git a/starlark/src/syntax/lexer.rs b/starlark/src/syntax/lexer.rs index 9e2832a3..2f0cbf6a 100644 --- a/starlark/src/syntax/lexer.rs +++ b/starlark/src/syntax/lexer.rs @@ -25,10 +25,10 @@ use std::str::CharIndices; // TODO: move that code in some common error code list? // CL prefix = Critical Lexing -const LEX_ERROR_CODE: &'static str = "CL00"; -const INDENT_ERROR_CODE: &'static str = "CL01"; -const UNFINISHED_STRING_LITERAL_CODE: &'static str = "CL02"; -const INVALID_ESCAPE_SEQUENCE_CODE: &'static str = "CL03"; +const LEX_ERROR_CODE: &str = "CL00"; +const INDENT_ERROR_CODE: &str = "CL01"; +const UNFINISHED_STRING_LITERAL_CODE: &str = "CL02"; +const INVALID_ESCAPE_SEQUENCE_CODE: &str = "CL03"; /// Errors that can be generated during lexing #[doc(hidden)] @@ -156,59 +156,59 @@ pub enum Token { impl fmt::Display for Token { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - &Token::Indent => write!(f, "new indentation block"), - &Token::Dedent => write!(f, "end of indentation block"), - &Token::Newline => write!(f, "new line"), - &Token::And => write!(f, "keyword 'and'"), - &Token::Else => write!(f, "keyword 'else'"), - &Token::Load => write!(f, "keyword 'load'"), - &Token::Break => write!(f, "keyword 'break'"), - &Token::For => write!(f, "keyword 'for'"), - &Token::Not => write!(f, "keyword 'not'"), - &Token::NotIn => write!(f, "keyword 'not in'"), - &Token::Continue => write!(f, "keyword 'continue'"), - &Token::If => write!(f, "keyword 'if'"), - &Token::Or => write!(f, "keyword 'or'"), - &Token::Def => write!(f, "keyword 'def'"), - &Token::In => write!(f, "keyword 'in'"), - &Token::Pass => write!(f, "keyword 'pass'"), - &Token::Elif => write!(f, "keyword 'elif'"), - &Token::Return => write!(f, "keyword 'return'"), - &Token::Comma => write!(f, "symbol ','"), - &Token::Semicolon => write!(f, "symbol ';'"), - &Token::Colon => write!(f, "symbol ':'"), - &Token::PlusEqual => write!(f, "symbol '+='"), - &Token::MinusEqual => write!(f, "symbol '-='"), - &Token::StarEqual => write!(f, "symbol '*='"), - &Token::SlashEqual => write!(f, "symbol '/='"), - &Token::DoubleSlashEqual => write!(f, "symbol '//='"), - &Token::PercentEqual => write!(f, "symbol '%='"), - &Token::DoubleEqual => write!(f, "symbol '=='"), - &Token::BangEqual => write!(f, "symbol '!='"), - &Token::LowerEqual => write!(f, "symbol '<='"), - &Token::GreaterEqual => write!(f, "symbol '>='"), - &Token::Doublestar => write!(f, "symbol '**'"), - &Token::Equal => write!(f, "symbol '='"), - &Token::LowerThan => write!(f, "symbol '<'"), - &Token::GreaterThan => write!(f, "symbol '>'"), - &Token::Minus => write!(f, "symbol '-'"), - &Token::Plus => write!(f, "symbol '+'"), - &Token::Star => write!(f, "symbol '*'"), - &Token::Percent => write!(f, "symbol '%'"), - &Token::Slash => write!(f, "symbol '/'"), - &Token::DoubleSlash => write!(f, "symbol '//'"), - &Token::Dot => write!(f, "symbol '.'"), - &Token::Pipe => write!(f, "symbol '|'"), - &Token::OpeningBracket => write!(f, "symbol '['"), - &Token::OpeningCurlyBracket => write!(f, "symbol '{{'"), - &Token::OpeningParenthesis => write!(f, "symbol '('"), - &Token::ClosingBracket => write!(f, "symbol ']'"), - &Token::ClosingCurlyBracket => write!(f, "symbol '}}'"), - &Token::ClosingParenthesis => write!(f, "symbol ')'"), - &Token::Reserved(ref s) => write!(f, "reserved keyword '{}'", s), - &Token::Identifier(ref s) => write!(f, "identifier '{}'", s), - &Token::IntegerLiteral(ref i) => write!(f, "integer literal '{}'", i), - &Token::StringLiteral(ref s) => write!(f, "string literal '{}'", s), + Token::Indent => write!(f, "new indentation block"), + Token::Dedent => write!(f, "end of indentation block"), + Token::Newline => write!(f, "new line"), + Token::And => write!(f, "keyword 'and'"), + Token::Else => write!(f, "keyword 'else'"), + Token::Load => write!(f, "keyword 'load'"), + Token::Break => write!(f, "keyword 'break'"), + Token::For => write!(f, "keyword 'for'"), + Token::Not => write!(f, "keyword 'not'"), + Token::NotIn => write!(f, "keyword 'not in'"), + Token::Continue => write!(f, "keyword 'continue'"), + Token::If => write!(f, "keyword 'if'"), + Token::Or => write!(f, "keyword 'or'"), + Token::Def => write!(f, "keyword 'def'"), + Token::In => write!(f, "keyword 'in'"), + Token::Pass => write!(f, "keyword 'pass'"), + Token::Elif => write!(f, "keyword 'elif'"), + Token::Return => write!(f, "keyword 'return'"), + Token::Comma => write!(f, "symbol ','"), + Token::Semicolon => write!(f, "symbol ';'"), + Token::Colon => write!(f, "symbol ':'"), + Token::PlusEqual => write!(f, "symbol '+='"), + Token::MinusEqual => write!(f, "symbol '-='"), + Token::StarEqual => write!(f, "symbol '*='"), + Token::SlashEqual => write!(f, "symbol '/='"), + Token::DoubleSlashEqual => write!(f, "symbol '//='"), + Token::PercentEqual => write!(f, "symbol '%='"), + Token::DoubleEqual => write!(f, "symbol '=='"), + Token::BangEqual => write!(f, "symbol '!='"), + Token::LowerEqual => write!(f, "symbol '<='"), + Token::GreaterEqual => write!(f, "symbol '>='"), + Token::Doublestar => write!(f, "symbol '**'"), + Token::Equal => write!(f, "symbol '='"), + Token::LowerThan => write!(f, "symbol '<'"), + Token::GreaterThan => write!(f, "symbol '>'"), + Token::Minus => write!(f, "symbol '-'"), + Token::Plus => write!(f, "symbol '+'"), + Token::Star => write!(f, "symbol '*'"), + Token::Percent => write!(f, "symbol '%'"), + Token::Slash => write!(f, "symbol '/'"), + Token::DoubleSlash => write!(f, "symbol '//'"), + Token::Dot => write!(f, "symbol '.'"), + Token::Pipe => write!(f, "symbol '|'"), + Token::OpeningBracket => write!(f, "symbol '['"), + Token::OpeningCurlyBracket => write!(f, "symbol '{{'"), + Token::OpeningParenthesis => write!(f, "symbol '('"), + Token::ClosingBracket => write!(f, "symbol ']'"), + Token::ClosingCurlyBracket => write!(f, "symbol '}}'"), + Token::ClosingParenthesis => write!(f, "symbol ')'"), + Token::Reserved(ref s) => write!(f, "reserved keyword '{}'", s), + Token::Identifier(ref s) => write!(f, "identifier '{}'", s), + Token::IntegerLiteral(ref i) => write!(f, "integer literal '{}'", i), + Token::StringLiteral(ref s) => write!(f, "string literal '{}'", s), } } } @@ -331,12 +331,10 @@ impl Lexer { fn replace_input(&mut self, input: &str) { self.offset = if let Some((p, _)) = self.peek() { p + } else if let Some((i, c)) = self.last_next { + i + (c.len_utf8() as u64) } else { - if let Some((i, c)) = self.last_next { - i + (c.len_utf8() as u64) - } else { - self.last_pos - } + self.last_pos }; assert!(self.offset >= self.last_pos); self.input = input.to_owned(); @@ -418,12 +416,10 @@ impl Lexer { fn end_pos(&mut self) -> (u64, u64) { if let Some((end, ..)) = self.peek() { (self.last_pos, end) + } else if let Some((i, c)) = self.last_next { + (self.last_pos, i + (c.len_utf8() as u64)) } else { - if let Some((i, c)) = self.last_next { - (self.last_pos, i + (c.len_utf8() as u64)) - } else { - (self.last_pos, self.last_pos) - } + (self.last_pos, self.last_pos) } } @@ -653,14 +649,10 @@ impl Lexer { fn consume_int_radix(&mut self, radix: u32) -> Option<::Item> { let val = self.consume_int_r(radix); - if self.peek_char().is_alphanumeric() { + if self.peek_char().is_alphanumeric() || val.is_err() { self.invalid() } else { - if val.is_err() { - self.invalid() - } else { - self.end(Token::IntegerLiteral(val.unwrap())) - } + self.end(Token::IntegerLiteral(val.unwrap())) } } @@ -861,131 +853,129 @@ impl Lexer { self.begin(); match self.peek_char() { '\0' => None, - '\n' | '\r' | '\u{2028}' | '\u{2029}' => return self.consume_nl(), - '\'' | '"' => return self.consume_string(false), + '\n' | '\r' | '\u{2028}' | '\u{2029}' => self.consume_nl(), + '\'' | '"' => self.consume_string(false), 'r' => { self.pop(); let p = self.peek_char(); if p == '\'' || p == '"' { - return self.consume_string(true); + self.consume_string(true) } else { - return self.consume_identifier_queue("r"); + self.consume_identifier_queue("r") } } - '0'...'9' => return self.consume_int(), - '_' => return self.consume_identifier(), - c if c.is_alphabetic() => return self.consume_identifier(), - ',' => return self.consume(Token::Comma), - ';' => return self.consume(Token::Semicolon), - ':' => return self.consume(Token::Colon), + '0'...'9' => self.consume_int(), + '_' => self.consume_identifier(), + c if c.is_alphabetic() => self.consume_identifier(), + ',' => self.consume(Token::Comma), + ';' => self.consume(Token::Semicolon), + ':' => self.consume(Token::Colon), '+' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::PlusEqual) } else { self.end(Token::Plus) - }; + } } '-' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::MinusEqual) } else { self.end(Token::Minus) - }; + } } '*' => { self.pop(); match self.peek_char() { - '=' => return self.consume(Token::StarEqual), - '*' => return self.consume(Token::Doublestar), - _ => return self.end(Token::Star), + '=' => self.consume(Token::StarEqual), + '*' => self.consume(Token::Doublestar), + _ => self.end(Token::Star), } } '/' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::SlashEqual) - } else { - if self.peek_char() == '/' { - self.pop(); - if self.peek_char() == '=' { - self.consume(Token::DoubleSlashEqual) - } else { - self.end(Token::DoubleSlash) - } + } else if self.peek_char() == '/' { + self.pop(); + if self.peek_char() == '=' { + self.consume(Token::DoubleSlashEqual) } else { - self.end(Token::Slash) + self.end(Token::DoubleSlash) } - }; + } else { + self.end(Token::Slash) + } } '%' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::PercentEqual) } else { self.end(Token::Percent) - }; + } } '=' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::DoubleEqual) } else { self.end(Token::Equal) - }; + } } '!' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::BangEqual) } else { self.invalid() - }; + } } '<' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::LowerEqual) } else { self.end(Token::LowerThan) - }; + } } '>' => { self.pop(); - return if self.peek_char() == '=' { + if self.peek_char() == '=' { self.consume(Token::GreaterEqual) } else { self.end(Token::GreaterThan) - }; + } } - '|' => return self.consume(Token::Pipe), - '.' => return self.consume(Token::Dot), + '|' => self.consume(Token::Pipe), + '.' => self.consume(Token::Dot), '[' => { self.parentheses += 1; - return self.consume(Token::OpeningBracket); + self.consume(Token::OpeningBracket) } ']' => { self.parentheses -= 1; - return self.consume(Token::ClosingBracket); + self.consume(Token::ClosingBracket) } '(' => { self.parentheses += 1; - return self.consume(Token::OpeningParenthesis); + self.consume(Token::OpeningParenthesis) } ')' => { self.parentheses -= 1; - return self.consume(Token::ClosingParenthesis); + self.consume(Token::ClosingParenthesis) } '{' => { self.parentheses += 1; - return self.consume(Token::OpeningCurlyBracket); + self.consume(Token::OpeningCurlyBracket) } '}' => { self.parentheses -= 1; - return self.consume(Token::ClosingCurlyBracket); + self.consume(Token::ClosingCurlyBracket) } - _ => return self.invalid(), + _ => self.invalid(), } } } @@ -1072,8 +1062,8 @@ mod tests { collect_result(s) .iter() .filter_map(|v| match v { - &Token::IntegerLiteral(r) => Some(r), - &Token::Newline => None, + Token::IntegerLiteral(r) => Some(*r), + Token::Newline => None, _ => panic!("{:?} is not a integer literal", v), }) .collect() diff --git a/starlark/src/syntax/mod.rs b/starlark/src/syntax/mod.rs index 73be39cc..7902d18c 100644 --- a/starlark/src/syntax/mod.rs +++ b/starlark/src/syntax/mod.rs @@ -26,15 +26,21 @@ pub mod ast; pub mod dialect; #[doc(hidden)] pub mod lexer; + +#[allow(clippy::all)] mod grammar { include!(concat!(env!("OUT_DIR"), "/syntax/grammar.rs")); - #[cfg(test)] - mod tests { - include!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/syntax/grammar.tests.rs" - )); - } } + +// TODO(damienmg): there doesn't seem to have a way to reactivate default +// clippy warning / errors only for the tests... +#[cfg(test)] +mod grammar_tests { + include!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/src/syntax/grammar.tests.rs" + )); +} + #[doc(hidden)] pub mod parser; diff --git a/starlark/src/syntax/parser.rs b/starlark/src/syntax/parser.rs index 8c420cbd..3d601bf1 100644 --- a/starlark/src/syntax/parser.rs +++ b/starlark/src/syntax/parser.rs @@ -28,13 +28,13 @@ extern crate lalrpop_util as lu; // TODO: move that code in some common error code list? // CP Prefix = Critical Parsing -const INVALID_TOKEN_ERROR_CODE: &'static str = "CP00"; -const UNEXPECTED_TOKEN_ERROR_CODE: &'static str = "CP01"; -const EXTRA_TOKEN_ERROR_CODE: &'static str = "CP02"; -const RESERVED_KEYWORD_ERROR_CODE: &'static str = "CP03"; -const IO_ERROR_CODE: &'static str = "CP04"; +const INVALID_TOKEN_ERROR_CODE: &str = "CP00"; +const UNEXPECTED_TOKEN_ERROR_CODE: &str = "CP01"; +const EXTRA_TOKEN_ERROR_CODE: &str = "CP02"; +const RESERVED_KEYWORD_ERROR_CODE: &str = "CP03"; +const IO_ERROR_CODE: &str = "CP04"; -fn one_of(expected: &Vec) -> String { +fn one_of(expected: &[String]) -> String { let mut result = String::new(); for (i, e) in expected.iter().enumerate() { let sep = match i { diff --git a/starlark/src/values/dict.rs b/starlark/src/values/dict.rs index ccaa974f..cdfed94f 100644 --- a/starlark/src/values/dict.rs +++ b/starlark/src/values/dict.rs @@ -26,6 +26,7 @@ pub struct Dictionary { } impl Dictionary { + #[allow(clippy::new_ret_no_self)] pub fn new() -> Value { Value::new(Dictionary { mutability: IterableMutability::Mutable, @@ -131,8 +132,8 @@ impl TypedValue for Dictionary { fn compare(&self, other: &TypedValue, recursion: u32) -> Result { if other.get_type() == "dict" { - let mut v1: Vec = self.into_iter()?.collect(); - let mut v2: Vec = other.into_iter()?.collect(); + let mut v1: Vec = self.iter()?.collect(); + let mut v2: Vec = other.iter()?.collect(); // We sort the keys because the dictionary preserve insertion order but ordering does // not matter in the comparison. This make the comparison O(n.log n) instead of O(n). v1.sort(); @@ -188,7 +189,7 @@ impl TypedValue for Dictionary { }) } - fn into_iter<'a>(&'a self) -> Result + 'a>, ValueError> { + fn iter<'a>(&'a self) -> Result + 'a>, ValueError> { Ok(Box::new(self.content.iter().map(|x| x.0.clone()))) } @@ -216,7 +217,7 @@ impl TypedValue for Dictionary { for (k, v) in self.content.iter() { result.content.insert(k.clone(), v.clone()); } - for k in other.into_iter()? { + for k in other.iter()? { result.content.insert(k.clone(), other.at(k)?.clone()); } Ok(Value::new(result)) diff --git a/starlark/src/values/function.rs b/starlark/src/values/function.rs index 6489ff1c..9f32cf8c 100644 --- a/starlark/src/values/function.rs +++ b/starlark/src/values/function.rs @@ -37,8 +37,11 @@ pub enum FunctionType { Def(String, String), } +pub type StarlarkFunctionPrototype = + Fn(&[(String, String)], Environment, Vec) -> ValueResult; + pub struct Function { - function: Box, Environment, Vec) -> ValueResult>, + function: Box, signature: Vec, function_type: FunctionType, } @@ -51,12 +54,12 @@ struct WrappedMethod { // TODO: move that code in some common error code list? // CV prefix = Critical Function call -const NOT_ENOUGH_PARAMS_ERROR_CODE: &'static str = "CF00"; -const WRONG_ARGS_IDENT_ERROR_CODE: &'static str = "CF01"; -const ARGS_NOT_ITERABLE_ERROR_CODE: &'static str = "CF02"; -const KWARGS_NOT_MAPPABLE_ERROR_CODE: &'static str = "CF03"; -// Not an error: const KWARGS_KEY_IDENT_ERROR_CODE: &'static str = "CF04"; -const EXTRA_PARAMETER_ERROR_CODE: &'static str = "CF05"; +const NOT_ENOUGH_PARAMS_ERROR_CODE: &str = "CF00"; +const WRONG_ARGS_IDENT_ERROR_CODE: &str = "CF01"; +const ARGS_NOT_ITERABLE_ERROR_CODE: &str = "CF02"; +const KWARGS_NOT_MAPPABLE_ERROR_CODE: &str = "CF03"; +// Not an error: const KWARGS_KEY_IDENT_ERROR_CODE: &str = "CF04"; +const EXTRA_PARAMETER_ERROR_CODE: &str = "CF05"; #[derive(Debug, Clone)] pub enum FunctionError { @@ -97,7 +100,7 @@ impl Into for FunctionError { signature, } => format!( "Missing parameter {} for call to {}", - missing.trim_start_matches("$"), + missing.trim_start_matches('$'), repr(&function_type, &signature) ), FunctionError::ArgsValueIsNotString => { @@ -124,13 +127,14 @@ impl Into for FunctionError { } impl Function { + #[allow(clippy::new_ret_no_self)] pub fn new(name: String, f: F, signature: Vec) -> Value where - F: Fn(&Vec<(String, String)>, Environment, Vec) -> ValueResult + 'static, + F: Fn(&[(String, String)], Environment, Vec) -> ValueResult + 'static, { Value::new(Function { function: Box::new(f), - signature: signature, + signature, function_type: FunctionType::Native(name), }) } @@ -142,7 +146,7 @@ impl Function { signature: Vec, ) -> Value where - F: Fn(&Vec<(String, String)>, Environment, Vec) -> ValueResult + 'static, + F: Fn(&[(String, String)], Environment, Vec) -> ValueResult + 'static, { Value::new(Function { function: Box::new(f), @@ -151,6 +155,7 @@ impl Function { }) } + #[allow(clippy::boxed_local)] pub fn new_def( name: String, module: String, @@ -180,55 +185,55 @@ impl Function { impl FunctionType { fn to_str(&self) -> String { match self { - &FunctionType::Native(ref name) => name.clone(), - &FunctionType::NativeMethod(ref function_type, ref name) => { + FunctionType::Native(ref name) => name.clone(), + FunctionType::NativeMethod(ref function_type, ref name) => { format!("{}.{}", function_type, name) } - &FunctionType::Def(ref name, ..) => name.clone(), + FunctionType::Def(ref name, ..) => name.clone(), } } fn to_repr(&self) -> String { match self { - &FunctionType::Native(ref name) => format!("", name), - &FunctionType::NativeMethod(ref function_type, ref name) => { + FunctionType::Native(ref name) => format!("", name), + FunctionType::NativeMethod(ref function_type, ref name) => { format!("", function_type, name) } - &FunctionType::Def(ref name, ref module, ..) => { + FunctionType::Def(ref name, ref module, ..) => { format!("", name, module) } } } } -fn repr(function_type: &FunctionType, signature: &Vec) -> String { +fn repr(function_type: &FunctionType, signature: &[FunctionParameter]) -> String { let v: Vec = signature .iter() .map(|x| -> String { match x { - &FunctionParameter::Normal(ref name) => name.clone(), - &FunctionParameter::WithDefaultValue(ref name, ref value) => { + FunctionParameter::Normal(ref name) => name.clone(), + FunctionParameter::WithDefaultValue(ref name, ref value) => { format!("{} = {}", name, value.to_repr()) } - &FunctionParameter::ArgsArray(ref name) => format!("*{}", name), - &FunctionParameter::KWArgsDict(ref name) => format!("**{}", name), + FunctionParameter::ArgsArray(ref name) => format!("*{}", name), + FunctionParameter::KWArgsDict(ref name) => format!("**{}", name), } }) .collect(); format!("{}({})", function_type.to_repr(), v.join(", ")) } -fn to_str(function_type: &FunctionType, signature: &Vec) -> String { +fn to_str(function_type: &FunctionType, signature: &[FunctionParameter]) -> String { let v: Vec = signature .iter() .map(|x| -> String { match x { - &FunctionParameter::Normal(ref name) => name.clone(), - &FunctionParameter::WithDefaultValue(ref name, ref value) => { + FunctionParameter::Normal(ref name) => name.clone(), + FunctionParameter::WithDefaultValue(ref name, ref value) => { format!("{} = {}", name, value.to_repr()) } - &FunctionParameter::ArgsArray(ref name) => format!("*{}", name), - &FunctionParameter::KWArgsDict(ref name) => format!("**{}", name), + FunctionParameter::ArgsArray(ref name) => format!("*{}", name), + FunctionParameter::KWArgsDict(ref name) => format!("**{}", name), } }) .collect(); @@ -266,7 +271,7 @@ impl TypedValue for Function { fn call( &self, - call_stack: &Vec<(String, String)>, + call_stack: &[(String, String)], env: Environment, positional: Vec, named: HashMap, @@ -277,7 +282,7 @@ impl TypedValue for Function { let mut v = Vec::new(); // Collect args let av: Vec = if let Some(x) = args { - match x.into_iter() { + match x.iter() { Ok(y) => positional.into_iter().chain(y).collect(), Err(..) => return Err(FunctionError::ArgsArrayIsNotIterable.into()), } @@ -288,7 +293,7 @@ impl TypedValue for Function { // Collect kwargs let mut kwargs_dict = named.clone(); if let Some(x) = kwargs { - match x.into_iter() { + match x.iter() { Ok(y) => { for n in y { if n.get_type() == "string" { @@ -309,40 +314,36 @@ impl TypedValue for Function { // Now verify signature and transform in a value vector for parameter in self.signature.iter() { match parameter { - &FunctionParameter::Normal(ref name) => { + FunctionParameter::Normal(ref name) => { if let Some(x) = args_iter.next() { v.push(x) + } else if let Some(ref r) = kwargs_dict.remove(name) { + v.push(r.clone()); } else { - if let Some(ref r) = kwargs_dict.remove(name) { - v.push(r.clone()); - } else { - return Err(FunctionError::NotEnoughParameter { - missing: name.to_string(), - function_type: self.function_type.clone(), - signature: self.signature.clone(), - } - .into()); + return Err(FunctionError::NotEnoughParameter { + missing: name.to_string(), + function_type: self.function_type.clone(), + signature: self.signature.clone(), } + .into()); } } - &FunctionParameter::WithDefaultValue(ref name, ref value) => { + FunctionParameter::WithDefaultValue(ref name, ref value) => { if let Some(x) = args_iter.next() { v.push(x) + } else if let Some(ref r) = kwargs_dict.remove(name) { + v.push(r.clone()); } else { - if let Some(ref r) = kwargs_dict.remove(name) { - v.push(r.clone()); - } else { - v.push(value.clone()); - } + v.push(value.clone()); } } - &FunctionParameter::ArgsArray(..) => { + FunctionParameter::ArgsArray(..) => { let argv: Vec = args_iter.clone().collect(); v.push(Value::from(argv)); // We do not use last so we keep ownership of the iterator while args_iter.next().is_some() {} } - &FunctionParameter::KWArgsDict(..) => { + FunctionParameter::KWArgsDict(..) => { v.push(Value::from(kwargs_dict.clone())); kwargs_dict.clear(); } @@ -385,7 +386,7 @@ impl TypedValue for WrappedMethod { fn call( &self, - call_stack: &Vec<(String, String)>, + call_stack: &[(String, String)], env: Environment, positional: Vec, named: HashMap, diff --git a/starlark/src/values/list.rs b/starlark/src/values/list.rs index 58ebab2e..0437f6e9 100644 --- a/starlark/src/values/list.rs +++ b/starlark/src/values/list.rs @@ -37,6 +37,7 @@ impl + Clone> From> for List { } impl List { + #[allow(clippy::new_ret_no_self)] pub fn new() -> Value { Value::new(List { mutability: IterableMutability::Mutable, @@ -109,8 +110,8 @@ impl TypedValue for List { fn compare(&self, other: &TypedValue, recursion: u32) -> Result { if other.get_type() == "list" { - let mut iter1 = self.into_iter()?; - let mut iter2 = other.into_iter()?; + let mut iter1 = self.iter()?; + let mut iter2 = other.iter()?; loop { match (iter1.next(), iter2.next()) { (None, None) => return Ok(Ordering::Equal), @@ -169,8 +170,8 @@ impl TypedValue for List { ))) } - fn into_iter<'a>(&'a self) -> Result + 'a>, ValueError> { - Ok(Box::new(self.content.iter().map(|x| x.clone()))) + fn iter<'a>(&'a self) -> Result + 'a>, ValueError> { + Ok(Box::new(self.content.iter().cloned())) } /// Concatenate `other` to the current value. @@ -197,7 +198,7 @@ impl TypedValue for List { for x in self.content.iter() { result.content.push(x.clone()); } - for x in other.into_iter()? { + for x in other.iter()? { result.content.push(x.clone()); } Ok(Value::new(result)) diff --git a/starlark/src/values/mod.rs b/starlark/src/values/mod.rs index 2e3a70ed..c2433206 100644 --- a/starlark/src/values/mod.rs +++ b/starlark/src/values/mod.rs @@ -90,21 +90,21 @@ use syntax::errors::SyntaxError; // TODO: move that code in some common error code list? // CV prefix = Critical Value expression -pub const NOT_SUPPORTED_ERROR_CODE: &'static str = "CV00"; -pub const IMMUTABLE_ERROR_CODE: &'static str = "CV01"; -pub const INCORRECT_PARAMETER_TYPE_ERROR_CODE: &'static str = "CV02"; -pub const OUT_OF_BOUND_ERROR_CODE: &'static str = "CV03"; -pub const NOT_HASHABLE_VALUE_ERROR_CODE: &'static str = "CV04"; -pub const KEY_NOT_FOUND_ERROR_CODE: &'static str = "CV05"; -pub const INTERPOLATION_FORMAT_ERROR_CODE: &'static str = "CV06"; -pub const INTERPOLATION_OUT_OF_UTF8_RANGE_ERROR_CODE: &'static str = "CV07"; -pub const DIVISION_BY_ZERO_ERROR_CODE: &'static str = "CV08"; -pub const INTERPOLATION_TOO_MANY_PARAMS_ERROR_CODE: &'static str = "CV09"; -pub const INTERPOLATION_NOT_ENOUGH_PARAMS_ERROR_CODE: &'static str = "CV10"; -pub const INTERPOLATION_VALUE_IS_NOT_CHAR_ERROR_CODE: &'static str = "CV12"; -pub const TOO_MANY_RECURSION_LEVEL_ERROR_CODE: &'static str = "CV13"; -pub const UNSUPPORTED_RECURSIVE_DATA_STRUCTURE_ERROR_CODE: &'static str = "CV14"; -pub const CANNOT_MUTATE_DURING_ITERATION_ERROR_CODE: &'static str = "CV15"; +pub const NOT_SUPPORTED_ERROR_CODE: &str = "CV00"; +pub const IMMUTABLE_ERROR_CODE: &str = "CV01"; +pub const INCORRECT_PARAMETER_TYPE_ERROR_CODE: &str = "CV02"; +pub const OUT_OF_BOUND_ERROR_CODE: &str = "CV03"; +pub const NOT_HASHABLE_VALUE_ERROR_CODE: &str = "CV04"; +pub const KEY_NOT_FOUND_ERROR_CODE: &str = "CV05"; +pub const INTERPOLATION_FORMAT_ERROR_CODE: &str = "CV06"; +pub const INTERPOLATION_OUT_OF_UTF8_RANGE_ERROR_CODE: &str = "CV07"; +pub const DIVISION_BY_ZERO_ERROR_CODE: &str = "CV08"; +pub const INTERPOLATION_TOO_MANY_PARAMS_ERROR_CODE: &str = "CV09"; +pub const INTERPOLATION_NOT_ENOUGH_PARAMS_ERROR_CODE: &str = "CV10"; +pub const INTERPOLATION_VALUE_IS_NOT_CHAR_ERROR_CODE: &str = "CV12"; +pub const TOO_MANY_RECURSION_LEVEL_ERROR_CODE: &str = "CV13"; +pub const UNSUPPORTED_RECURSIVE_DATA_STRUCTURE_ERROR_CODE: &str = "CV14"; +pub const CANNOT_MUTATE_DURING_ITERATION_ERROR_CODE: &str = "CV15"; // Maximum recursion level for comparison // TODO(dmarting): those are rather short, maybe make it configurable? @@ -483,7 +483,7 @@ pub trait TypedValue { /// * kwargs: if provided, the `**kwargs` argument. fn call( &self, - call_stack: &Vec<(String, String)>, + call_stack: &[(String, String)], env: Environment, positional: Vec, named: HashMap, @@ -553,7 +553,7 @@ pub trait TypedValue { /// Returns an iterator over the value of this container if this value hold an iterable /// container. - fn into_iter<'a>(&'a self) -> Result + 'a>, ValueError>; + fn iter<'a>(&'a self) -> Result + 'a>, ValueError>; /// Returns the length of the value, if this value is a sequence. fn length(&self) -> Result; @@ -760,7 +760,7 @@ macro_rules! not_supported { } }; (call) => { - fn call(&self, _call_stack: &Vec<(String, String)>, _env: Environment, + fn call(&self, _call_stack: &[(String, String)], _env: Environment, _positional: Vec, _named: HashMap, _args: Option, _kwargs: Option) -> ValueResult { Err(ValueError::OperationNotSupported { @@ -834,8 +834,8 @@ macro_rules! not_supported { op: "[::]".to_owned(), left: self.get_type().to_owned(), right: None }) } }; - (into_iter) => { - fn into_iter(&self) -> Result>, ValueError> { + (iter) => { + fn iter(&self) -> Result>, ValueError> { Err(ValueError::TypeNotX { object_type: self.get_type().to_owned(), op: "iterable".to_owned() @@ -847,7 +847,7 @@ macro_rules! not_supported { fn unfreeze_for_iteration(&mut self) {} }; // Special type: iterable, sequence, indexable, container, function - (iterable) => { not_supported!(into_iter, freeze_for_iteration); }; + (iterable) => { not_supported!(iter, freeze_for_iteration); }; (sequence) => { not_supported!(length, is_in); }; (set_indexable) => { not_supported!(set_at); }; (indexable) => { not_supported!(slice, at, set_indexable); }; @@ -1130,7 +1130,7 @@ impl Value { pub fn call( &self, - call_stack: &Vec<(String, String)>, + call_stack: &[(String, String)], env: Environment, positional: Vec, named: HashMap, @@ -1157,9 +1157,9 @@ impl Value { let borrowed = self.0.borrow_mut(); borrowed.slice(start, stop, stride) } - pub fn into_iter<'a>(&'a self) -> Result + 'a>, ValueError> { + pub fn iter<'a>(&'a self) -> Result + 'a>, ValueError> { let borrowed = self.0.borrow(); - let v: Vec = borrowed.into_iter()?.map(|x| x.clone()).collect(); + let v: Vec = borrowed.iter()?.collect(); Ok(Box::new(v.into_iter())) } pub fn length(&self) -> Result { @@ -1290,7 +1290,7 @@ impl TypedValue for Option<()> { } // just took the result of hash(None) in macos python 2.7.10 interpreter. fn get_hash(&self) -> Result { - Ok(9223380832852120682) + Ok(9_223_380_832_852_120_682) } default_compare!(); not_supported!(binop); @@ -1588,7 +1588,7 @@ impl TypedValue { stop: Option, stride: Option, ) -> Result<(i64, i64, i64), ValueError> { - let stride = stride.unwrap_or(Value::new(1)); + let stride = stride.unwrap_or_else(|| Value::new(1)); let stride = if stride.get_type() == "NoneType" { Ok(1) } else { @@ -1674,6 +1674,7 @@ macro_rules! from_X { ($x: ty, $y: tt, noT) => { impl From<$x> for Value { fn from(a: $x) -> Value { + #[allow(clippy::cast_lossless)] Value::new(a as $y) } } diff --git a/starlark/src/values/string.rs b/starlark/src/values/string.rs index 552cb0d3..2ac1f8b5 100644 --- a/starlark/src/values/string.rs +++ b/starlark/src/values/string.rs @@ -59,7 +59,7 @@ impl TypedValue for String { fn at(&self, index: Value) -> ValueResult { let i = index.convert_index(self.len() as i64)? as usize; - Ok(Value::new(self.chars().skip(i).next().unwrap().to_string())) + Ok(Value::new(self.chars().nth(i).unwrap().to_string())) } fn length(&self) -> Result { @@ -209,7 +209,7 @@ impl TypedValue for String { /// # ); /// ``` fn percent(&self, other: Value) -> ValueResult { - let mut split_it = self.split("%"); + let mut split_it = self.split('%'); let mut res = String::new(); let mut idx = 0; let mut len = 0; @@ -233,7 +233,7 @@ impl TypedValue for String { } other.at(Value::new(varname))?.clone() } else { - match other.into_iter() { + match other.iter() { Ok(..) => { let val = other.at(Value::new(idx)); idx += 1; diff --git a/starlark/src/values/tuple.rs b/starlark/src/values/tuple.rs index f20b3289..5959f95a 100644 --- a/starlark/src/values/tuple.rs +++ b/starlark/src/values/tuple.rs @@ -25,7 +25,7 @@ pub struct Tuple { } #[doc(hidden)] -pub fn slice_vector(start: i64, stop: i64, stride: i64, content: &Vec) -> Vec { +pub fn slice_vector(start: i64, stop: i64, stride: i64, content: &[Value]) -> Vec { let (low, take, astride) = if stride < 0 { (stop + 1, start - stop, -stride) } else { @@ -38,7 +38,7 @@ pub fn slice_vector(start: i64, stop: i64, stride: i64, content: &Vec) -> .iter() .skip(low as usize) .take(take as usize) - .map(|x| x.clone()) + .cloned() .collect(); if stride < 0 { v.reverse(); @@ -302,8 +302,8 @@ impl TypedValue for Tuple { fn compare(&self, other: &TypedValue, recursion: u32) -> Result { if other.get_type() == "tuple" { - let mut iter1 = self.into_iter()?; - let mut iter2 = other.into_iter()?; + let mut iter1 = self.iter()?; + let mut iter2 = other.iter()?; loop { match (iter1.next(), iter2.next()) { (None, None) => return Ok(Ordering::Equal), @@ -362,8 +362,8 @@ impl TypedValue for Tuple { )))) } - fn into_iter<'a>(&'a self) -> Result + 'a>, ValueError> { - Ok(Box::new(self.content.iter().map(|x| x.clone()))) + fn iter<'a>(&'a self) -> Result + 'a>, ValueError> { + Ok(Box::new(self.content.iter().cloned())) } /// Concatenate `other` to the current value. @@ -388,7 +388,7 @@ impl TypedValue for Tuple { for x in self.content.iter() { result.content.push(x.clone()); } - for x in other.into_iter()? { + for x in other.iter()? { result.content.push(x.clone()); } Ok(Value::new(result)) diff --git a/starlark/tests/testutil/mod.rs b/starlark/tests/testutil/mod.rs index 24f501dc..7a25ee1e 100644 --- a/starlark/tests/testutil/mod.rs +++ b/starlark/tests/testutil/mod.rs @@ -60,7 +60,7 @@ fn assert_diagnostic( }; if !msg.to_lowercase().contains(&expected) { io::stderr() - .write( + .write_all( &format!( "Expected error '{}' at {}:{}, got {}\n", expected, path, offset, msg, @@ -100,7 +100,8 @@ def assert_(cond, msg="assertion failed"): for (offset, content) in read_input(path) { let err = if let Some(x) = content.find("###") { let err = content.get(x + 3..).unwrap().trim(); - err.get(..err.find("\n").unwrap_or(err.len())).unwrap() + err.get(..err.find('\n').unwrap_or_else(|| err.len())) + .unwrap() } else { "" }; @@ -115,16 +116,14 @@ def assert_(cond, msg="assertion failed"): if err.is_empty() { Emitter::stderr(ColorConfig::Always, Some(&map.lock().unwrap())).emit(&[p]); return false; - } else { - if !assert_diagnostic(p, err, path, offset, &map) { - return false; - } + } else if !assert_diagnostic(p, err, path, offset, &map) { + return false; } } _ => { if !err.is_empty() { io::stderr() - .write( + .write_all( &format!( "Expected error '{}' at {}:{}, got success", err, path, offset, @@ -137,7 +136,7 @@ def assert_(cond, msg="assertion failed"): } } } - return true; + true } pub fn do_conformance_test(path: &str) {