Merge pull request #1114 from mkurnikov/fix-clippy-1.36

Enable clippy in CI

.travis.yml

@@ -39,6 +39,17 @@ matrix:
       env:
         - JOBCACHE=3
 
+    - name: Lint code with clippy
+      language: rust
+      rust: stable
+      cache: cargo
+      before_script:
+        - rustup component add clippy
+      script:
+        - cargo clippy --all -- -Dwarnings
+      env:
+        - JOBCACHE=8
+
     - name: publish documentation
       language: rust
       rust: stable

compiler/src/symboltable.rs

@@ -301,19 +301,17 @@ impl SymbolTableBuilder {
                     if let Some(alias) = &part.alias {
                         // `import mymodule as myalias`
                         self.register_name(alias, SymbolRole::Assigned)?;
+                    } else if part.symbols.is_empty() {
+                        // `import module`
+                        self.register_name(&part.module, SymbolRole::Assigned)?;
                     } else {
-                        if part.symbols.is_empty() {
-                            // `import module`
-                            self.register_name(&part.module, SymbolRole::Assigned)?;
-                        } else {
-                            // `from mymodule import myimport`
-                            for symbol in &part.symbols {
-                                if let Some(alias) = &symbol.alias {
-                                    // `from mymodule import myimportname as myalias`
-                                    self.register_name(alias, SymbolRole::Assigned)?;
-                                } else {
-                                    self.register_name(&symbol.symbol, SymbolRole::Assigned)?;
-                                }
+                        // `from mymodule import myimport`
+                        for symbol in &part.symbols {
+                            if let Some(alias) = &symbol.alias {
+                                // `from mymodule import myimportname as myalias`
+                                self.register_name(alias, SymbolRole::Assigned)?;
+                            } else {
+                                self.register_name(&symbol.symbol, SymbolRole::Assigned)?;
                             }
                         }
                     }

derive/src/compile_bytecode.rs

@@ -77,7 +77,7 @@ impl PyCompileInput {
         fn assert_source_empty(source: &Option<CompilationSource>) -> Result<(), Diagnostic> {
             if let Some(source) = source {
                 Err(Diagnostic::spans_error(
-                    source.span.clone(),
+                    source.span,
                     "Cannot have more than one source",
                 ))
             } else {
@@ -86,53 +86,50 @@ impl PyCompileInput {
         }
 
         for meta in &self.metas {
-            match meta {
-                Meta::NameValue(name_value) => {
-                    if name_value.ident == "mode" {
-                        mode = Some(match &name_value.lit {
-                            Lit::Str(s) => match s.value().as_str() {
-                                "exec" => compile::Mode::Exec,
-                                "eval" => compile::Mode::Eval,
-                                "single" => compile::Mode::Single,
-                                _ => bail_span!(s, "mode must be exec, eval, or single"),
-                            },
-                            _ => bail_span!(name_value.lit, "mode must be a string"),
-                        })
-                    } else if name_value.ident == "module_name" {
-                        module_name = Some(match &name_value.lit {
-                            Lit::Str(s) => s.value(),
-                            _ => bail_span!(name_value.lit, "module_name must be string"),
-                        })
-                    } else if name_value.ident == "source" {
-                        assert_source_empty(&source)?;
-                        let code = match &name_value.lit {
-                            Lit::Str(s) => s.value(),
-                            _ => bail_span!(name_value.lit, "source must be a string"),
-                        };
-                        source = Some(CompilationSource {
-                            kind: CompilationSourceKind::SourceCode(code),
-                            span: extract_spans(&name_value).unwrap(),
-                        });
-                    } else if name_value.ident == "file" {
-                        assert_source_empty(&source)?;
-                        let path = match &name_value.lit {
-                            Lit::Str(s) => PathBuf::from(s.value()),
-                            _ => bail_span!(name_value.lit, "source must be a string"),
-                        };
-                        source = Some(CompilationSource {
-                            kind: CompilationSourceKind::File(path),
-                            span: extract_spans(&name_value).unwrap(),
-                        });
-                    }
+            if let Meta::NameValue(name_value) = meta {
+                if name_value.ident == "mode" {
+                    mode = Some(match &name_value.lit {
+                        Lit::Str(s) => match s.value().as_str() {
+                            "exec" => compile::Mode::Exec,
+                            "eval" => compile::Mode::Eval,
+                            "single" => compile::Mode::Single,
+                            _ => bail_span!(s, "mode must be exec, eval, or single"),
+                        },
+                        _ => bail_span!(name_value.lit, "mode must be a string"),
+                    })
+                } else if name_value.ident == "module_name" {
+                    module_name = Some(match &name_value.lit {
+                        Lit::Str(s) => s.value(),
+                        _ => bail_span!(name_value.lit, "module_name must be string"),
+                    })
+                } else if name_value.ident == "source" {
+                    assert_source_empty(&source)?;
+                    let code = match &name_value.lit {
+                        Lit::Str(s) => s.value(),
+                        _ => bail_span!(name_value.lit, "source must be a string"),
+                    };
+                    source = Some(CompilationSource {
+                        kind: CompilationSourceKind::SourceCode(code),
+                        span: extract_spans(&name_value).unwrap(),
+                    });
+                } else if name_value.ident == "file" {
+                    assert_source_empty(&source)?;
+                    let path = match &name_value.lit {
+                        Lit::Str(s) => PathBuf::from(s.value()),
+                        _ => bail_span!(name_value.lit, "source must be a string"),
+                    };
+                    source = Some(CompilationSource {
+                        kind: CompilationSourceKind::File(path),
+                        span: extract_spans(&name_value).unwrap(),
+                    });
                 }
-                _ => {}
             }
         }
 
         source
             .ok_or_else(|| {
                 Diagnostic::span_error(
-                    self.span.clone(),
+                    self.span,
                     "Must have either file or source in py_compile_bytecode!()",
                 )
             })?

derive/src/error.rs

@@ -104,7 +104,7 @@ impl Diagnostic {
     }
 
     pub fn from_vec(diagnostics: Vec<Diagnostic>) -> Result<(), Diagnostic> {
-        if diagnostics.len() == 0 {
+        if diagnostics.is_empty() {
             Ok(())
         } else {
             Err(Diagnostic {

derive/src/from_args.rs

@@ -62,7 +62,7 @@ impl ArgAttribute {
                     optional: false,
                 };
 
-                while let Some(arg) = iter.next() {
+                for arg in iter {
                     attribute.parse_argument(arg)?;
                 }
 

derive/src/pyclass.rs

@@ -63,20 +63,14 @@ impl ClassItem {
                         NestedMeta::Meta(meta) => meta,
                         NestedMeta::Literal(_) => continue,
                     };
-                    match meta {
-                        Meta::NameValue(name_value) => {
-                            if name_value.ident == "name" {
-                                if let Lit::Str(s) = &name_value.lit {
-                                    py_name = Some(s.value());
-                                } else {
-                                    bail_span!(
-                                        &sig.ident,
-                                        "#[pymethod(name = ...)] must be a string"
-                                    );
-                                }
+                    if let Meta::NameValue(name_value) = meta {
+                        if name_value.ident == "name" {
+                            if let Lit::Str(s) = &name_value.lit {
+                                py_name = Some(s.value());
+                            } else {
+                                bail_span!(&sig.ident, "#[pymethod(name = ...)] must be a string");
                             }
                         }
-                        _ => {}
                     }
                 }
                 item = Some(ClassItem::Method {
@@ -104,20 +98,17 @@ impl ClassItem {
                         NestedMeta::Meta(meta) => meta,
                         NestedMeta::Literal(_) => continue,
                     };
-                    match meta {
-                        Meta::NameValue(name_value) => {
-                            if name_value.ident == "name" {
-                                if let Lit::Str(s) = &name_value.lit {
-                                    py_name = Some(s.value());
-                                } else {
-                                    bail_span!(
-                                        &sig.ident,
-                                        "#[pyclassmethod(name = ...)] must be a string"
-                                    );
-                                }
+                    if let Meta::NameValue(name_value) = meta {
+                        if name_value.ident == "name" {
+                            if let Lit::Str(s) = &name_value.lit {
+                                py_name = Some(s.value());
+                            } else {
+                                bail_span!(
+                                    &sig.ident,
+                                    "#[pyclassmethod(name = ...)] must be a string"
+                                );
                             }
                         }
-                        _ => {}
                     }
                 }
                 item = Some(ClassItem::ClassMethod {
@@ -235,24 +226,22 @@ pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> Result<TokenStream2, Dia
     let ty = &imp.self_ty;
     let mut properties: HashMap<&str, (Option<&Ident>, Option<&Ident>)> = HashMap::new();
     for item in items.iter() {
-        match item {
-            ClassItem::Property {
-                ref item_ident,
-                ref py_name,
-                setter,
-            } => {
-                let entry = properties.entry(py_name).or_default();
-                let func = if *setter { &mut entry.1 } else { &mut entry.0 };
-                if func.is_some() {
-                    bail_span!(
-                        item_ident,
-                        "Multiple property accessors with name {:?}",
-                        py_name
-                    )
-                }
-                *func = Some(item_ident);
+        if let ClassItem::Property {
+            ref item_ident,
+            ref py_name,
+            setter,
+        } = item
+        {
+            let entry = properties.entry(py_name).or_default();
+            let func = if *setter { &mut entry.1 } else { &mut entry.0 };
+            if func.is_some() {
+                bail_span!(
+                    item_ident,
+                    "Multiple property accessors with name {:?}",
+                    py_name
+                )
             }
-            _ => {}
+            *func = Some(item_ident);
         }
     }
     let methods = items.iter().filter_map(|item| match item {
@@ -319,7 +308,7 @@ fn generate_class_def(
     ident: &Ident,
     attr_name: &'static str,
     attr: AttributeArgs,
-    attrs: &Vec<Attribute>,
+    attrs: &[Attribute],
 ) -> Result<TokenStream2, Diagnostic> {
     let mut class_name = None;
     for attr in attr {

parser/src/lexer.rs

@@ -488,15 +488,15 @@ where
         for i in 1..=literal_number {
             match self.next_char() {
                 Some(c) => match c.to_digit(16) {
-                    Some(d) => p += d << (literal_number - i) * 4,
+                    Some(d) => p += d << ((literal_number - i) * 4),
                     None => return unicode_error,
                 },
                 None => return unicode_error,
             }
         }
         match wtf8::CodePoint::from_u32(p) {
-            Some(cp) => return Ok(cp.to_char_lossy()),
-            None => return unicode_error,
+            Some(cp) => Ok(cp.to_char_lossy()),
+            None => unicode_error,
         }
     }
 
@@ -755,12 +755,14 @@ where
         Ok(IndentationLevel { spaces, tabs })
     }
 
+    #[allow(clippy::cognitive_complexity)]
     fn inner_next(&mut self) -> LexResult {
         if !self.pending.is_empty() {
             return self.pending.remove(0);
         }
 
-        'top_loop: loop {
+        // top loop
+        loop {
             // Detect indentation levels
             if self.at_begin_of_line {
                 self.at_begin_of_line = false;

vm/src/builtins.rs

@@ -713,7 +713,7 @@ fn builtin_reversed(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
         vm.invoke(reversed_method?, PyFuncArgs::default())
     } else {
         vm.get_method_or_type_error(obj.clone(), "__getitem__", || {
-            format!("argument to reversed() must be a sequence")
+            "argument to reversed() must be a sequence".to_string()
         })?;
         let len = vm.call_method(&obj.clone(), "__len__", PyFuncArgs::default())?;
         let obj_iterator = objiter::PySequenceIterator {

vm/src/cformat.rs

@@ -119,7 +119,7 @@ impl CFormatSpec {
     ) -> String {
         let mut num_chars = string.chars().count();
         if let Some(num_prefix_chars) = num_prefix_chars {
-            num_chars = num_chars + num_prefix_chars;
+            num_chars += num_prefix_chars;
         }
         let num_chars = num_chars;
 
@@ -250,7 +250,7 @@ impl FromStr for CFormatString {
                 .or_else(|_| parse_specifier(cur_text))
                 .map(|(format_part, new_text, consumed)| {
                     parts.push((index, format_part));
-                    index = index + consumed;
+                    index += consumed;
                     new_text
                 })
                 .map_err(|(e, consumed)| CFormatError {
@@ -320,7 +320,7 @@ fn parse_literal(text: &str) -> Result<(CFormatPart, &str, usize), ParsingError>
         match parse_literal_single(cur_text) {
             Ok((next_char, remaining)) => {
                 result_string.push(next_char);
-                consumed = consumed + 1;
+                consumed += 1;
                 cur_text = remaining;
             }
             Err(err) => {
@@ -537,12 +537,12 @@ impl FromStr for CFormatSpec {
         };
 
         Ok(CFormatSpec {
-            mapping_key: mapping_key,
-            flags: flags,
+            mapping_key,
+            flags,
             min_field_width: width,
-            precision: precision,
-            format_type: format_type,
-            format_char: format_char,
+            precision,
+            format_type,
+            format_char,
             chars_consumed: calc_consumed(text, remaining_text),
         })
     }

vm/src/format.rs

@@ -20,7 +20,7 @@ impl FormatPreconversor {
         }
     }
 
-    pub fn from_str(text: &str) -> Option<FormatPreconversor> {
+    pub fn from_string(text: &str) -> Option<FormatPreconversor> {
         let mut chars = text.chars();
         if chars.next() != Some('!') {
             return None;
@@ -33,7 +33,7 @@ impl FormatPreconversor {
     }
 
     pub fn parse_and_consume(text: &str) -> (Option<FormatPreconversor>, &str) {
-        let preconversor = FormatPreconversor::from_str(text);
+        let preconversor = FormatPreconversor::from_string(text);
         match preconversor {
             None => (None, text),
             Some(_) => {

vm/src/frame.rs

@@ -333,6 +333,7 @@ impl Frame {
     }
 
     /// Execute a single instruction.
+    #[allow(clippy::cognitive_complexity)]
     fn execute_instruction(&self, vm: &VirtualMachine) -> FrameResult {
         let instruction = self.fetch_instruction();
         {
@@ -357,11 +358,11 @@ impl Frame {
                 ref name,
                 ref symbols,
                 ref level,
-            } => self.import(vm, name, symbols, level),
+            } => self.import(vm, name, symbols, *level),
             bytecode::Instruction::ImportStar {
                 ref name,
                 ref level,
-            } => self.import_star(vm, name, level),
+            } => self.import_star(vm, name, *level),
             bytecode::Instruction::LoadName {
                 ref name,
                 ref scope,
@@ -913,14 +914,14 @@ impl Frame {
         &self,
         vm: &VirtualMachine,
         module: &str,
-        symbols: &Vec<String>,
-        level: &usize,
+        symbols: &[String],
+        level: usize,
     ) -> FrameResult {
         let from_list = symbols
             .iter()
             .map(|symbol| vm.ctx.new_str(symbol.to_string()))
             .collect();
-        let module = vm.import(module, &vm.ctx.new_tuple(from_list), *level)?;
+        let module = vm.import(module, &vm.ctx.new_tuple(from_list), level)?;
 
         if symbols.is_empty() {
             self.push_value(module);
@@ -935,8 +936,8 @@ impl Frame {
         Ok(None)
     }
 
-    fn import_star(&self, vm: &VirtualMachine, module: &str, level: &usize) -> FrameResult {
-        let module = vm.import(module, &vm.ctx.new_tuple(vec![]), *level)?;
+    fn import_star(&self, vm: &VirtualMachine, module: &str, level: usize) -> FrameResult {
+        let module = vm.import(module, &vm.ctx.new_tuple(vec![]), level)?;
 
         // Grab all the names from the module and put them in the context
         if let Some(dict) = &module.dict {

vm/src/import.rs

@@ -102,17 +102,13 @@ pub fn remove_importlib_frames(vm: &VirtualMachine, exc: &PyObjectRef) -> PyObje
                         if run_obj_name == "_call_with_frames_removed" {
                             in_importlib = true;
                         }
-                        if always_trim || in_importlib {
-                            false
-                        } else {
-                            true
-                        }
+                        !always_trim && !in_importlib
                     } else {
                         in_importlib = false;
                         true
                     }
                 })
-                .map(|x| x.clone())
+                .cloned()
                 .collect();
             vm.set_attr(exc, "__traceback__", vm.ctx.new_list(new_tb))
                 .unwrap();

vm/src/obj/objdict.rs

@@ -418,6 +418,7 @@ macro_rules! dict_iterator {
             }
 
             #[pymethod(name = "__next__")]
+            #[allow(clippy::redundant_closure_call)]
             fn next(&self, vm: &VirtualMachine) -> PyResult {
                 let mut position = self.position.get();
                 let dict = self.dict.entries.borrow();

vm/src/obj/objfloat.rs

@@ -24,7 +24,7 @@ pub struct PyFloat {
 }
 
 impl PyFloat {
-    pub fn to_f64(&self) -> f64 {
+    pub fn to_f64(self) -> f64 {
         self.value
     }
 }
@@ -138,6 +138,7 @@ fn inner_gt_int(value: f64, other_int: &BigInt) -> bool {
 }
 
 #[pyimpl]
+#[allow(clippy::trivially_copy_pass_by_ref)]
 impl PyFloat {
     #[pymethod(name = "__eq__")]
     fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef {

vm/src/obj/objrange.rs

@@ -78,7 +78,7 @@ impl PyRange {
         let stop = self.stop.as_bigint();
         let step = self.step.as_bigint();
 
-        let index = if index < &BigInt::zero() {
+        let index = if *index < BigInt::zero() {
             let index = stop + index;
             if index < BigInt::zero() {
                 return None;
@@ -90,8 +90,8 @@ impl PyRange {
 
         let result = start + step * &index;
 
-        if (self.forward() && !self.is_empty() && &result < stop)
-            || (!self.forward() && !self.is_empty() && &result > stop)
+        if (self.forward() && !self.is_empty() && result < *stop)
+            || (!self.forward() && !self.is_empty() && result > *stop)
         {
             Some(result)
         } else {

vm/src/obj/objstr.rs

@@ -928,7 +928,7 @@ impl PyString {
                                 ));
                             }
                         }
-                    } else if let Some(_) = value.payload::<PyNone>() {
+                    } else if value.payload::<PyNone>().is_some() {
                         // Do Nothing
                     } else {
                         return Err(vm.new_type_error(
@@ -1279,19 +1279,16 @@ fn do_cformat(
     }
 
     // check that all arguments were converted
-    if !mapping_required {
-        if objtuple::get_value(&values_obj)
+    if !mapping_required
+        && objtuple::get_value(&values_obj)
             .into_iter()
-            .skip(tuple_index)
-            .next()
+            .nth(tuple_index)
             .is_some()
-        {
-            return Err(vm.new_type_error(
-                "not all arguments converted during string formatting".to_string(),
-            ));
-        }
+    {
+        return Err(
+            vm.new_type_error("not all arguments converted during string formatting".to_string())
+        );
     }
-
     Ok(vm.ctx.new_str(final_string))
 }
 

vm/src/stdlib/imp.rs

@@ -40,12 +40,10 @@ fn imp_create_builtin(spec: PyObjectRef, vm: &VirtualMachine) -> PyResult {
 
     if let Ok(module) = sys_modules.get_item(name, vm) {
         Ok(module)
+    } else if let Some(make_module_func) = vm.stdlib_inits.borrow().get(name) {
+        Ok(make_module_func(vm))
     } else {
-        if let Some(make_module_func) = vm.stdlib_inits.borrow().get(name) {
-            Ok(make_module_func(vm))
-        } else {
-            Ok(vm.get_none())
-        }
+        Ok(vm.get_none())
     }
 }
 

vm/src/stdlib/io.rs

@@ -36,7 +36,7 @@ struct BufferedIO {
 
 impl BufferedIO {
     fn new(cursor: Cursor<Vec<u8>>) -> BufferedIO {
-        BufferedIO { cursor: cursor }
+        BufferedIO { cursor }
     }
 
     fn write(&mut self, data: Vec<u8>) -> Option<u64> {
@@ -55,7 +55,7 @@ impl BufferedIO {
 
     //skip to the jth position
     fn seek(&mut self, offset: u64) -> Option<u64> {
-        match self.cursor.seek(SeekFrom::Start(offset.clone())) {
+        match self.cursor.seek(SeekFrom::Start(offset)) {
             Ok(_) => Some(offset),
             Err(_) => None,
         }
@@ -69,7 +69,8 @@ impl BufferedIO {
         if bytes > 0 {
             let mut handle = self.cursor.clone().take(bytes as u64);
             //read handle into buffer
-            if let Err(_) = handle.read_to_end(&mut buffer) {
+
+            if handle.read_to_end(&mut buffer).is_err() {
                 return None;
             }
             //the take above consumes the struct value
@@ -77,7 +78,7 @@ impl BufferedIO {
             self.cursor = handle.into_inner();
         } else {
             //read handle into buffer
-            if let Err(_) = self.cursor.read_to_end(&mut buffer) {
+            if self.cursor.read_to_end(&mut buffer).is_err() {
                 return None;
             }
         };

vm/src/stdlib/os.rs

@@ -120,7 +120,7 @@ pub fn os_open(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     let fname = &make_path(vm, name, &dir_fd).value;
 
     let flags = FileCreationFlags::from_bits(objint::get_value(flags).to_u32().unwrap())
-        .ok_or(vm.new_value_error("Unsupported flag".to_string()))?;
+        .ok_or_else(|| vm.new_value_error("Unsupported flag".to_string()))?;
 
     let mut options = &mut OpenOptions::new();
 
@@ -460,7 +460,7 @@ impl StatResultRef {
 
 // Copied code from Duration::as_secs_f64 as it's still unstable
 fn duration_as_secs_f64(duration: Duration) -> f64 {
-    (duration.as_secs() as f64) + (duration.subsec_nanos() as f64) / (1_000_000_000 as f64)
+    (duration.as_secs() as f64) + f64::from(duration.subsec_nanos()) / 1_000_000_000_f64
 }
 
 fn to_seconds_from_unix_epoch(sys_time: SystemTime) -> f64 {

vm/src/stdlib/socket.rs

@@ -93,7 +93,7 @@ impl Connection {
             Connection::UdpSocket(con) => con.as_raw_fd(),
             Connection::TcpStream(con) => con.as_raw_fd(),
         };
-        raw_fd as i64
+        i64::from(raw_fd)
     }
 
     #[cfg(windows)]

vm/src/sysmodule.rs

@@ -124,7 +124,7 @@ pub fn make_module(vm: &VirtualMachine, module: PyObjectRef, builtins: PyObjectR
                     .map(|path| {
                         path.into_os_string()
                             .into_string()
-                            .expect(&format!("{} isn't valid unicode", env_variable_name))
+                            .unwrap_or_else(|_| panic!("{} isn't valid unicode", env_variable_name))
                     })
                     .collect(),
                 None => vec![],

wasm/lib/src/js_module.rs

@@ -54,7 +54,7 @@ impl TryFromObject for JsProperty {
 }
 
 impl JsProperty {
-    fn to_jsvalue(self) -> JsValue {
+    fn into_jsvalue(self) -> JsValue {
         match self {
             JsProperty::Str(s) => s.as_str().into(),
             JsProperty::Js(value) => value.value.clone(),
@@ -99,7 +99,7 @@ impl PyJsValue {
             } else if proto.value.is_null() {
                 Object::create(proto.value.unchecked_ref())
             } else {
-                return Err(vm.new_value_error(format!("prototype must be an Object or null")));
+                return Err(vm.new_value_error("prototype must be an Object or null".to_string()));
             }
         } else {
             Object::new()
@@ -109,12 +109,12 @@ impl PyJsValue {
 
     #[pymethod]
     fn has_prop(&self, name: JsProperty, vm: &VirtualMachine) -> PyResult<bool> {
-        has_prop(&self.value, &name.to_jsvalue()).map_err(|err| new_js_error(vm, err))
+        has_prop(&self.value, &name.into_jsvalue()).map_err(|err| new_js_error(vm, err))
     }
 
     #[pymethod]
     fn get_prop(&self, name: JsProperty, vm: &VirtualMachine) -> PyResult<PyJsValue> {
-        let name = &name.to_jsvalue();
+        let name = &name.into_jsvalue();
         if has_prop(&self.value, name).map_err(|err| new_js_error(vm, err))? {
             get_prop(&self.value, name)
                 .map(PyJsValue::new)
@@ -126,7 +126,8 @@ impl PyJsValue {
 
     #[pymethod]
     fn set_prop(&self, name: JsProperty, value: PyJsValueRef, vm: &VirtualMachine) -> PyResult<()> {
-        set_prop(&self.value, &name.to_jsvalue(), &value.value).map_err(|err| new_js_error(vm, err))
+        set_prop(&self.value, &name.into_jsvalue(), &value.value)
+            .map_err(|err| new_js_error(vm, err))
     }
 
     #[pymethod]
@@ -167,7 +168,7 @@ impl PyJsValue {
         let proto = opts
             .prototype
             .as_ref()
-            .and_then(|proto| proto.value.dyn_ref::<js_sys::Function>().clone());
+            .and_then(|proto| proto.value.dyn_ref::<js_sys::Function>());
         let js_args = Array::new();
         for arg in args {
             js_args.push(&arg.value);