Add unrooted_must_root lint for usages of JS<T> in let/for bindings

components/layout/lib.rs

@@ -8,6 +8,7 @@
 #![feature(globs, macro_rules, phase, thread_local, unsafe_destructor)]
 
 #![deny(unused_imports, unused_variable)]
+#![allow(unrooted_must_root)]
 
 #[phase(plugin, link)]
 extern crate log;

components/macros/lib.rs

@@ -85,7 +85,8 @@ impl LintPass for UnrootedPass {
     fn check_struct_def(&mut self, cx: &Context, def: &ast::StructDef, _i: ast::Ident, _gen: &ast::Generics, id: ast::NodeId) {
         if cx.tcx.map.expect_item(id).attrs.iter().all(|a| !a.check_name("must_root")) {
             for ref field in def.fields.iter() {
-                lint_unrooted_ty(cx, &*field.node.ty, "Type must be rooted, use #[must_root] on the struct definition to propagate");
+                lint_unrooted_ty(cx, &*field.node.ty,
+                                 "Type must be rooted, use #[must_root] on the struct definition to propagate");
             }
         }
     }
@@ -96,7 +97,8 @@ impl LintPass for UnrootedPass {
             match var.node.kind {
                 ast::TupleVariantKind(ref vec) => {
                     for ty in vec.iter() {
-                        lint_unrooted_ty(cx, &*ty.ty, "Type must be rooted, use #[must_root] on the enum definition to propagate")
+                        lint_unrooted_ty(cx, &*ty.ty,
+                                         "Type must be rooted, use #[must_root] on the enum definition to propagate")
                     }
                 }
                 _ => () // Struct variants already caught by check_struct_def
@@ -104,23 +106,55 @@ impl LintPass for UnrootedPass {
         }
     }
 
-    fn check_fn(&mut self, cx: &Context, kind: &syntax::visit::FnKind, decl: &ast::FnDecl, block: &ast::Block, _span: syntax::codemap::Span, _id: ast::NodeId) {
+    fn check_fn(&mut self, cx: &Context, kind: &syntax::visit::FnKind, decl: &ast::FnDecl,
+                block: &ast::Block, _span: syntax::codemap::Span, _id: ast::NodeId) {
         match *kind {
             syntax::visit::FkItemFn(i, _, _, _) |
             syntax::visit::FkMethod(i, _, _) if i.as_str() == "new" || i.as_str() == "new_inherited" => {
-
+                return;
             }
             _ => ()
         }
         match block.rules {
             ast::DefaultBlock => {
                 for arg in decl.inputs.iter() {
-                    lint_unrooted_ty(cx, &*arg.ty, "Type must be rooted, use #[must_root] on the struct definition to propagate")
+                    lint_unrooted_ty(cx, &*arg.ty,
+                                     "Type must be rooted, use #[must_root] on the struct definition to propagate")
                 }
             }
             _ => () // fn is `unsafe`
         }
     }
+
+    // Partially copied from rustc::middle::lint::builtin
+    // Catches `let` statements which store a #[must_root] value
+    // Expressions which return out of blocks eventually end up in a `let`
+    // statement or a function return (which will be caught when it is used elsewhere)
+    fn check_stmt(&mut self, cx: &Context, s: &ast::Stmt) {
+        // Catch the let binding
+        let expr = match s.node {
+            ast::StmtDecl(ref decl, _) => match decl.node {
+                ast::DeclLocal(ref loc) => match loc.init {
+                        Some(ref e) => &**e,
+                        _ => return
+                },
+                _ => return
+            },
+            _ => return
+        };
+
+        let t = expr_ty(cx.tcx, &*expr);
+        match ty::get(t).sty {
+            ty::ty_struct(did, _) |
+            ty::ty_enum(did, _) => {
+                if ty::has_attr(cx.tcx, did, "must_root") {
+                    cx.span_lint(UNROOTED_MUST_ROOT, expr.span,
+                                 format!("Expression of type {} must be rooted", t.repr(cx.tcx)).as_slice());
+                }
+            }
+            _ => {}
+        }
+    }
 }
 
 #[plugin_registrar]

components/script/dom/attr.rs

@@ -108,8 +108,8 @@ impl Attr {
     pub fn new(window: &JSRef<Window>, local_name: Atom, value: AttrValue,
                name: Atom, namespace: Namespace,
                prefix: Option<DOMString>, owner: &JSRef<Element>) -> Temporary<Attr> {
-        let attr = Attr::new_inherited(local_name, value, name, namespace, prefix, owner);
+        reflect_dom_object(box Attr::new_inherited(local_name, value, name, namespace, prefix, owner),
-        reflect_dom_object(box attr, &Window(*window), AttrBinding::Wrap)
+                           &Window(*window), AttrBinding::Wrap)
     }
 }
 

components/script/dom/bindings/callback.rs

@@ -139,6 +139,7 @@ pub struct CallSetup {
 
 impl CallSetup {
     /// Performs the setup needed to make a call.
+    #[allow(unrooted_must_root)]
     pub fn new<T: CallbackContainer>(callback: &T, handling: ExceptionHandling) -> CallSetup {
         let global = global_object_for_js_object(callback.callback());
         let global = global.root();

components/script/dom/bindings/utils.rs

@@ -668,6 +668,7 @@ pub extern fn outerize_global(_cx: *mut JSContext, obj: JSHandleObject) -> *mut
 }
 
 /// Returns the global object of the realm that the given JS object was created in.
+#[allow(unrooted_must_root)]
 pub fn global_object_for_js_object(obj: *mut JSObject) -> GlobalField {
     unsafe {
         let global = GetGlobalForObjectCrossCompartment(obj);
@@ -689,6 +690,7 @@ pub fn global_object_for_js_object(obj: *mut JSObject) -> GlobalField {
 
 /// Get the `JSContext` for the `JSRuntime` associated with the thread
 /// this object is on.
+#[allow(unrooted_must_root)]
 fn cx_for_dom_reflector(obj: *mut JSObject) -> *mut JSContext {
     let global = global_object_for_js_object(obj);
     let global = global.root();

components/script/dom/document.rs

@@ -310,8 +310,8 @@ impl Document {
     }
 
     pub fn new(window: &JSRef<Window>, url: Option<Url>, doctype: IsHTMLDocument, content_type: Option<DOMString>) -> Temporary<Document> {
-        let document = Document::new_inherited(window, url, doctype, content_type);
+        let document = reflect_dom_object(box Document::new_inherited(window, url, doctype, content_type),
-        let document = reflect_dom_object(box document, &Window(*window),
+                                          &Window(*window),
                                           DocumentBinding::Wrap).root();
 
         let node: &JSRef<Node> = NodeCast::from_ref(&*document);

components/script/dom/element.rs

@@ -220,6 +220,7 @@ pub trait LayoutElementHelpers {
 }
 
 impl LayoutElementHelpers for JS<Element> {
+    #[allow(unrooted_must_root)]
     unsafe fn html_element_in_html_document_for_layout(&self) -> bool {
         if (*self.unsafe_get()).namespace != namespace::HTML {
             return false

components/script/dom/formdata.rs

@@ -55,6 +55,7 @@ impl FormData {
 }
 
 impl<'a> FormDataMethods for JSRef<'a, FormData> {
+    #[allow(unrooted_must_root)]
     fn Append(&self, name: DOMString, value: &JSRef<Blob>, filename: Option<DOMString>) {
         let file = FileData(JS::from_rooted(&self.get_file_from_blob(value, filename)));
         self.data.deref().borrow_mut().insert_or_update_with(name.clone(), vec!(file.clone()),
@@ -86,7 +87,7 @@ impl<'a> FormDataMethods for JSRef<'a, FormData> {
     fn Has(&self, name: DOMString) -> bool {
         self.data.deref().borrow().contains_key_equiv(&name)
     }
-
+    #[allow(unrooted_must_root)]
     fn Set(&self, name: DOMString, value: &JSRef<Blob>, filename: Option<DOMString>) {
         let file = FileData(JS::from_rooted(&self.get_file_from_blob(value, filename)));
         self.data.deref().borrow_mut().insert(name, vec!(file));

components/script/dom/performance.rs

@@ -29,8 +29,8 @@ impl Performance {
     }
 
     pub fn new(window: &JSRef<Window>) -> Temporary<Performance> {
-        let performance = Performance::new_inherited(window);
+        reflect_dom_object(box Performance::new_inherited(window),
-        reflect_dom_object(box performance, &Window(*window),
+                           &Window(*window),
                            PerformanceBinding::Wrap)
     }
 }

components/script/dom/xmlhttprequest.rs

@@ -133,7 +133,7 @@ pub struct XMLHttpRequest {
 
 impl XMLHttpRequest {
     pub fn new_inherited(global: &GlobalRef) -> XMLHttpRequest {
-        let xhr = XMLHttpRequest {
+        XMLHttpRequest {
             eventtarget: XMLHttpRequestEventTarget::new_inherited(XMLHttpRequestTypeId),
             ready_state: Traceable::new(Cell::new(Unsent)),
             timeout: Traceable::new(Cell::new(0u32)),
@@ -163,8 +163,7 @@ impl XMLHttpRequest {
             fetch_time: Traceable::new(Cell::new(0)),
             timeout_pinned: Traceable::new(Cell::new(false)),
             terminate_sender: Untraceable::new(RefCell::new(None)),
-        };
+        }
-        xhr
     }
     pub fn new(global: &GlobalRef) -> Temporary<XMLHttpRequest> {
         reflect_dom_object(box XMLHttpRequest::new_inherited(global),

components/script/lib.rs

@@ -58,6 +58,7 @@ pub mod dom {
 
         /// Generated JS-Rust bindings.
         pub mod codegen {
+            #[allow(unrooted_must_root)]
             pub mod Bindings;
             pub mod InterfaceTypes;
             pub mod InheritTypes;