From 583175bbe1d5f97e08f288f7c5e5a352718f1548 Mon Sep 17 00:00:00 2001
From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
Date: Mon, 9 May 2022 13:36:04 +0000
Subject: [PATCH] Make derived SystemParam readonly if possible (#4650)

Required for https://github.com/bevyengine/bevy/pull/4402.

# Objective

- derived `SystemParam` implementations were never `ReadOnlySystemParamFetch`
- We want them to be, e.g. for `EventReader`

## Solution

- If possible, 'forward' the impl of `ReadOnlySystemParamFetch`.
---
 crates/bevy_ecs/macros/src/lib.rs             |  5 +++-
 crates/bevy_ecs/src/event.rs                  | 14 +++++++++++
 .../tests/ui/system_param_derive_readonly.rs  | 25 +++++++++++++++++++
 .../ui/system_param_derive_readonly.stderr    | 25 +++++++++++++++++++
 4 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.rs
 create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr

diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs
index 0f57f448a18e7c..3273faf9ab1a22 100644
--- a/crates/bevy_ecs/macros/src/lib.rs
+++ b/crates/bevy_ecs/macros/src/lib.rs
@@ -324,7 +324,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
                             }
                             Ok(())
                         })
-                        .expect("Invalid 'render_resources' attribute format.");
+                        .expect("Invalid 'system_param' attribute format.");
 
                         attributes
                     }),
@@ -420,6 +420,9 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
                     }
                 }
             }
+
+            // Safety: The `ParamState` is `ReadOnlySystemParamFetch`, so this can only read from the `World`
+            unsafe impl<TSystemParamState: #path::system::SystemParamState + #path::system::ReadOnlySystemParamFetch, #punctuated_generics> #path::system::ReadOnlySystemParamFetch for FetchState <TSystemParamState, #punctuated_generic_idents> #where_clause {}
         };
     })
 }
diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs
index b28e036c4060c4..0ed437deedd093 100644
--- a/crates/bevy_ecs/src/event.rs
+++ b/crates/bevy_ecs/src/event.rs
@@ -513,6 +513,8 @@ impl<E: Event> std::iter::Extend<E> for Events<E> {
 
 #[cfg(test)]
 mod tests {
+    use crate::{prelude::World, system::SystemState};
+
     use super::*;
 
     #[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -753,4 +755,16 @@ mod tests {
             vec![EmptyTestEvent::default()]
         );
     }
+
+    #[test]
+    fn ensure_reader_readonly() {
+        fn read_for<E: Resource>() {
+            let mut world = World::new();
+            world.init_resource::<Events<E>>();
+            let mut state = SystemState::<EventReader<E>>::new(&mut world);
+            // This can only work if EventReader only reads the world
+            let _reader = state.get(&world);
+        }
+        read_for::<EmptyTestEvent>();
+    }
 }
diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.rs
new file mode 100644
index 00000000000000..048a1557693f8d
--- /dev/null
+++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.rs
@@ -0,0 +1,25 @@
+use bevy_ecs::prelude::*;
+use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
+
+#[derive(Component)]
+struct Foo;
+
+#[derive(SystemParam)]
+struct Mutable<'w, 's> {
+    a: Query<'w, 's, &'static mut Foo>,
+}
+
+fn main() {
+    // Ideally we'd use:
+    // let mut world = World::default();
+    // let state = SystemState::<Mutable>::new(&mut world);
+    // state.get(&world);
+    // But that makes the test show absolute paths
+    assert_readonly::<Mutable>();
+}
+
+fn assert_readonly<P: SystemParam>()
+where
+    <P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
+{
+}
diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr
new file mode 100644
index 00000000000000..870e2fb3131c53
--- /dev/null
+++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/system_param_derive_readonly.stderr
@@ -0,0 +1,25 @@
+warning: unused import: `SystemState`
+ --> tests/ui/system_param_derive_readonly.rs:2:63
+  |
+2 | use bevy_ecs::system::{ReadOnlySystemParamFetch, SystemParam, SystemState};
+  |                                                               ^^^^^^^^^^^
+  |
+  = note: `#[warn(unused_imports)]` on by default
+
+error[E0277]: the trait bound `for<'x> WriteFetch<'x, Foo>: ReadOnlyFetch` is not satisfied
+  --> tests/ui/system_param_derive_readonly.rs:18:5
+   |
+18 |     assert_readonly::<Mutable>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'x> ReadOnlyFetch` is not implemented for `WriteFetch<'x, Foo>`
+   |
+   = note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `QueryState<&'static mut Foo>`
+   = note: 2 redundant requirements hidden
+   = note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `_::FetchState<(QueryState<&'static mut Foo>,)>`
+note: required by a bound in `assert_readonly`
+  --> tests/ui/system_param_derive_readonly.rs:23:32
+   |
+21 | fn assert_readonly<P: SystemParam>()
+   |    --------------- required by a bound in this
+22 | where
+23 |     <P as SystemParam>::Fetch: ReadOnlySystemParamFetch,
+   |                                ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_readonly`