feat: switch to check_path based on zulip feedback

Author: DaAlbrecht

bevy_lint/src/lints/style/bevy_platform_alternative_exists.rs

@@ -1,9 +1,12 @@
-use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet, ty::ty_from_hir_ty};
+use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, source::snippet};
 use rustc_errors::Applicability;
-use rustc_hir::{QPath, TyKind};
+use rustc_hir::{
+    HirId, Path, PathSegment,
+    def::{DefKind, Res},
+};
 use rustc_lint::{LateContext, LateLintPass};
 
-use crate::{declare_bevy_lint, declare_bevy_lint_pass};
+use crate::{declare_bevy_lint, declare_bevy_lint_pass, sym, utils::hir_parse::generic_args_snippet};
 
 declare_bevy_lint! {
     pub(crate) BEVY_PLATFORM_ALTERNATIVE_EXISTS,
@@ -16,48 +19,70 @@ declare_bevy_lint_pass! {
 }
 
 impl<'tcx> LateLintPass<'tcx> for BevyPlatformAlternativeExists {
-    fn check_ty(
-        &mut self,
-        cx: &LateContext<'tcx>,
-        hir_ty: &'tcx rustc_hir::Ty<'tcx, rustc_hir::AmbigArg>,
-    ) {
-        if hir_ty.span.in_external_macro(cx.tcx.sess.source_map()) {
-            return;
-        }
-
-        let as_unambig_ty = hir_ty.as_unambig_ty();
-
-        // lower the [`hir::Ty`] to a [`rustc_middle::ty::Ty`]
-        let ty = ty_from_hir_ty(cx, as_unambig_ty);
+    fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
+        if let Res::Def(def_kind, def_id) = path.res 
+            // Retrieve the first path segment, this could look like: `bevy`, `std`, `serde`.
+            && let Some(first_segment) = get_first_segment(path)
+            // Skip if this span originates from an external macro.
+            //  Or likely originates from a proc_macro, note this should be called after
+            // `in_external_macro`.
+            && !path.span.in_external_macro(cx.tcx.sess.source_map())
+            && !is_from_proc_macro(cx, &first_segment.ident)
+            // Skip if this Definition is not originating from `std`.
+            && first_segment.ident.name == sym::std
+            // Get the def_id of the crate from first segment.
+            && let Res::Def(DefKind::Mod,crate_def_id) = first_segment.res
+            // If the first segment is not the crate root, then this type was checked when
+            // importing.
+            && crate_def_id.is_crate_root()
+            // Get potential generic arguments.
+            && let Some(generic_args) = path.segments.last().map(|s| generic_args_snippet(cx, s))
+        {
 
-        // Get the path to the type definition.
-        let TyKind::Path(QPath::Resolved(_, path)) = &as_unambig_ty.kind else {
-            return;
-        };
+            // Skip Resolutions that are modules for example: `use std::time`.
+            // This lint checks if a given Type from the `std` exists in `bevy_platform` and does not
+            // compare entire modules and getting the ty from a module DefId will result in a
+            // panic.
+            if DefKind::Mod == def_kind{
+                return;
+            }
 
-        //  if for the given `ty` an alternative from `bevy_platform` exists.
-        if let Some(bevy_platform_alternative) = BevyPlatformType::try_from_ty(cx, ty)
-        // Only emit a lint if the first segment of this path is `std` thus the type originates
-        // from the standart library. This prevents linting for `bevy::platform` types that are just a reexport of the `std`.
-        && path.segments.first().is_some_and(|segment| segment.ident.name.as_str().starts_with("std"))
-        {
-            span_lint_and_sugg(
-                cx,
-                BEVY_PLATFORM_ALTERNATIVE_EXISTS,
-                hir_ty.span,
-                BEVY_PLATFORM_ALTERNATIVE_EXISTS.desc,
-                format!(
-                    "the type `{}` can be replaced with the `no_std` compatible type {}",
-                    snippet(cx.tcx.sess, hir_ty.span, ""),
-                    bevy_platform_alternative.full_path()
-                ),
-                bevy_platform_alternative.full_path().to_string(),
-                Applicability::MachineApplicable,
-            );
+            // Get the Ty of this Definition.
+            let ty = cx.tcx.type_of(def_id).skip_binder();
+            //Check if an alternative exists in `bevy_platform`.
+            if let Some(bevy_platform_alternative) = BevyPlatformType::try_from_ty(cx, ty) {
+                span_lint_and_sugg(
+                    cx,
+                    BEVY_PLATFORM_ALTERNATIVE_EXISTS,
+                    path.span,
+                    BEVY_PLATFORM_ALTERNATIVE_EXISTS.desc,
+                    format!(
+                        "the type `{}` can be replaced with the `no_std` compatible type {}{}",
+                        snippet(cx.tcx.sess, path.span, ""),
+                        bevy_platform_alternative.full_path(),generic_args,
+                    ),
+                    format!("{}{}",bevy_platform_alternative.full_path(),generic_args),
+                    Applicability::MachineApplicable,
+                );
+            }
         }
     }
 }
 
+/// Returns the first named segment of a [`Path`].
+///
+/// If this is a global path (such as `::std::fmt::Debug`), then the segment after [`kw::PathRoot`]
+/// is returned.
+fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
+    match path.segments {
+        // A global path will have PathRoot as the first segment. In this case, return the segment
+        // after.
+        [x, y, ..] if x.ident.name == rustc_span::symbol::kw::PathRoot => Some(y),
+        [x, ..] => Some(x),
+        _ => None,
+    }
+}
+
 /// Creates an enum containing all the types form `bevy_platform` as variants.
 ///
 /// # Example
@@ -109,6 +134,7 @@ macro_rules! declare_bevy_platform_types {
 }
 
 declare_bevy_platform_types! {
+    Arc("sync") => ARC,
     Barrier("sync") => BARRIER,
     BarrierWaitResult("sync") => BARRIERWAITRESULT,
     HashMap("collections") => HASHMAP,

bevy_lint/src/paths.rs

@@ -94,6 +94,7 @@ pub mod bevy_platform_types {
 
     use crate::sym;
 
+    pub static ARC: PathLookup = type_path!(std::sync::Arc);
     pub static BARRIER: PathLookup = type_path!(std::sync::Barrier);
     pub static BARRIERWAITRESULT: PathLookup = type_path!(std::sync::BarrierWaitResult);
     pub static HASHMAP: PathLookup = type_path!(std::collections::HashMap);

bevy_lint/src/sym.rs

@@ -58,7 +58,7 @@ use clippy_utils::sym::EXTRA_SYMBOLS as CLIPPY_SYMBOLS;
 /// These are symbols that we use but are already interned by either the compiler or Clippy.
 pub use clippy_utils::sym::filter;
 pub use rustc_span::sym::{
-    HashMap, HashSet, Instant, Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard,
+    Arc, HashMap, HashSet, Instant, Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard,
     SyncUnsafeCell, bevy_ecs, bundle, message, plugin, reflect, std, sync,
 };
 use rustc_span::{Symbol, symbol::PREDEFINED_SYMBOLS_COUNT};

bevy_lint/tests/ui.rs

@@ -9,5 +9,6 @@ mod test_utils;
 
 fn main() {
     let config = base_config("ui").unwrap();
+
     run_tests(config).unwrap();
 }

bevy_lint/tests/ui/bevy_platform_alternative_exists/main.fixed

@@ -1,14 +1,7 @@
 #![feature(register_tool)]
 #![register_tool(bevy)]
 #![deny(bevy::bevy_platform_alternative_exists)]
-
-#[allow(dead_code)]
-struct Player {
-    attack_cd: bevy::platform::time::Instant,
-    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
-    //~| HELP: the type `std::time::Instant` can be replaced with the `no_std` compatible
-    // type bevy::platform::time::Instant
-}
+#![allow(dead_code)]
 
 fn main() {
     let mut hash_map = bevy::platform::collections::HashMap::new();
@@ -17,12 +10,46 @@ fn main() {
     // type bevy::platform::collections::HashMap
     hash_map.insert("foo", "bar");
 
-    let _time = bevy::platform::time::Instant::now();
+    // compatible type bevy::platform::collections::HashMap
+    //~| HELP: the type `std::collections::HashMap` can be replaced with the `no_std`
+    //~v ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    let _declared_hash_map: bevy::platform::collections::HashMap::<u32, u32> = bevy::platform::collections::HashMap::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashMap<u32, u32>` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashMap<u32, u32>
+
+    let _arc = bevy::platform::sync::Arc::new(10);
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::Arc` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::Arc
+
+    let barrier = bevy::platform::sync::Barrier::new(10);
     //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
-    //~| HELP: the type `std::time::Instant` can be replaced with the `no_std` compatible
-    // type bevy::platform::time::Instant
+    //~| HELP: the type `std::sync::Barrier` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::Barrier
 
-    // This should be fine even tho it will result in a `std::time::Instant` after full path
-    // resolution.
-    let _bevy_time = bevy::platform::time::Instant::now();
+    let _barrier_wait_result: bevy::platform::sync::BarrierWaitResult = barrier.wait();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::BarrierWaitResult` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::BarrierWaitResult
+
+    let mut hash_set = bevy::platform::collections::HashSet::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashSet` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashSet
+
+    hash_set.insert(1);
+
+    // compatible type bevy::platform::collections::HashSet
+    //~| HELP: the type `std::collections::HashSet` can be replaced with the `no_std`
+    //~v ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    let _declared_hash_map: bevy::platform::collections::HashSet::<u32> = bevy::platform::collections::HashSet::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashSet<u32>` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashSet<u32>
+
+    let _lazy = bevy::platform::sync::LazyLock::new(|| "lazy");
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::LazyLock` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::LazyLock
 }

bevy_lint/tests/ui/bevy_platform_alternative_exists/main.rs

@@ -1,16 +1,7 @@
 #![feature(register_tool)]
 #![register_tool(bevy)]
 #![deny(bevy::bevy_platform_alternative_exists)]
-
-use std::time;
-
-#[allow(dead_code)]
-struct Player {
-    attack_cd: time::Instant,
-    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
-    //~| HELP: the type `std::time::Instant` can be replaced with the `no_std` compatible
-    // type bevy::platform::time::Instant
-}
+#![allow(dead_code)]
 
 fn main() {
     let mut hash_map = std::collections::HashMap::new();
@@ -19,12 +10,46 @@ fn main() {
     // type bevy::platform::collections::HashMap
     hash_map.insert("foo", "bar");
 
-    let _time = std::time::Instant::now();
+    // compatible type bevy::platform::collections::HashMap
+    //~| HELP: the type `std::collections::HashMap` can be replaced with the `no_std`
+    //~v ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    let _declared_hash_map: std::collections::HashMap<u32, u32> = std::collections::HashMap::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashMap<u32, u32>` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashMap<u32, u32>
+
+    let _arc = std::sync::Arc::new(10);
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::Arc` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::Arc
+
+    let barrier = std::sync::Barrier::new(10);
     //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
-    //~| HELP: the type `std::time::Instant` can be replaced with the `no_std` compatible
-    // type bevy::platform::time::Instant
+    //~| HELP: the type `std::sync::Barrier` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::Barrier
 
-    // This should be fine even tho it will result in a `std::time::Instant` after full path
-    // resolution.
-    let _bevy_time = bevy::platform::time::Instant::now();
+    let _barrier_wait_result: std::sync::BarrierWaitResult = barrier.wait();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::BarrierWaitResult` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::BarrierWaitResult
+
+    let mut hash_set = std::collections::HashSet::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashSet` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashSet
+
+    hash_set.insert(1);
+
+    // compatible type bevy::platform::collections::HashSet
+    //~| HELP: the type `std::collections::HashSet` can be replaced with the `no_std`
+    //~v ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    let _declared_hash_map: std::collections::HashSet<u32> = std::collections::HashSet::new();
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::collections::HashSet<u32>` can be replaced with the `no_std`
+    // compatible type bevy::platform::collections::HashSet<u32>
+
+    let _lazy = std::sync::LazyLock::new(|| "lazy");
+    //~^ ERROR: Used type from the `std` that has an existing alternative from `bevy_platform`
+    //~| HELP: the type `std::sync::LazyLock` can be replaced with the `no_std`
+    // compatible type bevy::platform::sync::LazyLock
 }