servo: Merge #15649 - Rewrite the ban-type lint in Python (from zimio:issue-15591-rewrite-ban-type-lint); r=Wafflespeanut
authorJefry Lagrange <jefry.reyes@gmail.com>
Sun, 26 Feb 2017 09:35:12 -0800
changeset 489969 a5ffcb2625fd75c3c06691cc1daaf4ab3c1c4c6a
parent 489968 e066948f0339eb8a4dd98b437f1e0124e9d2a6e4
child 489970 8e332422e57c39ee72bc7493875fad72f23dc640
push id46964
push userbmo:mjzffr@gmail.com
push dateMon, 27 Feb 2017 13:49:06 +0000
reviewersWafflespeanut
milestone54.0a1
servo: Merge #15649 - Rewrite the ban-type lint in Python (from zimio:issue-15591-rewrite-ban-type-lint); r=Wafflespeanut <!-- Please describe your changes on the following line: --> Rewrite the ban-type lint in Python. Question: Should the old ban-type lint written in rust be deleted? --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x ] `./mach build -d` does not report any errors - [x ] `./mach test-tidy` does not report any errors - [x ] These changes fix #15591 <!-- Either: --> - [ x] There are tests for these changes <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 261df34ced0bdcb8126994c8653ac101d1172085
servo/components/script_plugins/ban.rs
servo/components/script_plugins/lib.rs
servo/python/tidy/servo_tidy/tidy.py
servo/python/tidy/servo_tidy_tests/ban-domrefcell.rs
servo/python/tidy/servo_tidy_tests/ban.rs
servo/python/tidy/servo_tidy_tests/test_tidy.py
servo/tests/compiletest/plugin/compile-fail/ban-domrefcell.rs
servo/tests/compiletest/plugin/compile-fail/ban.rs
deleted file mode 100644
--- a/servo/components/script_plugins/ban.rs
+++ /dev/null
@@ -1,53 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-use rustc::lint::{EarlyContext, LintPass, LintArray, EarlyLintPass, LintContext};
-use syntax::ast::Ty;
-use utils::match_ty_unwrap;
-
-declare_lint!(BANNED_TYPE, Deny,
-              "Ban various unsafe type combinations");
-
-/// Lint for banning various unsafe types
-///
-/// Banned types:
-///
-/// - `Cell<JSVal>`
-/// - `Cell<JS<T>>`
-pub struct BanPass;
-
-impl LintPass for BanPass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(BANNED_TYPE)
-    }
-}
-
-impl EarlyLintPass for BanPass {
-    fn check_ty(&mut self, cx: &EarlyContext, ty: &Ty) {
-        if match_ty_unwrap(ty, &["std", "cell", "Cell"])
-            .and_then(|t| t.get(0))
-            .and_then(|t| match_ty_unwrap(&**t, &["dom", "bindings", "js", "JS"]))
-            .is_some() {
-            cx.span_lint(BANNED_TYPE, ty.span, "Banned type Cell<JS<T>> detected. Use MutJS<JS<T>> instead")
-        }
-        if match_ty_unwrap(ty, &["std", "cell", "Cell"])
-            .and_then(|t| t.get(0))
-            .and_then(|t| match_ty_unwrap(&**t, &["js", "jsval", "JSVal"]))
-            .is_some() {
-            cx.span_lint(BANNED_TYPE, ty.span, "Banned type Cell<JSVal> detected. Use MutJS<JSVal> instead")
-        }
-        if match_ty_unwrap(ty, &["dom", "bindings", "cell", "DOMRefCell"])
-            .and_then(|t| t.get(0))
-            .and_then(|t| match_ty_unwrap(&**t, &["dom", "bindings", "js", "JS"]))
-            .is_some() {
-            cx.span_lint(BANNED_TYPE, ty.span, "Banned type DOMRefCell<JS<T>> detected. Use MutJS<JS<T>> instead")
-        }
-        if match_ty_unwrap(ty, &["dom", "bindings", "cell", "DOMRefCell"])
-            .and_then(|t| t.get(0))
-            .and_then(|t| match_ty_unwrap(&**t, &["js", "jsapi", "Heap"]))
-            .is_some() {
-            cx.span_lint(BANNED_TYPE, ty.span, "Banned type DOMRefCell<Heap<T>> detected. Use Heap<T> directly instead")
-        }
-    }
-}
--- a/servo/components/script_plugins/lib.rs
+++ b/servo/components/script_plugins/lib.rs
@@ -20,20 +20,18 @@
 #[macro_use]
 extern crate rustc;
 extern crate rustc_plugin;
 extern crate syntax;
 
 use rustc_plugin::Registry;
 use syntax::feature_gate::AttributeType::Whitelisted;
 
-mod ban;
 mod unrooted_must_root;
 /// Utilities for writing plugins
 mod utils;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box unrooted_must_root::UnrootedPass::new());
-    reg.register_early_lint_pass(box ban::BanPass);
     reg.register_attribute("allow_unrooted_interior".to_string(), Whitelisted);
     reg.register_attribute("must_root".to_string(), Whitelisted);
 }
--- a/servo/python/tidy/servo_tidy/tidy.py
+++ b/servo/python/tidy/servo_tidy/tidy.py
@@ -544,16 +544,22 @@ def check_rust(file_name, lines):
                 lambda match, line: not re.match(r'^(pub )?use', line)),
             # ignore cases like "{}", "`{", "{{" and "use::std::{Foo, Bar}"
             (r"[^`]{[^\s{}]", "missing space after {{",
                 lambda match, line: not re.match(r'^(pub )?use', line)),
             # There should not be any extra pointer dereferencing
             (r": &Vec<", "use &[T] instead of &Vec<T>", no_filter),
             # No benefit over using &str
             (r": &String", "use &str instead of &String", no_filter),
+            # There should be any use of banned types:
+            # Cell<JSVal>, Cell<JS<T>>, DOMRefCell<JS<T>>, DOMRefCell<HEAP<T>>
+            (r"(\s|:)+Cell<JSVal>", "Banned type Cell<JSVal> detected. Use MutJS<JSVal> instead", no_filter),
+            (r"(\s|:)+Cell<JS<.+>>", "Banned type Cell<JS<T>> detected. Use MutJS<JS<T>> instead", no_filter),
+            (r"DOMRefCell<JS<.+>>", "Banned type DOMRefCell<JS<T>> detected. Use MutJS<JS<T>> instead", no_filter),
+            (r"DOMRefCell<Heap<.+>>", "Banned type DOMRefCell<Heap<T>> detected. Use MutJS<JS<T>> instead", no_filter),
             # No benefit to using &Root<T>
             (r": &Root<", "use &T instead of &Root<T>", no_filter),
             (r"^&&", "operators should go at the end of the first line", no_filter),
             (r"\{[A-Za-z0-9_]+\};", "use statement contains braces for single import",
                 lambda match, line: line.startswith('use ')),
             (r"^\s*else {", "else braces should be on the same line", no_filter),
             (r"[^$ ]\([ \t]", "extra space after (", no_filter),
             # This particular pattern is not reentrant-safe in script_thread.rs
rename from servo/tests/compiletest/plugin/compile-fail/ban-domrefcell.rs
rename to servo/python/tidy/servo_tidy_tests/ban-domrefcell.rs
rename from servo/tests/compiletest/plugin/compile-fail/ban.rs
rename to servo/python/tidy/servo_tidy_tests/ban.rs
--- a/servo/tests/compiletest/plugin/compile-fail/ban.rs
+++ b/servo/python/tidy/servo_tidy_tests/ban.rs
@@ -4,15 +4,18 @@
 
 #![feature(plugin)]
 #![plugin(script_plugins)]
 
 extern crate js;
 
 use js::jsval::JSVal;
 use std::cell::Cell;
+use std::cell::UnsafeCell;
 
 struct Foo {
-    bar: Cell<JSVal>
+    bar: Cell<JSVal>,
     //~^ ERROR Banned type Cell<JSVal> detected. Use MutJS<JSVal> instead
+    foo: UnsafeCell<JSVal>
+    //~^ NOT AN ERROR
 }
 
 fn main() {}
--- a/servo/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py
@@ -135,16 +135,24 @@ class CheckTidiness(unittest.TestCase):
         feature_errors = tidy.collect_errors_for_files(iterFile('lib.rs'), [], [tidy.check_rust], print_text=False)
 
         self.assertTrue('feature attribute is not in alphabetical order' in feature_errors.next()[2])
         self.assertTrue('feature attribute is not in alphabetical order' in feature_errors.next()[2])
         self.assertTrue('feature attribute is not in alphabetical order' in feature_errors.next()[2])
         self.assertTrue('feature attribute is not in alphabetical order' in feature_errors.next()[2])
         self.assertNoMoreErrors(feature_errors)
 
+        ban_errors = tidy.collect_errors_for_files(iterFile('ban.rs'), [], [tidy.check_rust], print_text=False)
+        self.assertEqual('Banned type Cell<JSVal> detected. Use MutJS<JSVal> instead', ban_errors.next()[2])
+        self.assertNoMoreErrors(ban_errors)
+
+        ban_errors = tidy.collect_errors_for_files(iterFile('ban-domrefcell.rs'), [], [tidy.check_rust], print_text=False)
+        self.assertEqual('Banned type DOMRefCell<JS<T>> detected. Use MutJS<JS<T>> instead', ban_errors.next()[2])
+        self.assertNoMoreErrors(ban_errors)
+
     def test_spec_link(self):
         tidy.SPEC_BASE_PATH = base_path
         errors = tidy.collect_errors_for_files(iterFile('speclink.rs'), [], [tidy.check_spec], print_text=False)
         self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_script_thread(self):
         errors = tidy.collect_errors_for_files(iterFile('script_thread.rs'), [], [tidy.check_rust], print_text=False)