Skip to content

Commit

Permalink
Polish linter diagnostics (#285)
Browse files Browse the repository at this point in the history
This makes a few small improvements to the diagnostics of the linter so
they're simpler, clearer, and more consistent. If you're reviewing, I
recommend going through each commit individually!

Part of #284.
  • Loading branch information
BD103 authored Feb 26, 2025
1 parent 3cb0224 commit f624b6a
Show file tree
Hide file tree
Showing 22 changed files with 90 additions and 69 deletions.
26 changes: 22 additions & 4 deletions bevy_lint/src/lints/borrowed_reborrowable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@
use std::ops::ControlFlow;

use crate::{declare_bevy_lint, declare_bevy_lint_pass};
use clippy_utils::{diagnostics::span_lint_and_sugg, ty::match_type};
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt, ty::match_type};
use rustc_errors::Applicability;
use rustc_hir::{Body, FnDecl, Mutability, intravisit::FnKind};
use rustc_hir::{Body, FnDecl, MutTy, Mutability, intravisit::FnKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{Interner, Ty, TyKind, TypeVisitable, TypeVisitor};
use rustc_span::{
Expand All @@ -113,7 +113,7 @@ use rustc_span::{
declare_bevy_lint! {
pub BORROWED_REBORROWABLE,
PEDANTIC,
"parameter takes a mutable reference to a re-borrowable type",
"function parameter takes a mutable reference to a re-borrowable type",
}

declare_bevy_lint_pass! {
Expand Down Expand Up @@ -184,13 +184,31 @@ impl<'tcx> LateLintPass<'tcx> for BorrowedReborrowable {

let span = decl.inputs[arg_index].span.to(arg_ident.span);

// This tries to get the user-written form of `T` given the HIR representation for `&T`
// / `&mut T`. If we cannot for whatever reason, we fallback to using
// `Ty::to_string()` to get the fully-qualified form of `T`.
//
// For example, given a function signature like `fn(&mut Commands)`, we try to get the
// snippet for just `Commands` but default to `bevy::prelude::Commands<'_, '_>` if we
// cannot.
let ty_snippet = match decl.inputs[arg_index].kind {
// The `Ty` should be a `Ref`, since we proved that above.
rustc_hir::TyKind::Ref(_, MutTy { ty: inner_ty, .. }) => {
// Get the snippet for the inner type.
snippet_opt(cx, inner_ty.span)
}
// If it's not a `Ref` for whatever reason, fallback to our default value.
_ => None,
}
.unwrap_or_else(|| ty.to_string());

span_lint_and_sugg(
cx,
BORROWED_REBORROWABLE.lint,
span,
reborrowable.message(),
reborrowable.help(),
reborrowable.suggest(arg_ident, ty.to_string()),
reborrowable.suggest(arg_ident, ty_snippet),
// Not machine-applicable since the function body may need to
// also be updated to account for the removed ref
Applicability::MaybeIncorrect,
Expand Down
7 changes: 2 additions & 5 deletions bevy_lint/src/lints/cargo/duplicate_bevy_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,11 @@ fn lint_with_target_version(
cx,
DUPLICATE_BEVY_DEPENDENCIES.lint,
toml_span(cargo_toml_reference.span(), file),
format!(
"Mismatching versions of `bevy` found, {} used bevy version {}",
mismatching_dependency.0, mismatching_dependency.1
),
DUPLICATE_BEVY_DEPENDENCIES.lint.desc,
|diag| {
diag.span_help(
bevy_cargo_toml_span,
format!("Expected all crates to use `bevy` {target_version}"),
format!("expected all crates to use `bevy` {target_version}, but `{}` uses `bevy` {}", mismatching_dependency.0, mismatching_dependency.1),
);
},
);
Expand Down
10 changes: 6 additions & 4 deletions bevy_lint/src/lints/insert_event_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ declare_bevy_lint_pass! {
},
}

const HELP_MESSAGE: &str = "inserting an `Events` resource does not fully setup that event";

impl<'tcx> LateLintPass<'tcx> for InsertEventResource {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
// Find a method call.
Expand Down Expand Up @@ -128,7 +130,7 @@ fn check_insert_resource(cx: &LateContext<'_>, method_call: &MethodCall) {
format!(
"called `App::insert_resource{generics_snippet}({receiver_snippet}, {args_snippet})` instead of `App::add_event::<{event_ty_snippet}>({receiver_snippet})`"
),
"inserting an `Events` resource does not fully setup that event",
HELP_MESSAGE,
format!("App::add_event::<{event_ty_snippet}>({receiver_snippet})"),
applicability,
);
Expand All @@ -140,7 +142,7 @@ fn check_insert_resource(cx: &LateContext<'_>, method_call: &MethodCall) {
format!(
"called `App::insert_resource{generics_snippet}({args_snippet})` instead of `App::add_event::<{event_ty_snippet}>()`"
),
"inserting an `Events` resource does not fully setup that event",
HELP_MESSAGE,
format!("add_event::<{event_ty_snippet}>()"),
applicability,
);
Expand Down Expand Up @@ -208,7 +210,7 @@ fn check_init_resource<'tcx>(cx: &LateContext<'tcx>, method_call: &MethodCall<'t
format!(
"called `App::init_resource{generics_snippet}({receiver_snippet})` instead of `App::add_event::<{event_ty_snippet}>({receiver_snippet})`"
),
"inserting an `Events` resource does not fully setup that event",
HELP_MESSAGE,
format!("App::add_event::<{event_ty_snippet}>({receiver_snippet})"),
applicability,
);
Expand All @@ -220,7 +222,7 @@ fn check_init_resource<'tcx>(cx: &LateContext<'tcx>, method_call: &MethodCall<'t
format!(
"called `App::init_resource{generics_snippet}({args_snippet})` instead of `App::add_event::<{event_ty_snippet}>()`"
),
"inserting an `Events` resource does not fully setup that event",
HELP_MESSAGE,
format!("add_event::<{event_ty_snippet}>()"),
applicability,
);
Expand Down
12 changes: 8 additions & 4 deletions bevy_lint/src/lints/zst_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ use rustc_middle::ty::{

declare_bevy_lint! {
pub ZST_QUERY,
// This will eventually be a `RESTRICTION` lint, but due to <https://github.com/TheBevyFlock/bevy_cli/issues/252>
// it is not yet ready for production.
// This will eventually be a `RESTRICTION` lint, but due to
// <https://github.com/TheBevyFlock/bevy_cli/issues/279> it is not yet ready for production.
NURSERY,
"query for a zero-sized type",
"queried a zero-sized type",
}

declare_bevy_lint_pass! {
Expand Down Expand Up @@ -136,7 +136,11 @@ impl QueryKind {
// In the future, we might want to span the usage site of `is_added()`/`is_changed()`
// and suggest to use `Added<Foo>`/`Changed<Foo>` instead.
match self {
Self::Query => format!("consider using a filter instead: `With<{ty}>`"),
Self::Query => format!(
// NOTE: This isn't actually true, please see #279 for more info and how this will
// be fixed!
"zero-sized types do not retrieve any data, consider using a filter instead: `With<{ty}>`"
),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: Mismatching versions of `bevy` found, leafwing-input-manager used bevy version ^0.13
error: multiple versions of the `bevy` crate found
--> Cargo.toml:12:26
|
12 | leafwing-input-manager = "0.13"
| ^^^^^^
|
help: Expected all crates to use `bevy` 0.14.2
help: expected all crates to use `bevy` 0.14.2, but `leafwing-input-manager` uses `bevy` ^0.13
--> Cargo.toml:11:8
|
11 | bevy = { version = "0.14.2" }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: Mismatching versions of `bevy` found, leafwing-input-manager used bevy version ^0.13
error: multiple versions of the `bevy` crate found
--> Cargo.toml:12:26
|
12 | leafwing-input-manager = "0.13"
| ^^^^^^
|
help: Expected all crates to use `bevy` 0.14.2
help: expected all crates to use `bevy` 0.14.2, but `leafwing-input-manager` uses `bevy` ^0.13
--> Cargo.toml:11:8
|
11 | bevy = "0.14.2"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: Mismatching versions of `bevy` found, leafwing-input-manager used bevy version ^0.13
error: multiple versions of the `bevy` crate found
--> Cargo.toml:12:26
|
12 | leafwing-input-manager = "0.13"
| ^^^^^^
|
help: Expected all crates to use `bevy` 0.14.2
help: expected all crates to use `bevy` 0.14.2, but `leafwing-input-manager` uses `bevy` ^0.13
--> Cargo.toml:11:8
|
11 | bevy = { version = "0.14.2", features = ["android_shared_stdcxx"] }
Expand Down
2 changes: 1 addition & 1 deletion bevy_lint/tests/ui/borrowed_reborrowable/closures.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut Commands` instead of a re-borrowed `Commands`
--> tests/ui/borrowed_reborrowable/closures.rs:15:20
|
15 | let closure = |commands: &mut Commands| {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut commands: bevy::prelude::Commands<'_, '_>`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut commands: Commands`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/closures.rs:5:9
Expand Down
4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/commands.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut Commands` instead of a re-borrowed `Commands`
--> tests/ui/borrowed_reborrowable/commands.rs:17:22
|
17 | fn mutable_reference(commands: &mut Commands) {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut commands: bevy::prelude::Commands<'_, '_>`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut commands: Commands`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/commands.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut Commands` instead of a re-borrowed `Commands`
--> tests/ui/borrowed_reborrowable/commands.rs:23:33
|
23 | fn mutable_reference_return<'a>(_commands: &'a mut Commands) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut _commands: bevy::prelude::Commands<'_, '_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Commands` instead: `mut _commands: Commands`

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/deferred.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut Deferred` instead of a re-borrowed `Deferred`
--> tests/ui/borrowed_reborrowable/deferred.rs:17:22
|
17 | fn mutable_reference(_param: &mut Deferred<CommandQueue>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Deferred` instead: `mut _param: bevy::prelude::Deferred<'_, bevy::bevy_ecs::world::CommandQueue>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Deferred` instead: `mut _param: Deferred<CommandQueue>`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/deferred.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut Deferred` instead of a re-borrowed `Deferred`
--> tests/ui/borrowed_reborrowable/deferred.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut Deferred<CommandQueue>) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Deferred` instead: `mut _param: bevy::prelude::Deferred<'_, bevy::bevy_ecs::world::CommandQueue>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Deferred` instead: `mut _param: Deferred<CommandQueue>`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut DeferredWorld` instead of a re-borrowed `DeferredWo
--> tests/ui/borrowed_reborrowable/deferred_world.rs:17:22
|
17 | fn mutable_reference(_param: &mut DeferredWorld) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `DeferredWorld` instead: `mut _param: bevy::bevy_ecs::world::DeferredWorld<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `DeferredWorld` instead: `mut _param: DeferredWorld`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/deferred_world.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut DeferredWorld` instead of a re-borrowed `DeferredWo
--> tests/ui/borrowed_reborrowable/deferred_world.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut DeferredWorld) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `DeferredWorld` instead: `mut _param: bevy::bevy_ecs::world::DeferredWorld<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `DeferredWorld` instead: `mut _param: DeferredWorld`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut EntityCommands` instead of a re-borrowed `EntityCom
--> tests/ui/borrowed_reborrowable/entity_commands.rs:17:22
|
17 | fn mutable_reference(_param: &mut EntityCommands) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityCommands` instead: `mut _param: bevy::prelude::EntityCommands<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityCommands` instead: `mut _param: EntityCommands`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/entity_commands.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut EntityCommands` instead of a re-borrowed `EntityCom
--> tests/ui/borrowed_reborrowable/entity_commands.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut EntityCommands) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityCommands` instead: `mut _param: bevy::prelude::EntityCommands<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityCommands` instead: `mut _param: EntityCommands`

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/entity_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut EntityMut` instead of a re-borrowed `EntityMut`
--> tests/ui/borrowed_reborrowable/entity_mut.rs:17:22
|
17 | fn mutable_reference(_param: &mut EntityMut) {
| ^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityMut` instead: `mut _param: bevy::prelude::EntityMut<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityMut` instead: `mut _param: EntityMut`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/entity_mut.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut EntityMut` instead of a re-borrowed `EntityMut`
--> tests/ui/borrowed_reborrowable/entity_mut.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut EntityMut) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityMut` instead: `mut _param: bevy::prelude::EntityMut<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `EntityMut` instead: `mut _param: EntityMut`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut FilteredEntityMut` instead of a re-borrowed `Filter
--> tests/ui/borrowed_reborrowable/filtered_entity_mut.rs:17:22
|
17 | fn mutable_reference(_param: &mut FilteredEntityMut) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `FilteredEntityMut` instead: `mut _param: bevy::bevy_ecs::world::FilteredEntityMut<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `FilteredEntityMut` instead: `mut _param: FilteredEntityMut`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/filtered_entity_mut.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut FilteredEntityMut` instead of a re-borrowed `Filter
--> tests/ui/borrowed_reborrowable/filtered_entity_mut.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut FilteredEntityMut) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `FilteredEntityMut` instead: `mut _param: bevy::bevy_ecs::world::FilteredEntityMut<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `FilteredEntityMut` instead: `mut _param: FilteredEntityMut`

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/mut_typed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut Mut` instead of a re-borrowed `Mut`
--> tests/ui/borrowed_reborrowable/mut_typed.rs:17:22
|
17 | fn mutable_reference(_param: &mut Mut<Name>) {
| ^^^^^^^^^^^^^^^^^^^^^^ help: use `Mut` instead: `mut _param: bevy::prelude::Mut<'_, bevy::prelude::Name>`
| ^^^^^^^^^^^^^^^^^^^^^^ help: use `Mut` instead: `mut _param: Mut<Name>`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/mut_typed.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut Mut` instead of a re-borrowed `Mut`
--> tests/ui/borrowed_reborrowable/mut_typed.rs:23:33
|
23 | fn mutable_reference_return<'a>(_param: &'a mut Mut<Name>) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Mut` instead: `mut _param: bevy::prelude::Mut<'_, bevy::prelude::Name>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `Mut` instead: `mut _param: Mut<Name>`

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/mut_untyped.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut MutUntyped` instead of a re-borrowed `MutUntyped`
--> tests/ui/borrowed_reborrowable/mut_untyped.rs:18:22
|
18 | fn mutable_reference(_param: &mut MutUntyped) {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `MutUntyped` instead: `mut _param: bevy::bevy_ecs::change_detection::MutUntyped<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `MutUntyped` instead: `mut _param: MutUntyped`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/mut_untyped.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut MutUntyped` instead of a re-borrowed `MutUntyped`
--> tests/ui/borrowed_reborrowable/mut_untyped.rs:24:33
|
24 | fn mutable_reference_return<'a>(_param: &'a mut MutUntyped) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `MutUntyped` instead: `mut _param: bevy::bevy_ecs::change_detection::MutUntyped<'_>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `MutUntyped` instead: `mut _param: MutUntyped`

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions bevy_lint/tests/ui/borrowed_reborrowable/non_send_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: parameter takes `&mut NonSendMut` instead of a re-borrowed `NonSendMut`
--> tests/ui/borrowed_reborrowable/non_send_mut.rs:19:22
|
19 | fn mutable_reference(_param: &mut NonSendMut<MyResource>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `NonSendMut` instead: `mut _param: bevy::prelude::NonSendMut<'_, MyResource>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `NonSendMut` instead: `mut _param: NonSendMut<MyResource>`
|
note: the lint level is defined here
--> tests/ui/borrowed_reborrowable/non_send_mut.rs:5:9
Expand All @@ -14,7 +14,7 @@ error: parameter takes `&mut NonSendMut` instead of a re-borrowed `NonSendMut`
--> tests/ui/borrowed_reborrowable/non_send_mut.rs:25:33
|
25 | fn mutable_reference_return<'a>(_param: &'a mut NonSendMut<MyResource>) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `NonSendMut` instead: `mut _param: bevy::prelude::NonSendMut<'_, MyResource>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `NonSendMut` instead: `mut _param: NonSendMut<MyResource>`

error: aborting due to 2 previous errors

Loading

0 comments on commit f624b6a

Please sign in to comment.