Merge pull request #802 from godot-rust/qol/small-cleanup

Typos + code reordering

.github/workflows/release-version.yml

@@ -179,7 +179,7 @@ jobs:
 #        # Note: first block was for an alternative approach, where a separate `releases` branch tracks deployments.
 #        # Such a branch would however be disjoint and require quite a bit of extra space in the repo.
 #        run: |
-#          # Backup current repo, so we can checkout.
+#          # Backup current repo, so we can check out.
 #          mkdir -p /tmp/repo
 #          rsync -av --exclude .git --exclude target ./ /tmp/repo/
 #          git fetch origin releases && git switch releases || git switch --orphan releases

godot-bindings/src/godot_version.rs

@@ -90,7 +90,7 @@ fn test_godot_versions() {
         ("4.0.rc1.official.8843d9ad3", 4, 0, 0, "rc1", s("8843d9ad3")),
         ("4.0.stable.arch_linux", 4, 0, 0, "stable", None),
         ("4.1.1.stable.arch_linux\n", 4, 1, 1, "stable", None),
-        // Output from 4.0.stable on MacOS in debug mode:
+        // Output from 4.0.stable on macOS in debug mode:
         // https://github.com/godotengine/godot/issues/74906
         ("arguments
 0: /Users/runner/work/_temp/godot_bin/godot.macos.editor.dev.x86_64

godot-cell/src/blocking_cell.rs

@@ -15,7 +15,7 @@ use crate::blocking_guards::{MutGuardBlocking, RefGuardBlocking};
 use crate::cell::GdCellInner;
 use crate::guards::InaccessibleGuard;
 
-/// Blocking version of [`panicking::GdCell`](crate::panicking::GdCell) for multi-threaded usage.
+/// Blocking version of [`panicking::GdCell`](crate::panicking::GdCell) for multithreaded usage.
 ///
 /// This version of GdCell blocks the current thread if it does not yet hold references to the cell.
 ///

godot-cell/src/cell.rs

@@ -121,7 +121,7 @@ impl<T> GdCellInner<T> {
         let value = state.get_ptr();
 
         // SAFETY: `increment_mut` succeeded, therefore any existing mutable references are inaccessible.
-        // Additionally no new references can be created, unless the returned guard is made inaccessible.
+        // Additionally, no new references can be created, unless the returned guard is made inaccessible.
         //
         // This is the case because the only way for a new `GdMut` or `GdRef` to be made after this is for
         // either this guard to be dropped or `make_inaccessible` to be called and succeed.
@@ -170,10 +170,9 @@ impl<T> GdCellInner<T> {
     }
 }
 
-// SAFETY: `T` is sync so we can return references to it on different threads, it is also send so we can return
-// mutable references to it on different threads.
-// Additionally all internal state is synchronized via a mutex, so we wont have race conditions when trying
-// to use it from multiple threads.
+// SAFETY: `T` is Sync, so we can return references to it on different threads.
+// It is also Send, so we can return mutable references to it on different threads.
+// Additionally, all internal state is synchronized via a mutex, so we won't have race conditions when trying to use it from multiple threads.
 unsafe impl<T: Send + Sync> Sync for GdCellInner<T> {}
 
 /// Mutable state of the `GdCell`, bundled together to make it easier to avoid deadlocks when locking the
@@ -227,7 +226,7 @@ impl<T> CellState<T> {
         NonNull::new(self.ptr).unwrap()
     }
 
-    /// Push a pointer to this state..
+    /// Push a pointer to this state.
     pub(crate) fn push_ptr(&mut self, new_ptr: NonNull<T>) -> usize {
         self.ptr = new_ptr.as_ptr();
         self.stack_depth += 1;

godot-cell/src/guards.rs

@@ -196,7 +196,7 @@ impl<'a, T> Drop for MutGuard<'a, T> {
 /// creation. When the guard is dropped, `state`'s pointer is reset to the original pointer.
 ///
 /// This ensures that any new references are derived from the new reference we pass in, and when this guard
-/// is dropped the state is reset to what it was before, as if this guard never existed.
+/// is dropped, it resets the state to what it was before, as if this guard never existed.
 #[derive(Debug)]
 pub struct InaccessibleGuard<'a, T> {
     state: &'a Mutex<CellState<T>>,

godot-cell/src/lib.rs

@@ -25,10 +25,10 @@
 //! reference (or other pointer) to some value is considered accessible when it is possible to either read
 //! from or write to the value it points to without using `unsafe`. Importantly, if we know that a reference
 //! `a` is inaccessible, and then we create a new reference `b` derived from `a` to the same value, then we
-//! know for sure that `b` wont alias `a`. This is because aliasing in rust is based on accesses, and if we
+//! know for sure that `b` won't alias `a`. This is because aliasing in rust is based on accesses, and if we
 //! never access `a` then we cannot ever violate aliasing for `a` and `b`. And since `b` is derived from `a`
 //! (that is, `b` was created from `a` somehow such as by casting `a` to a raw pointer then to a reference
-//! `b`), then `a` wont get invalidated by accesses to `b`.
+//! `b`), then `a` won't get invalidated by accesses to `b`.
 
 mod blocking_cell;
 mod blocking_guards;

godot-cell/tests/mock/blocking.rs

@@ -32,7 +32,7 @@ impl MyClass {
 
 /// Call each method from different threads, allowing them to run in parallel.
 ///
-/// This should not cause borrow failures and should not lead to dead locks.
+/// This should not cause borrow failures and should not lead to deadlocks.
 #[test]
 fn calls_parallel() {
     use std::thread;
@@ -61,7 +61,7 @@ fn calls_parallel() {
 
 /// Call each method from different threads, allowing them to run in parallel.
 ///
-/// This should not cause borrow failures and should not lead to dead locks.
+/// This should not cause borrow failures and should not lead to deadlocks.
 ///
 /// Runs each method several times in a row. This should reduce the non-determinism that comes from
 /// scheduling of threads.
@@ -95,7 +95,7 @@ fn calls_parallel_many_serial() {
 
 /// Call each method from different threads, allowing them to run in parallel.
 ///
-/// This should not cause borrow failures and should not lead to dead locks.
+/// This should not cause borrow failures and should not lead to deadlocks.
 ///
 /// Runs all the tests several times. This is different from [`calls_parallel_many_serial`] as that calls the
 /// methods like AAA...BBB...CCC..., whereas this interleaves the methods like ABC...ABC...ABC...

godot-cell/tests/mock/panicking.rs

@@ -83,7 +83,7 @@ fn calls_different_thread() {
 /// Call each method from different threads, allowing them to run in parallel.
 ///
 /// This may cause borrow failures, we do a best-effort attempt at estimating the value then. We can detect
-/// if the first call failed, so then we know the integer was incremented by 0. Otherwise we at least know
+/// if the first call failed, so then we know the integer was incremented by 0. Otherwise, we at least know
 /// the range of values that it can be incremented by.
 #[test]
 fn calls_parallel() {
@@ -114,7 +114,7 @@ fn calls_parallel() {
 /// Call each method from different threads, allowing them to run in parallel.
 ///
 /// This may cause borrow failures, we do a best-effort attempt at estimating the value then. We can detect
-/// if the first call failed, so then we know the integer was incremented by 0. Otherwise we at least know
+/// if the first call failed, so then we know the integer was incremented by 0. Otherwise, we at least know
 /// the range of values that it can be incremented by.
 ///
 /// Runs each method several times in a row. This should reduce the non-determinism that comes from
@@ -150,7 +150,7 @@ fn calls_parallel_many_serial() {
 /// Call each method from different threads, allowing them to run in parallel.
 ///
 /// This may cause borrow failures, we do a best-effort attempt at estimating the value then. We can detect
-/// if the first call failed, so then we know the integer was incremented by 0. Otherwise we at least know
+/// if the first call failed, so then we know the integer was incremented by 0. Otherwise, we at least know
 /// the range of values that it can be incremented by.
 ///
 /// Runs all the tests several times. This is different from [`calls_parallel_many_serial`] as that calls the

godot-codegen/src/conv/name_conversions.rs

@@ -280,7 +280,7 @@ fn try_strip_prefixes<'e>(enumerator: &'e str, prefixes: &[&str]) -> &'e str {
     enumerator
 }
 
-/// Check if input is a valid identifier; ie. no special characters except '_' and not starting with a digit.
+/// Check if input is a valid identifier; i.e. no special characters except '_' and not starting with a digit.
 fn is_valid_ident(s: &str) -> bool {
     !starts_with_invalid_char(s) && s.chars().all(|c| c == '_' || c.is_ascii_alphanumeric())
 }

godot-codegen/src/conv/type_conversions.rs

@@ -82,7 +82,7 @@ fn to_hardcoded_rust_enum(ty: &str) -> Option<&str> {
     Some(result)
 }
 
-/// Maps an input type to a Godot type with the same C representation. This is subtly different than [`to_rust_type`],
+/// Maps an input type to a Godot type with the same C representation. This is subtly different from [`to_rust_type`],
 /// which maps to an appropriate corresponding Rust type. This function should be used in situations where the C ABI for
 /// a type must match the Godot equivalent exactly, such as when dealing with pointers.
 pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> (RustTy, bool) {
@@ -105,7 +105,7 @@ pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> (RustTy, bool) {
     (ty, is_obj)
 }
 
-/// Maps an _input_ type from the Godot JSON to the corresponding Rust type (wrapping some sort of a token stream).
+/// Maps an _input_ type from the Godot JSON to the corresponding Rust type (wrapping some sort of token stream).
 ///
 /// Uses an internal cache (via `ctx`), as several types are ubiquitous.
 // TODO take TyName as input
@@ -401,7 +401,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
 }
 
 fn suffixed_lit(num: impl fmt::Display, suffix: &Ident) -> TokenStream {
-    // i32, u16 etc happens to be also the literal suffix
+    // i32, u16 etc. happen to be also the literal suffixes
     let combined = format!("{num}{suffix}");
     combined
         .parse::<Literal>()

godot-codegen/src/formatter/mod.rs

@@ -45,7 +45,7 @@ use proc_macro2::{Delimiter, Spacing, TokenStream, TokenTree};
 ///   after the `,` but because this only looks at one TokenKind at a time
 ///   and only keeps a single state, it's too much effort to keep track of which
 ///   `,` in a [`TokenTree`] should trigger a newline (it would involve a stack
-///   of infos whether that group is in a match or not, detecting pattern etc)
+///   of infos whether that group is in a match or not, detecting pattern etc.)
 /// - Because `,` can be used for function args (no-newline) and separators
 ///   in `struct` and `enum` definitons, those definitions are more awkward
 ///   and crammed into one line as a trade-off to make function calls and args

godot-codegen/src/generator/default_parameters.rs

@@ -59,7 +59,7 @@ pub fn make_function_definition_with_defaults(
     // #[allow] exceptions:
     // - wrong_self_convention:     to_*() and from_*() are taken from Godot
     // - redundant_field_names:     'value: value' is a possible initialization pattern
-    // - needless-update:           '..self' has nothing left to change
+    // - needless-update:           Remainder expression '..self' has nothing left to change
     let builders = quote! {
         #[doc = #builder_doc]
         #[must_use]
@@ -191,10 +191,8 @@ fn make_extender_receiver(sig: &dyn Function) -> ExtenderReceiver {
             ExtenderReceiver {
                 object_fn_param: Some(FnParam {
                     name: ident("surround_object"),
-                    // Not exactly EngineClass, but close enough
-                    type_: RustTy::EngineClass {
+                    type_: RustTy::ExtenderReceiver {
                         tokens: quote! { &'a #builder_mut re_export::#class },
-                        inner_class: ident("unknown"),
                     },
                     default_value: None,
                 }),

godot-codegen/src/generator/enums.rs

@@ -258,7 +258,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum) -> TokenStream {
 
 /// Creates implementations for bitwise operators for the given enum.
 ///
-/// Currently this is just [`BitOr`](std::ops::BitOr) for bitfields but that could be expanded in the future.
+/// Currently, this is just [`BitOr`](std::ops::BitOr) for bitfields but that could be expanded in the future.
 fn make_enum_bitwise_operators(enum_: &Enum) -> TokenStream {
     let name = &enum_.name;
 
@@ -278,7 +278,7 @@ fn make_enum_bitwise_operators(enum_: &Enum) -> TokenStream {
 }
 /// Returns the documentation for the given enum.
 ///
-/// Each string is one line of documentation, usually this needs to be wrapped in a `#[doc = ..]`.
+/// Each string is one line of documentation, usually this needs to be wrapped in a `#[doc = ...]`.
 fn make_enum_doc(enum_: &Enum) -> Vec<String> {
     let mut docs = Vec::new();
 

godot-codegen/src/generator/native_structures.rs

@@ -263,7 +263,7 @@ pub(crate) fn parse_native_structures_format(input: &str) -> Option<Vec<NativeSt
             }
 
             // If the field is an array, store array size separately.
-            // Not part of type because fixed-size arrays are not a concept in the JSON outside of native structures.
+            // Not part of type because fixed-size arrays are not a concept in the JSON outside native structures.
             let mut array_size = None;
             if let Some(index) = field_name.find('[') {
                 array_size = Some(field_name[index + 1..field_name.len() - 1].parse().ok()?);

godot-codegen/src/generator/virtual_traits.rs

@@ -127,7 +127,7 @@ fn make_special_virtual_methods(notification_enum_name: &Ident) -> TokenStream {
         /// function will usually be called twice by Godot to find the revert.
         ///
         /// Note that this should be a _pure_ function. That is, it should always return the same value for a property as long as `self`
-        /// remains unchanged. Otherwise this may lead to unexpected (safe) behavior.
+        /// remains unchanged. Otherwise, this may lead to unexpected (safe) behavior.
         ///
         /// [`Object::_property_get_revert`]: https://docs.godotengine.org/en/latest/classes/class_object.html#class-object-private-method-property-get-revert
         /// [`Object::_property_can_revert`]: https://docs.godotengine.org/en/latest/classes/class_object.html#class-object-private-method-property-can-revert

godot-codegen/src/models/domain.rs

@@ -473,7 +473,11 @@ impl FnQualifier {
 
 pub struct FnParam {
     pub name: Ident,
+
+    /// Type, as it appears in `type CallSig` tuple definition.
     pub type_: RustTy,
+
+    /// Rust expression for default value, if available.
     pub default_value: Option<TokenStream>,
 }
 
@@ -573,15 +577,6 @@ pub struct GodotTy {
     pub meta: Option<String>,
 }
 
-// impl GodotTy {
-//     fn new<'a>(ty: &'a String, meta: &'a Option<String>) -> Self {
-//         Self {
-//             ty: ty.clone(),
-//             meta: meta.clone(),
-//         }
-//     }
-// }
-
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Rust type
 
@@ -627,6 +622,9 @@ pub enum RustTy {
         #[allow(dead_code)] // only read in minimal config
         inner_class: Ident,
     },
+
+    /// Receiver type of default parameters extender constructor.
+    ExtenderReceiver { tokens: TokenStream },
 }
 
 impl RustTy {
@@ -655,6 +653,7 @@ impl ToTokens for RustTy {
             RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens),
             RustTy::EngineBitfield { tokens: path, .. } => path.to_tokens(tokens),
             RustTy::EngineClass { tokens: path, .. } => path.to_tokens(tokens),
+            RustTy::ExtenderReceiver { tokens: path } => path.to_tokens(tokens),
         }
     }
 }

godot-codegen/src/models/domain_mapping.rs

@@ -353,7 +353,7 @@ impl BuiltinMethod {
                 name: method.name.clone(),
                 godot_name: method.name.clone(),
                 // Disable default parameters for builtin classes.
-                // They are not public-facing and need more involved implementation (lifetimes etc). Also reduces number of symbols in API.
+                // They are not public-facing and need more involved implementation (lifetimes etc.). Also reduces number of symbols in API.
                 parameters: FnParam::new_range_no_defaults(&method.arguments, ctx),
                 return_value: FnReturn::new(&return_value, ctx),
                 is_vararg: method.is_vararg,

godot-codegen/src/special_cases/codegen_special_cases.rs

@@ -53,6 +53,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
                 Some(class) => is_class_excluded(class.as_str()),
             },
             RustTy::EngineClass { inner_class, .. } => is_class_excluded(&inner_class.to_string()),
+            RustTy::ExtenderReceiver { .. } => false,
         }
     }
     is_rust_type_excluded(&conv::to_rust_type(ty, None, ctx))
@@ -69,7 +70,7 @@ pub(crate) fn is_class_method_excluded(method: &JsonClassMethod, ctx: &mut Conte
         // so passing in a class name while checking for any types is fine.
         let class_deleted = special_cases::is_godot_type_deleted(ty);
 
-        // Then also check if the type is excluded from codegen (due to current Cargo feature. RHS is always false in full-codegen.
+        // Then also check if the type is excluded from codegen (due to current Cargo feature). RHS is always false in full-codegen.
         class_deleted || is_type_excluded(ty, _ctx)
     };
 

godot-core/src/builtin/callable.rs

@@ -22,7 +22,7 @@ use sys::{ffi_methods, GodotFfi};
 /// also be a custom callable, which is usually created from `bind`, `unbind`, or a GDScript lambda. See
 /// [`Callable::is_custom`].
 ///
-/// Currently it is impossible to use `bind` and `unbind` in GDExtension, see [godot-cpp#802].
+/// Currently, it is impossible to use `bind` and `unbind` in GDExtension, see [godot-cpp#802].
 ///
 /// [godot-cpp#802]: https://github.com/godotengine/godot-cpp/issues/802
 pub struct Callable {
@@ -145,7 +145,7 @@ impl Callable {
         }
     }
 
-    /// Creates an invalid/empty object that is not able to be called.
+    /// Creates an invalid/empty object that cannot be called.
     ///
     /// _Godot equivalent: `Callable()`_
     pub fn invalid() -> Self {

godot-core/src/builtin/collections/array.rs

@@ -702,7 +702,7 @@ impl<T: ArrayElement> Array<T> {
     /// This has the same safety issues as doing `self.assume_type::<Variant>()` and so the relevant safety invariants from
     /// [`assume_type`](Self::assume_type) must be upheld.
     ///
-    /// In particular this means that all reads are fine, since all values can be converted to `Variant`. However writes are only ok
+    /// In particular this means that all reads are fine, since all values can be converted to `Variant`. However, writes are only OK
     /// if they match the type `T`.
     #[doc(hidden)]
     pub unsafe fn as_inner_mut(&self) -> inner::InnerArray {
@@ -1077,7 +1077,7 @@ impl<T: ArrayElement + ToGodot> FromIterator<T> for Array<T> {
 impl<T: ArrayElement + ToGodot> Extend<T> for Array<T> {
     fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
         // Unfortunately the GDExtension API does not offer the equivalent of `Vec::reserve`.
-        // Otherwise we could use it to pre-allocate based on `iter.size_hint()`.
+        // Otherwise, we could use it to pre-allocate based on `iter.size_hint()`.
         //
         // A faster implementation using `resize()` and direct pointer writes might still be
         // possible.
@@ -1122,7 +1122,7 @@ impl<'a, T: ArrayElement + FromGodot> Iterator for Iter<'a, T> {
             let element_ptr = self.array.ptr_or_null(idx);
 
             // SAFETY: We just checked that the index is not out of bounds, so the pointer won't be null.
-            // We immediately convert this to the right element, so barring `experimental-threads` the pointer wont be invalidated in time.
+            // We immediately convert this to the right element, so barring `experimental-threads` the pointer won't be invalidated in time.
             let variant = unsafe { Variant::borrow_var_sys(element_ptr) };
             let element = T::from_variant(variant);
             Some(element)

godot-core/src/builtin/collections/dictionary.rs

@@ -333,7 +333,7 @@ impl Dictionary {
 
 // SAFETY:
 // - `move_return_ptr`
-//   Nothing special needs to be done beyond a `std::mem::swap` when returning an Dictionary.
+//   Nothing special needs to be done beyond a `std::mem::swap` when returning a Dictionary.
 //   So we can just use `ffi_methods`.
 //
 // - `from_arg_ptr`