Deprecate using msg_send! without comma between arguments

Author: madsmtm

crates/objc2/CHANGELOG.md

@@ -15,6 +15,30 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: `AnyClass::verify_sel` now take more well-defined types
   `EncodeArguments` and  `EncodeReturn`.
 
+### Deprecated
+* Soft deprecated using `msg_send!` without a comma between arguments (i.e.
+  deprecated when the `"unstable-msg-send-always-comma"` feature is enabled).
+
+  See the following for an example of how to upgrade:
+  ```rust
+  // Before
+  let _: NSInteger = msg_send![
+      obj,
+      addTrackingRect:rect
+      owner:obj
+      userData:ptr::null_mut::<c_void>()
+      assumeInside:Bool::NO
+  ];
+  // After
+  let _: NSInteger = msg_send![
+      obj,
+      addTrackingRect: rect,
+      owner: obj,
+      userData: ptr::null_mut::<c_void>(),
+      assumeInside: false,
+  ];
+  ```
+
 
 ## 0.4.1 - 2023-07-31
 

crates/objc2/Cargo.toml

@@ -42,6 +42,9 @@ verify = ["malloc"]
 # to objc2.
 relax-void-encoding = []
 
+# Enable deprecation of using `msg_send!` without a comma between arguments.
+unstable-msg-send-always-comma = []
+
 # Expose features that require linking to `libc::free`.
 #
 # This is not enabled by default because most users won't need it, and it

crates/objc2/src/macros/__msg_send_parse.rs

@@ -78,7 +78,6 @@ macro_rules! __msg_send_parse {
     };
 
     // Handle calls without comma between `selector: argument` pair.
-    // TODO: Deprecate this
     {
         ($out_macro:path)
         @($error_fn:ident)
@@ -87,7 +86,17 @@ macro_rules! __msg_send_parse {
         @()
         @($($selector:ident : $argument:expr)*)
         $($macro_args:tt)*
-    } => {
+    } => {{
+        $crate::__comma_between_args!(
+            @($(
+                ", ",
+                stringify!($selector),
+                ": ",
+                stringify!($argument),
+            )+)
+            $($macro_args)*
+        );
+
         $crate::__msg_send_parse! {
             ($out_macro)
             @($error_fn)
@@ -96,5 +105,79 @@ macro_rules! __msg_send_parse {
             @($($selector : $argument),*)
             $($macro_args)*
         }
+    }};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(not(feature = "unstable-msg-send-always-comma"))]
+macro_rules! __comma_between_args {
+    ($($args:tt)*) => {};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(feature = "unstable-msg-send-always-comma")]
+macro_rules! __comma_between_args {
+    (
+        @__output
+        @($($args:tt)*)
+        @($macro_name:literal)
+    ) => {
+        #[deprecated = concat!(
+            "using ", $macro_name, "! without a comma between arguments is ",
+            "technically not valid macro syntax, and may break in a future ",
+            "version of Rust. You should use the following instead:\n",
+            $macro_name, "![", $($args)* "]"
+        )]
+        #[inline]
+        fn __msg_send_missing_comma() {}
+        __msg_send_missing_comma();
+    };
+    (
+        @($($args:tt)*)
+        @(__send_super_message_static)
+        @($obj:expr)
+    ) => {
+        $crate::__comma_between_args! {
+            @__output
+            @(stringify!(super($obj)), $($args)*)
+            @("msg_send")
+        }
+    };
+    (
+        @($($args:tt)*)
+        @(send_super_message)
+        @($obj:expr, $superclass:expr)
+    ) => {
+        $crate::__comma_between_args! {
+            @__output
+            @(stringify!(super($obj, $superclass)), $($args)*)
+            @("msg_send")
+        }
+    };
+    (
+        @($($args:tt)*)
+        @(send_message)
+        @($obj:expr)
+    ) => {
+        $crate::__comma_between_args! {
+            @__output
+            @(stringify!($obj), $($args)*)
+            @("msg_send")
+        }
+    };
+    // Catch-all for msg_send_id!
+    (
+        @($($args:tt)*)
+        @($fn:ident)
+        @($obj:expr)
+        @()
+    ) => {
+        $crate::__comma_between_args! {
+            @__output
+            @(stringify!($obj), $($args)*)
+            @("msg_send_id")
+        }
     };
 }

crates/objc2/src/macros/mod.rs

@@ -725,9 +725,11 @@ macro_rules! __class_inner {
 ///
 /// # Specification
 ///
-/// The syntax is similar to the message syntax in Objective-C, except with
-/// an (optional, though consider that deprecated) comma between arguments,
-/// since that works much better with rustfmt.
+/// The syntax is somewhat similar to the message syntax in Objective-C,
+/// except with a comma between arguments. Eliding the comma is possible,
+/// but it is soft-deprecated, and will be fully deprecated in a future
+/// release. The deprecation can be tried out with the
+/// `"unstable-msg-send-always-comma"` feature flag.
 ///
 /// The first expression, know as the "receiver", can be any type that
 /// implements [`MessageReceiver`], like a reference or a pointer to an

crates/objc2/tests/use_macros.rs

@@ -34,6 +34,7 @@ pub fn test_msg_send_comma_handling(obj: &MyObject, superclass: &AnyClass) {
         let _: () = msg_send![obj, a,];
         let _: () = msg_send![obj, a: 32i32];
         let _: () = msg_send![obj, a: 32i32,];
+        #[cfg_attr(feature = "unstable-msg-send-always-comma", allow(deprecated))]
         let _: () = msg_send![obj, a: 32i32 b: 32i32];
         let _: () = msg_send![obj, a: 32i32, b: 32i32];
         let _: () = msg_send![obj, a: 32i32, b: 32i32,];
@@ -44,6 +45,7 @@ pub fn test_msg_send_comma_handling(obj: &MyObject, superclass: &AnyClass) {
         let _: () = msg_send![super(obj, superclass), a,];
         let _: () = msg_send![super(obj, superclass), a: 32i32];
         let _: () = msg_send![super(obj, superclass), a: 32i32,];
+        #[cfg_attr(feature = "unstable-msg-send-always-comma", allow(deprecated))]
         let _: () = msg_send![super(obj, superclass), a: 32i32 b: 32i32];
         let _: () = msg_send![super(obj, superclass), a: 32i32, b: 32i32];
         let _: () = msg_send![super(obj, superclass), a: 32i32, b: 32i32,];

crates/test-ui/Cargo.toml

@@ -21,6 +21,7 @@ default = [
     "icrate/Foundation_NSArray",
     "icrate/Foundation_NSMutableArray",
     "icrate/Foundation_NSValue",
+    "objc2/unstable-msg-send-always-comma",
 ]
 std = ["block2/std", "objc2/std", "icrate/std"]
 

crates/test-ui/src/main.rs

@@ -2,7 +2,7 @@
 //!
 //! Use as:
 //! ```
-//! TRYBUILD=overwrite cargo +nightly run --features=run --bin test-ui
+//! TRYBUILD=overwrite RUSTFLAGS="--deny=warnings" cargo +nightly run --features=run --bin test-ui
 //! ```
 //!
 //! These are not part of the normal test suite, because trybuild doesn't pass

crates/test-ui/ui/msg_send_missing_comma.rs

@@ -0,0 +1,16 @@
+//! Test msg_send! syntax with missing commas.
+use icrate::Foundation::NSString;
+use objc2::rc::Id;
+use objc2::{msg_send, msg_send_bool, msg_send_id, ClassType};
+
+fn main() {
+    let obj: &NSString = &NSString::new();
+
+    let _: () = unsafe { msg_send![super(obj), a:obj b:obj] };
+    let _: () = unsafe { msg_send![super(obj, NSString::class()), a:obj b:obj] };
+    let _: () = unsafe { msg_send![obj, a:obj b:obj] };
+
+    unsafe { msg_send_bool![obj, c:obj d:obj] };
+
+    let _: Id<NSString> = unsafe { msg_send_id![obj, e:obj f:obj] };
+}

crates/test-ui/ui/msg_send_missing_comma.stderr

@@ -0,0 +1,58 @@
+error: use of deprecated macro `msg_send_bool`: use a normal msg_send! instead, it will perform the conversion for you
+ --> ui/msg_send_missing_comma.rs
+  |
+  |     unsafe { msg_send_bool![obj, c:obj d:obj] };
+  |              ^^^^^^^^^^^^^
+  |
+  = note: `-D deprecated` implied by `-D warnings`
+
+error: use of deprecated macro `objc2::msg_send_bool`: use a normal msg_send! instead, it will perform the conversion for you
+ --> ui/msg_send_missing_comma.rs
+  |
+  | use objc2::{msg_send, msg_send_bool, msg_send_id, ClassType};
+  |                       ^^^^^^^^^^^^^
+
+error: use of deprecated function `main::__msg_send_missing_comma`: using msg_send! without a comma between arguments is technically not valid macro syntax, and may break in a future version of Rust. You should use the following instead:
+       msg_send![super(obj), a: obj, b: obj]
+ --> ui/msg_send_missing_comma.rs
+  |
+  |     let _: () = unsafe { msg_send![super(obj), a:obj b:obj] };
+  |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: this error originates in the macro `$crate::__comma_between_args` which comes from the expansion of the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: use of deprecated function `main::__msg_send_missing_comma`: using msg_send! without a comma between arguments is technically not valid macro syntax, and may break in a future version of Rust. You should use the following instead:
+       msg_send![super(obj, NSString::class()), a: obj, b: obj]
+  --> ui/msg_send_missing_comma.rs
+   |
+   |     let _: () = unsafe { msg_send![super(obj, NSString::class()), a:obj b:obj] };
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in the macro `$crate::__comma_between_args` which comes from the expansion of the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: use of deprecated function `main::__msg_send_missing_comma`: using msg_send! without a comma between arguments is technically not valid macro syntax, and may break in a future version of Rust. You should use the following instead:
+       msg_send![obj, a: obj, b: obj]
+  --> ui/msg_send_missing_comma.rs
+   |
+   |     let _: () = unsafe { msg_send![obj, a:obj b:obj] };
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in the macro `$crate::__comma_between_args` which comes from the expansion of the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: use of deprecated function `main::__msg_send_missing_comma`: using msg_send! without a comma between arguments is technically not valid macro syntax, and may break in a future version of Rust. You should use the following instead:
+       msg_send![obj, c: obj, d: obj]
+  --> ui/msg_send_missing_comma.rs
+   |
+   |     unsafe { msg_send_bool![obj, c:obj d:obj] };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in the macro `$crate::__comma_between_args` which comes from the expansion of the macro `msg_send_bool` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: use of deprecated function `main::__msg_send_missing_comma`: using msg_send_id! without a comma between arguments is technically not valid macro syntax, and may break in a future version of Rust. You should use the following instead:
+       msg_send_id![obj, e: obj, f: obj]
+  --> ui/msg_send_missing_comma.rs
+   |
+   |     let _: Id<NSString> = unsafe { msg_send_id![obj, e:obj f:obj] };
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in the macro `$crate::__comma_between_args` which comes from the expansion of the macro `msg_send_id` (in Nightly builds, run with -Z macro-backtrace for more info)