diff options
author | Simon Sapin <simon.sapin@exyr.org> | 2019-09-29 23:11:40 +0200 |
---|---|---|
committer | Simon Sapin <simon.sapin@exyr.org> | 2019-09-29 23:11:40 +0200 |
commit | 47c269360cebba4857433bc2ef4dead70b42a01a (patch) | |
tree | 9351fb3fb7afaab852122b3aec4228c827568e54 | |
parent | 086e06b28b7722b3e268b846e6c507f1060a2931 (diff) | |
download | servo-47c269360cebba4857433bc2ef4dead70b42a01a.tar.gz servo-47c269360cebba4857433bc2ef4dead70b42a01a.zip |
Remove redundant webidl_must_inherit compiler plugin lint
At first I was considering moving it to a procedural macro
(source-level information should be sufficient),
and started by trying to reproduce the error case
by changing `htmldivelement.rs` to use `Element` instead of `HTMLElement`
as the first field.
The output was:
```rust
error[E0308]: mismatched types
--> /home/simon/servo2/target/debug/build/script-4caa244faca7d10f/out/Bindings/HTMLDivElementBinding.rs:665:31
|
665 | let _: &HTMLElement = self.as_parent();
| ^^^^^^^^^^^^^^^^ expected struct `dom::htmlelement::HTMLElement`, found struct `dom::element::Element`
|
= note: expected type `&dom::htmlelement::HTMLElement`
found type `&dom::element::Element`
```
This line number is inside a generated method called `__assert_parent_type`.
As far as I can tell, any case where this lint would error is already caught
by such methods. The lint is therefore redundant and can safely be removed.
-rw-r--r-- | Cargo.lock | 12 | ||||
-rw-r--r-- | components/dom_struct/lib.rs | 1 | ||||
-rw-r--r-- | components/script/Cargo.toml | 3 | ||||
-rw-r--r-- | components/script_plugins/Cargo.toml | 4 | ||||
-rw-r--r-- | components/script_plugins/lib.rs | 12 | ||||
-rw-r--r-- | components/script_plugins/webidl_must_inherit.rs | 210 |
6 files changed, 1 insertions, 241 deletions
diff --git a/Cargo.lock b/Cargo.lock index f019a379a12..bb75e1713ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3992,9 +3992,6 @@ dependencies = [ [[package]] name = "script_plugins" version = "0.0.1" -dependencies = [ - "weedle 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", -] [[package]] name = "script_plugins_tests" @@ -5674,14 +5671,6 @@ dependencies = [ ] [[package]] -name = "weedle" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "nom 4.1.1 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] name = "which" version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6316,7 +6305,6 @@ dependencies = [ "checksum webrender_build 0.0.1 (git+https://github.com/servo/webrender)" = "<none>" "checksum webxr 0.0.1 (git+https://github.com/servo/webxr)" = "<none>" "checksum webxr-api 0.0.1 (git+https://github.com/servo/webxr)" = "<none>" -"checksum weedle 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3bb43f70885151e629e2a19ce9e50bd730fd436cfd4b666894c9ce4de9141164" "checksum which 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "240a31163872f7e8e49f35b42b58485e35355b07eb009d9f3686733541339a69" "checksum winapi 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)" = "f10e386af2b13e47c89e7236a7a14a086791a2b88ebad6df9bf42040195cf770" "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" diff --git a/components/dom_struct/lib.rs b/components/dom_struct/lib.rs index f316a6f22bb..e7b2b290ce5 100644 --- a/components/dom_struct/lib.rs +++ b/components/dom_struct/lib.rs @@ -19,7 +19,6 @@ pub fn dom_struct(args: TokenStream, input: TokenStream) -> TokenStream { #[derive(DenyPublicFields, DomObject, JSTraceable, MallocSizeOf)] #[must_root] #[repr(C)] - #[webidl] }; // Work around https://github.com/rust-lang/rust/issues/46489 diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index 6eec808b8df..5c9fa86e95f 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -16,8 +16,7 @@ path = "lib.rs" debugmozjs = ['js/debugmozjs'] profilemozjs = ['js/profilemozjs'] unrooted_must_root_lint = ["script_plugins/unrooted_must_root_lint"] -webidl_lint = ["script_plugins/webidl_lint"] -default = ["unrooted_must_root_lint", "webidl_lint"] +default = ["unrooted_must_root_lint"] webgl_backtrace = ["backtrace", "canvas_traits/webgl_backtrace"] js_backtrace = ["backtrace"] uwp = ["js/uwp"] diff --git a/components/script_plugins/Cargo.toml b/components/script_plugins/Cargo.toml index f1537b655a0..c6bc3eb2250 100644 --- a/components/script_plugins/Cargo.toml +++ b/components/script_plugins/Cargo.toml @@ -11,7 +11,3 @@ plugin = true [features] unrooted_must_root_lint = [] -webidl_lint = [] - -[dependencies] -weedle = "0.10" diff --git a/components/script_plugins/lib.rs b/components/script_plugins/lib.rs index cdebd231092..aa45098befe 100644 --- a/components/script_plugins/lib.rs +++ b/components/script_plugins/lib.rs @@ -24,8 +24,6 @@ extern crate rustc; extern crate rustc_driver; extern crate syntax; -extern crate weedle; - use rustc_driver::plugin::Registry; use syntax::feature_gate::AttributeType::Whitelisted; use syntax::symbol::Symbol; @@ -33,9 +31,6 @@ use syntax::symbol::Symbol; #[cfg(feature = "unrooted_must_root_lint")] mod unrooted_must_root; -#[cfg(feature = "webidl_lint")] -mod webidl_must_inherit; - /// Utilities for writing plugins #[cfg(feature = "unrooted_must_root_lint")] mod utils; @@ -49,15 +44,9 @@ pub fn plugin_registrar(reg: &mut Registry) { symbols.clone(), ))); - #[cfg(feature = "webidl_lint")] - reg.register_late_lint_pass(Box::new(webidl_must_inherit::WebIdlPass::new( - symbols.clone(), - ))); - reg.register_attribute(symbols.allow_unrooted_interior, Whitelisted); reg.register_attribute(symbols.allow_unrooted_in_rc, Whitelisted); reg.register_attribute(symbols.must_root, Whitelisted); - reg.register_attribute(symbols.webidl, Whitelisted); } macro_rules! symbols { @@ -82,7 +71,6 @@ symbols! { allow_unrooted_interior allow_unrooted_in_rc must_root - webidl alloc rc Rc diff --git a/components/script_plugins/webidl_must_inherit.rs b/components/script_plugins/webidl_must_inherit.rs deleted file mode 100644 index 95d8ad386a9..00000000000 --- a/components/script_plugins/webidl_must_inherit.rs +++ /dev/null @@ -1,210 +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 https://mozilla.org/MPL/2.0/. */ - -use rustc::hir; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass}; -use rustc::ty; -use std::boxed; -use std::env; -use std::error::Error; -use std::fmt; -use std::fs; -use std::io; -use std::path; -use weedle; - -declare_lint!( - WEBIDL_INHERIT_CORRECT, - Deny, - "Warn and report usage of incorrect webidl inheritance" -); - -pub(crate) struct WebIdlPass { - symbols: crate::Symbols, -} - -#[derive(Clone, Debug)] -pub struct ParentMismatchError { - name: String, - rust_parent: String, - webidl_parent: String, -} - -impl fmt::Display for ParentMismatchError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "webidl-rust inheritance mismatch, rust: {:?}, rust parent: {:?}, webidl parent: {:?}", - self.name, self.rust_parent, self.webidl_parent - ) - } -} - -impl Error for ParentMismatchError { - fn description(&self) -> &str { - "ParentMismatchError" - } - - fn cause(&self) -> Option<&dyn Error> { - None - } -} - -impl WebIdlPass { - pub fn new(symbols: crate::Symbols) -> WebIdlPass { - WebIdlPass { symbols } - } -} - -fn get_ty_name(ty: &str) -> &str { - if let Some(i) = ty.rfind(':') { - &ty[i + 1..] - } else { - ty - } -} - -fn get_manifest_dir() -> io::Result<path::PathBuf> { - match env::var("CARGO_MANIFEST_DIR") { - Ok(var) => { - let mut dir = path::PathBuf::new(); - dir.push(var); - Ok(dir) - }, - Err(env::VarError::NotPresent) => Err(io::Error::new( - io::ErrorKind::NotFound, - "CARGO_MANIFEST_DIR environment variable was not found", - )), - Err(env::VarError::NotUnicode(_)) => Err(io::Error::new( - io::ErrorKind::InvalidData, - "CARGO_MANIFEST_DIR environment variable's contents are non valid UTF-8", - )), - } -} - -fn get_webidl_path(struct_name: &str) -> io::Result<path::PathBuf> { - let mut dir = get_manifest_dir()?; - dir.push("dom/webidls/"); - dir.push(format!("{}.webidl", struct_name)); - - Ok(dir) -} - -fn is_webidl_ty(symbols: &crate::Symbols, cx: &LateContext, ty: &ty::TyS) -> bool { - let mut ret = false; - ty.maybe_walk(|t| { - match t.kind { - ty::Adt(did, _substs) => { - if cx.tcx.has_attr(did.did, symbols.webidl) { - ret = true; - } - false - }, - ty::Ref(..) => false, // don't recurse down &ptrs - ty::RawPtr(..) => false, // don't recurse down *ptrs - ty::FnDef(..) | ty::FnPtr(_) => false, - _ => true, - } - }); - ret -} - -fn check_inherits(code: &str, name: &str, parent_name: &str) -> Result<(), Box<dyn Error>> { - let idl = weedle::parse(code).expect("Invalid webidl provided"); - let mut inherits = ""; - - for def in idl { - if let weedle::Definition::Interface(def) = def { - if let Some(parent) = def.inheritance { - inherits = parent.identifier.0; - break; - } - } else if let weedle::Definition::CallbackInterface(def) = def { - if let Some(parent) = def.inheritance { - inherits = parent.identifier.0; - break; - } - } - } - - if inherits == parent_name { - return Ok(()); - } - - // If there is no parent, first field must be of type Reflector. - if inherits == "" && parent_name == "Reflector" { - return Ok(()); - } - - if inherits == "" && - name == "PaintRenderingContext2D" && - parent_name == "CanvasRenderingContext2D" - { - // PaintRenderingContext2D embeds a CanvasRenderingContext2D - // instead of a Reflector as an optimization, - // but this is fine since CanvasRenderingContext2D - // also has a reflector - return Ok(()); - } - - Err(boxed::Box::from(ParentMismatchError { - name: name.to_string(), - rust_parent: parent_name.to_string(), - webidl_parent: inherits.to_string(), - })) -} - -fn check_webidl(name: &str, parent_name: &Option<String>) -> Result<(), Box<dyn Error>> { - let path = get_webidl_path(&name)?; - if let Some(parent) = parent_name { - let code = fs::read_to_string(path)?; - return check_inherits(&code, name, &parent); - } - - Ok(()) -} - -impl LintPass for WebIdlPass { - fn name(&self) -> &'static str { - "ServoWebIDLPass" - } - - fn get_lints(&self) -> LintArray { - lint_array!(WEBIDL_INHERIT_CORRECT) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for WebIdlPass { - fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) { - let def = match &item.kind { - hir::ItemKind::Struct(def, ..) => def, - _ => return, - }; - let id = item.hir_id; - let def_id = cx.tcx.hir().local_def_id(id); - if !is_webidl_ty(&self.symbols, cx, cx.tcx.type_of(def_id)) { - return; - } - - let item = match cx.tcx.hir().get(id) { - hir::Node::Item(item) => item, - _ => cx.tcx.hir().expect_item(cx.tcx.hir().get_parent_item(id)), - }; - - let parent_name = def.fields().iter().next().map(|field| { - let def_id = cx.tcx.hir().local_def_id(field.hir_id); - let ty = cx.tcx.type_of(def_id).to_string(); - get_ty_name(&ty).to_string() - }); - - let struct_name = item.ident.to_string(); - match check_webidl(&struct_name, &parent_name) { - Ok(()) => {}, - Err(e) => { - let description = format!("{}", e); - cx.span_lint(WEBIDL_INHERIT_CORRECT, item.ident.span, &description) - }, - }; - } -} |