servo: Merge #4488 - Add a lint for usages of to_string() (from servo:into_string-lint); r=jdm

This is a rebase of #4366.

Source-Repo: https://github.com/servo/servo
Source-Revision: 0da57abec6014f8a6edde9781598053deab996c7

servo/components/plugins/lib.rs

@@ -48,6 +48,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box lints::UnrootedPass as LintPassObject);
     reg.register_lint_pass(box lints::PrivatizePass as LintPassObject);
     reg.register_lint_pass(box lints::InheritancePass as LintPassObject);
+    reg.register_lint_pass(box lints::StrToStringPass as LintPassObject);
 }
 
 

servo/components/plugins/lints.rs

@@ -21,6 +21,8 @@ declare_lint!(PRIVATIZE, Deny,
               "Allows to enforce private fields for struct definitions")
 declare_lint!(INHERITANCE_INTEGRITY, Deny,
               "Ensures that struct fields are properly laid out for inheritance to work")
+declare_lint!(STR_TO_STRING, Deny,
+              "Warn when a String could use into_string() instead of to_string()")
 
 /// Lint for auditing transmutes
 ///
@@ -51,6 +53,11 @@ pub struct PrivatizePass;
 /// which itself is a DOM struct (in which case it must be the first field).
 pub struct InheritancePass;
 
+/// Prefer str.into_string() over str.to_string()
+///
+/// The latter creates a `Formatter` and is 5x slower than the former
+pub struct StrToStringPass;
+
 impl LintPass for TransmutePass {
     fn get_lints(&self) -> LintArray {
         lint_array!(TRANSMUTE_TYPE_LINT)
@@ -332,3 +339,34 @@ impl LintPass for InheritancePass {
         }
     }
 }
+
+impl LintPass for StrToStringPass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(STR_TO_STRING)
+    }
+
+    fn check_expr(&mut self, cx: &Context, expr: &ast::Expr) {
+        match expr.node {
+            ast::ExprMethodCall(ref method, _, ref args)
+                if method.node.as_str() == "to_string"
+                && is_str(cx, &*args[0]) => {
+                cx.span_lint(STR_TO_STRING, expr.span,
+                             "str.into_string() is more efficient than str.to_string(), please use it instead");
+            },
+            _ => ()
+        }
+
+        fn is_str(cx: &Context, expr: &ast::Expr) -> bool {
+            fn walk_ty<'t>(ty: ty::t) -> ty::t {
+                match ty::get(ty).sty {
+                    ty::ty_ptr(ref tm) | ty::ty_rptr(_, ref tm) => walk_ty(tm.ty),
+                    _ => ty
+                }
+            }
+            match ty::get(walk_ty(expr_ty(cx.tcx, expr))).sty {
+                ty::ty_str => true,
+                _ => false
+            }
+        }
+    }
+}

servo/components/script/dom/cssstyledeclaration.rs

@@ -39,10 +39,10 @@ macro_rules! css_properties(
     ( $([$getter:ident, $setter:ident, $cssprop:expr]),* ) => (
         $(
             fn $getter(self) -> DOMString {
-                self.GetPropertyValue($cssprop.to_string())
+                self.GetPropertyValue($cssprop.into_string())
             }
             fn $setter(self, value: DOMString) {
-                self.SetPropertyValue($cssprop.to_string(), value).unwrap();
+                self.SetPropertyValue($cssprop.into_string(), value).unwrap();
             }
         )*
     );
@@ -123,7 +123,7 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
             }
         });
 
-        result.unwrap_or("".to_string())
+        result.unwrap_or("".into_string())
     }
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
@@ -145,7 +145,7 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
                 // Step 2.2.2 & 2.2.3
                 match declaration {
                     Some(declaration) => list.push(declaration),
-                    None => return "".to_string(),
+                    None => return "".into_string(),
                 }
             }
 
@@ -157,7 +157,7 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
         if let Some(ref declaration) = self.get_declaration(&property) {
             serialize_value(declaration)
         } else {
-            "".to_string()
+            "".into_string()
         }
     }
 
@@ -174,15 +174,15 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
                                   .map(|longhand| self.GetPropertyPriority(longhand.clone()))
                                   .all(|priority| priority.as_slice() == "important") {
 
-                return "important".to_string();
+                return "important".into_string();
             }
         // Step 3
         } else if self.get_important_declaration(&property).is_some() {
-            return "important".to_string();
+            return "important".into_string();
         }
 
         // Step 4
-        "".to_string()
+        "".into_string()
     }
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty
@@ -287,7 +287,7 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setpropertyvalue
     fn SetPropertyValue(self, property: DOMString, value: DOMString) -> ErrorResult {
-        self.SetProperty(property, value, "".to_string())
+        self.SetProperty(property, value, "".into_string())
     }
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty
@@ -326,12 +326,12 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat
     fn CssFloat(self) -> DOMString {
-        self.GetPropertyValue("float".to_string())
+        self.GetPropertyValue("float".into_string())
     }
 
     // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat
     fn SetCssFloat(self, value: DOMString) -> ErrorResult {
-        self.SetPropertyValue("float".to_string(), value)
+        self.SetPropertyValue("float".into_string(), value)
     }
 
     fn IndexedGetter(self, index: u32, found: &mut bool) -> DOMString {

servo/components/script/dom/htmlelement.rs

@@ -167,7 +167,7 @@ impl<'a> HTMLElementCustomAttributeHelpers for JSRef<'a, HTMLElement> {
         let element: JSRef<Element> = ElementCast::from_ref(self);
         element.get_attribute(ns!(""), &Atom::from_slice(to_snake_case(name).as_slice())).map(|attr| {
             let attr = attr.root();
-            attr.value().as_slice().to_string()
+            attr.value().as_slice().into_string()
         })
     }
 

servo/components/script/dom/htmlscriptelement.rs

@@ -210,7 +210,7 @@ impl<'a> HTMLScriptElementHelpers for JSRef<'a, HTMLScriptElement> {
         window.evaluate_script_with_result(source.as_slice(), url.serialize().as_slice());
 
         let event = Event::new(GlobalRef::Window(*window),
-                               "load".to_string(),
+                               "load".into_string(),
                                EventBubbles::DoesNotBubble,
                                EventCancelable::NotCancelable).root();
         event.set_trusted(true);

servo/components/script/script_task.rs

@@ -1199,7 +1199,7 @@ impl ScriptTask {
                       let y = point.y.to_i32().unwrap_or(0);
 
                       let mouse_event = MouseEvent::new(*window,
-                          "mousemove".to_string(),
+                          "mousemove".into_string(),
                           true,
                           true,
                           Some(*window),

servo/components/style/properties/mod.rs.mako

@@ -2484,7 +2484,7 @@ impl<T: Show> DeclaredValue<T> {
         match self {
             &DeclaredValue::SpecifiedValue(ref inner) => Some(format!("{}", inner)),
             &DeclaredValue::Initial => None,
-            &DeclaredValue::Inherit => Some("inherit".to_string()),
+            &DeclaredValue::Inherit => Some("inherit".into_string()),
         }
     }
 }
@@ -2509,10 +2509,10 @@ impl PropertyDeclaration {
         match self {
             % for property in LONGHANDS:
                 % if property.derived_from is None:
-                    &PropertyDeclaration::${property.camel_case}Declaration(..) => "${property.name}".to_string(),
+                    &PropertyDeclaration::${property.camel_case}Declaration(..) => "${property.name}".into_string(),
                 % endif
             % endfor
-            _ => "".to_string(),
+            _ => "".into_string(),
         }
     }
 
@@ -3201,7 +3201,7 @@ pub fn longhands_from_shorthand(shorthand: &str) -> Option<Vec<String>> {
         % for property in SHORTHANDS:
             "${property.name}" => Some(vec!(
             % for sub in property.sub_properties:
-                "${sub.name}".to_string(),
+                "${sub.name}".into_string(),
             % endfor
             )),
         % endfor

servo/components/util/opts.rs

@@ -297,7 +297,7 @@ pub fn from_cmdline_args(args: &[String]) -> bool {
         }
     };
 
-    let render_api = match opt_match.opt_str("r").unwrap_or("gl".to_string()).as_slice() {
+    let render_api = match opt_match.opt_str("r").unwrap_or("gl".into_string()).as_slice() {
         "mesa" => RenderApi::Mesa,
         "gl" => RenderApi::OpenGL,
         _ => {

servo/components/util/rtinstrument.rs

@@ -83,7 +83,7 @@ pub fn instrument(f: proc()) {
             let task = Local::borrow(None::<Task>);
             match task.name {
                 Some(ref name) => name.to_string(),
-                None => "unknown".to_string(),
+                None => "unknown".into_string(),
             }
         };
         let stats = TaskStats {

servo/components/util/smallvec.rs

@@ -529,34 +529,34 @@ pub mod tests {
     #[test]
     pub fn test_inline() {
         let mut v = SmallVec16::new();
-        v.push("hello".to_string());
-        v.push("there".to_string());
-        assert_eq!(v.as_slice(), vec!["hello".to_string(), "there".to_string()].as_slice());
+        v.push("hello".into_string());
+        v.push("there".into_string());
+        assert_eq!(v.as_slice(), vec!["hello".into_string(), "there".into_string()].as_slice());
     }
 
     #[test]
     pub fn test_spill() {
         let mut v = SmallVec2::new();
-        v.push("hello".to_string());
-        v.push("there".to_string());
-        v.push("burma".to_string());
-        v.push("shave".to_string());
-        assert_eq!(v.as_slice(), vec!["hello".to_string(), "there".to_string(), "burma".to_string(), "shave".to_string()].as_slice());
+        v.push("hello".into_string());
+        v.push("there".into_string());
+        v.push("burma".into_string());
+        v.push("shave".into_string());
+        assert_eq!(v.as_slice(), vec!["hello".into_string(), "there".into_string(), "burma".into_string(), "shave".into_string()].as_slice());
     }
 
     #[test]
     pub fn test_double_spill() {
         let mut v = SmallVec2::new();
-        v.push("hello".to_string());
-        v.push("there".to_string());
-        v.push("burma".to_string());
-        v.push("shave".to_string());
-        v.push("hello".to_string());
-        v.push("there".to_string());
-        v.push("burma".to_string());
-        v.push("shave".to_string());
+        v.push("hello".into_string());
+        v.push("there".into_string());
+        v.push("burma".into_string());
+        v.push("shave".into_string());
+        v.push("hello".into_string());
+        v.push("there".into_string());
+        v.push("burma".into_string());
+        v.push("shave".into_string());
         assert_eq!(v.as_slice(), vec![
-            "hello".to_string(), "there".to_string(), "burma".to_string(), "shave".to_string(), "hello".to_string(), "there".to_string(), "burma".to_string(), "shave".to_string(),
+            "hello".into_string(), "there".into_string(), "burma".into_string(), "shave".into_string(), "hello".into_string(), "there".into_string(), "burma".into_string(), "shave".into_string(),
         ].as_slice());
     }
 }

servo/components/util/str.rs

@@ -16,10 +16,11 @@ pub type StaticCharVec = &'static [char];
 pub type StaticStringVec = &'static [&'static str];
 
 pub fn null_str_as_empty(s: &Option<DOMString>) -> DOMString {
-    // We don't use map_default because it would allocate "".to_string() even for Some.
+    // We don't use map_default because it would allocate "".into_string() even
+    // for Some.
     match *s {
         Some(ref s) => s.clone(),
-        None => "".to_string()
+        None => "".into_string()
     }
 }
 

servo/components/util/task.rs

@@ -42,7 +42,7 @@ pub fn spawn_named_with_send_on_failure<T: Send>(name: &'static str,
         TaskBuilder::new().named(name).try_future(with_state)
     };
 
-    let watched_name = name.to_string();
+    let watched_name = name.into_string();
     let watcher_name = format!("{}Watcher", watched_name);
     TaskBuilder::new().named(watcher_name).spawn(proc() {
         rtinstrument::instrument(proc() {

servo/ports/cef/browser.rs

@@ -107,7 +107,7 @@ fn browser_host_create(window_info: &cef_window_info_t,
                        callback_executed: bool)
                        -> CefBrowser {
     let mut urls = Vec::new();
-    urls.push("http://s27.postimg.org/vqbtrolyr/servo.jpg".to_string());
+    urls.push("http://s27.postimg.org/vqbtrolyr/servo.jpg".into_string());
     let mut opts = opts::default_opts();
     opts.urls = urls;
     let browser = ServoCefBrowser::new(window_info, client).as_cef_interface();

servo/ports/cef/core.rs

@@ -76,7 +76,7 @@ pub extern "C" fn cef_initialize(args: *const cef_main_args_t,
 
     message_queue.replace(Some(RefCell::new(Vec::new())));
 
-    let urls = vec![HOME_URL.to_string()];
+    let urls = vec![HOME_URL.into_string()];
     opts::set_opts(opts::Opts {
         urls: urls,
         n_paint_threads: 1,