Add a Bevy Linter

.github/workflows/ci.yml

@@ -70,6 +70,39 @@ jobs:
         # See tools/ci/src/main.rs for the commands this runs
         run: cargo run -p ci
 
+  dylint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/cache@v2
+        id: cache
+        with:
+          path: |
+            ~/.cargo/bin/
+            ~/.cargo/registry/index/
+            ~/.cargo/registry/cache/
+            ~/.cargo/git/db/
+            ~/.dylint_drivers
+            target/
+            crates/bevy_dylint/target
+          key: ${{ runner.os }}-cargo-dylint-${{ hashFiles('**/Cargo.toml') }}
+      - name: Install alsa and udev
+        run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev
+      - name: Install dylint
+        run: cargo install cargo-dylint; cargo install dylint-link
+        if: steps.cache.outputs.cache-hit != 'true'
+      - name: Run dylint
+        run: cargo dylint --all
+      - name: Check Format
+        run: cd crates/bevy_dylint; cargo fmt --all -- --check
+      - name: Check Clippy
+        run: cd crates/bevy_dylint; cargo clippy --all-targets --all-features -- -D warnings -A clippy::type_complexity
+      - name: Run Tests
+        run: cd crates/bevy_dylint; cargo test
+        env:
+          # Can be removed once https://github.com/trailofbits/dylint/pull/48 is merged and released
+          CARGO_TERM_COLOR: never
+
   build-wasm:
     strategy:
       matrix:

Cargo.toml

@@ -16,9 +16,14 @@ readme = "README.md"
 repository = "https://github.com/bevyengine/bevy"
 
 [workspace]
-exclude = ["benches"]
+exclude = ["benches", "crates/bevy_dylint"]
 members = ["crates/*", "examples/ios", "tools/ci"]
 
+[workspace.metadata.dylint]
+libraries = [
+    { path = "crates/bevy_dylint" },
+]
+
 [features]
 default = [
   "bevy_audio",

crates/bevy_dylint/.cargo/config.toml

@@ -0,0 +1,8 @@
+[target.x86_64-apple-darwin]
+linker = "dylint-link"
+
+[target.x86_64-unknown-linux-gnu]
+linker = "dylint-link"
+
+[target.x86_64-pc-windows-msvc]
+linker = "dylint-link"

crates/bevy_dylint/Cargo.toml

@@ -0,0 +1,26 @@
+[package]
+name = "bevy_dylint"
+version = "0.1.0"
+authors = ["Bevy Contributors <bevyengine@gmail.com>"]
+description = "decription goes here"
+edition = "2018"
+publish = false
+
+[package.metadata.rust-analyzer]
+rustc_private=true
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", tag = "rust-1.54.0"}
+dylint_linting = "1.0.1"
+if_chain = "1.0.1"
+
+[dev-dependencies]
+dylint_testing = "1.0.1"
+bevy_ecs = { path = "../bevy_ecs"}
+
+[[example]]
+name = "unnecessary_with"
+path = "ui/unnecessary_with.rs"

crates/bevy_dylint/rust-toolchain

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "nightly-2021-06-03"
+components = ["llvm-tools-preview", "rustc-dev", "rustfmt", "clippy"]

crates/bevy_dylint/src/bevy_paths.rs

@@ -0,0 +1,5 @@
+pub const ADDED: &[&str] = &["bevy_ecs", "query", "filter", "Added"];
+pub const CHANGED: &[&str] = &["bevy_ecs", "query", "filter", "Changed"];
+pub const OR: &[&str] = &["bevy_ecs", "query", "filter", "Or"];
+pub const WITH: &[&str] = &["bevy_ecs", "query", "filter", "With"];
+pub const QUERY: &[&str] = &["bevy_ecs", "system", "query", "Query"];

crates/bevy_dylint/src/lib.rs

@@ -0,0 +1,39 @@
+#![feature(rustc_private)]
+//#![warn(unused_extern_crates)]
+
+dylint_linting::dylint_library!();
+
+extern crate rustc_ast;
+extern crate rustc_ast_pretty;
+extern crate rustc_attr;
+extern crate rustc_data_structures;
+extern crate rustc_errors;
+extern crate rustc_hir;
+extern crate rustc_hir_pretty;
+extern crate rustc_index;
+extern crate rustc_infer;
+extern crate rustc_lexer;
+extern crate rustc_lint;
+extern crate rustc_middle;
+extern crate rustc_mir;
+extern crate rustc_parse;
+extern crate rustc_parse_format;
+extern crate rustc_session;
+extern crate rustc_span;
+extern crate rustc_target;
+extern crate rustc_trait_selection;
+extern crate rustc_typeck;
+
+mod bevy_paths;
+mod unnecessary_with;
+
+#[no_mangle]
+pub fn register_lints(_sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) {
+    lint_store.register_lints(&[unnecessary_with::UNNECESSARY_WITH]);
+    lint_store.register_late_pass(|| Box::new(unnecessary_with::UnnecessaryWith));
+}
+
+#[test]
+fn ui() {
+    dylint_testing::ui_test_examples(env!("CARGO_PKG_NAME"));
+}

crates/bevy_dylint/src/unnecessary_with.rs

@@ -0,0 +1,223 @@
+use clippy_utils::diagnostics::span_lint;
+use if_chain::if_chain;
+use rustc_hir::{
+    def::Res, hir_id::HirId, intravisit::FnKind, Body, FnDecl, GenericArg, MutTy, Path, QPath, Ty,
+    TyKind,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::{def_id::DefId, symbol::Symbol, Span};
+
+use crate::bevy_paths;
+
+declare_lint! {
+    /// **What it does:**
+    /// Detectes unnecessary instances of the `With`
+    /// **Why is this bad?**
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// # use bevy_ecs::system::Query;
+    /// # use bevy_ecs::query::With;
+    /// fn system(query: Query<&A, With<A>>) {}
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use bevy_ecs::system::Query;
+    /// fn system(query: Query<&A>) {}
+    /// ```
+    pub UNNECESSARY_WITH,
+    Warn,
+    "description goes here"
+}
+
+declare_lint_pass!(UnnecessaryWith => [UNNECESSARY_WITH]);
+
+impl<'hir> LateLintPass<'hir> for UnnecessaryWith {
+    // A list of things you might check can be found here:
+    // https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/trait.LateLintPass.html
+
+    fn check_fn(
+        &mut self,
+        ctx: &LateContext<'hir>,
+        _: FnKind<'hir>,
+        decl: &'hir FnDecl<'hir>,
+        _: &'hir Body<'hir>,
+        _: Span,
+        _: HirId,
+    ) {
+        for typ in decl.inputs {
+            if_chain! {
+                if let TyKind::Path(QPath::Resolved(_, path)) = &typ.kind;
+                if path_matches_symbol_path(ctx, path, bevy_paths::QUERY);
+                if let Some(segment) = path.segments.iter().last();
+                if let Some(generic_args) = segment.args;
+                if let Some(GenericArg::Type(world)) = &generic_args.args.get(1);
+                if let Some(GenericArg::Type(filter)) = &generic_args.args.get(2);
+                then {
+                    check_for_overlap(ctx, world, filter);
+                }
+            }
+        }
+    }
+}
+
+fn check_for_overlap<'hir>(ctx: &LateContext<'hir>, world: &Ty, filter: &Ty) {
+    let mut required_types = Vec::new();
+    let mut with_types = Vec::new();
+
+    match &world.kind {
+        TyKind::Rptr(_, mut_type) => {
+            if let Some(def_id) = get_def_id_of_reference(&mut_type) {
+                required_types.push(def_id);
+            }
+        }
+        TyKind::Tup(types) => {
+            for typ in *types {
+                if let TyKind::Rptr(_, mut_type) = &typ.kind {
+                    if let Some(def_id) = get_def_id_of_reference(&mut_type) {
+                        required_types.push(def_id);
+                    }
+                }
+            }
+        }
+        _ => (),
+    }
+
+    match &filter.kind {
+        TyKind::Path(QPath::Resolved(_, path)) => {
+            if path_matches_symbol_path(ctx, path, bevy_paths::OR) {
+                with_types.extend(check_or_filter(ctx, path));
+            }
+            if path_matches_symbol_path(ctx, path, bevy_paths::WITH) {
+                if let Some(def_id) = get_def_id_of_first_generic_arg(path) {
+                    with_types.push((def_id, filter.span));
+                }
+            }
+        }
+        TyKind::Tup(types) => {
+            for typ in *types {
+                if let TyKind::Path(QPath::Resolved(_, path)) = typ.kind {
+                    if path_matches_symbol_path(ctx, path, bevy_paths::OR) {
+                        with_types.extend(check_or_filter(ctx, path));
+                    }
+                    if path_matches_symbol_path(ctx, path, bevy_paths::ADDED)
+                        || path_matches_symbol_path(ctx, path, bevy_paths::CHANGED)
+                    {
+                        if let Some(def_id) = get_def_id_of_first_generic_arg(path) {
+                            required_types.push(def_id);
+                        }
+                    }
+                    if path_matches_symbol_path(ctx, path, bevy_paths::WITH) {
+                        if let Some(def_id) = get_def_id_of_first_generic_arg(path) {
+                            with_types.push((def_id, typ.span));
+                        }
+                    }
+                }
+            }
+        }
+        _ => (),
+    }
+
+    for with_type in with_types {
+        if required_types.contains(&with_type.0) {
+            span_lint(
+                ctx,
+                UNNECESSARY_WITH,
+                with_type.1,
+                "Unnecessary `With` Filter",
+            );
+        }
+    }
+}
+
+fn get_def_id_of_reference(reference: &MutTy) -> Option<DefId> {
+    if let TyKind::Path(QPath::Resolved(_, path)) = reference.ty.kind {
+        if let Res::Def(_, def_id) = path.res {
+            return Some(def_id);
+        }
+    }
+
+    None
+}
+
+fn path_matches_symbol_path<'hir>(
+    ctx: &LateContext<'hir>,
+    path: &Path,
+    symbol_path: &[&str],
+) -> bool {
+    if let Res::Def(_, def_id) = path.res {
+        return ctx.match_def_path(
+            def_id,
+            symbol_path
+                .iter()
+                .map(|str| Symbol::intern(str))
+                .collect::<Vec<_>>()
+                .as_slice(),
+        );
+    };
+
+    false
+}
+
+fn get_def_id_of_first_generic_arg(path: &Path) -> Option<DefId> {
+    if let Some(segment) = path.segments.iter().last() {
+        if let Some(generic_args) = segment.args {
+            if let Some(GenericArg::Type(component)) = &generic_args.args.get(0) {
+                if let TyKind::Path(QPath::Resolved(_, path)) = component.kind {
+                    if let Res::Def(_, def_id) = path.res {
+                        return Some(def_id);
+                    }
+                }
+            }
+        }
+    }
+
+    None
+}
+
+fn check_or_filter<'hir>(ctx: &LateContext<'hir>, path: &Path) -> Vec<(DefId, Span)> {
+    let mut local_required_types = Vec::new();
+    let mut local_with_types = Vec::new();
+
+    if let Some(segment) = path.segments.iter().last() {
+        if let Some(generic_args) = segment.args {
+            if let GenericArg::Type(tuple) = &generic_args.args[0] {
+                if let TyKind::Tup(types) = tuple.kind {
+                    for typ in types {
+                        if let TyKind::Path(QPath::Resolved(_, path)) = typ.kind {
+                            if path_matches_symbol_path(ctx, path, bevy_paths::ADDED)
+                                || path_matches_symbol_path(ctx, path, bevy_paths::CHANGED)
+                            {
+                                if let Some(def_id) = get_def_id_of_first_generic_arg(path) {
+                                    local_required_types.push(def_id);
+                                }
+                            }
+                            if path_matches_symbol_path(ctx, path, bevy_paths::WITH) {
+                                if let Some(def_id) = get_def_id_of_first_generic_arg(path) {
+                                    local_with_types.push((def_id, typ.span));
+                                }
+                            }
+                        }
+                    }
+
+                    for with_type in &local_with_types {
+                        if local_required_types.contains(&with_type.0) {
+                            span_lint(
+                                ctx,
+                                UNNECESSARY_WITH,
+                                with_type.1,
+                                "Unnecessary `With` Filter",
+                            );
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    local_with_types
+}