Merge #998

998: Random chores r=chitoyuu a=chitoyuu Performed a number of miscellaneous chores around the codebase. This PR contains a number of unrelated changes: please check the commit log for details. A list of all issues created in the process can be found in #377. Close #377. Co-authored-by: Chitose Yuuzaki <[email protected]>
godot-rust · Dec 8, 2022 · f920000 · f920000
2 parents 7eafdd7 + 2cde642
commit f920000
Show file tree

Hide file tree

Showing 62 changed files with 297 additions and 332 deletions.
diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml
@@ -12,6 +12,8 @@ env:
   # Don't use more features like "gdnative_bindings_generator/debug" to keep CI truly minimal
   GDRUST_FEATURES: "gdnative/async,gdnative/serde"
 
+  RIPGREP_VERSION: "13.0.0"
+
 on:
   pull_request:
     branches:
@@ -52,6 +54,19 @@ jobs:
       - name: "Check clippy"
         run: cargo clippy --workspace --features ${GDRUST_FEATURES} -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented
 
+  check-todo:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: "Install ripgrep"
+        run: |
+          cd /tmp
+          wget --no-verbose https://github.com/BurntSushi/ripgrep/releases/download/${RIPGREP_VERSION}/ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl.tar.gz -O ripgrep.tar.gz
+          tar -zxvf ripgrep.tar.gz
+          sudo mv ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg /usr/bin
+      - name: "Look for TODO comments without issue numbers attached to them"
+        run: bash tools/detect-todo.sh
+
   unit-test:
     runs-on: ubuntu-latest
     steps:

diff --git a/bindings-generator/src/api.rs b/bindings-generator/src/api.rs
@@ -785,7 +785,7 @@ mod tests {
         ];
         tests.iter().for_each(|(class_name, expected)| {
             let actual = module_name_from_class_name(class_name);
-            assert_eq!(*expected, actual, "Input: {}", class_name);
+            assert_eq!(*expected, actual, "Input: {class_name}");
         });
     }
 }
diff --git a/bindings-generator/src/class_docs.rs b/bindings-generator/src/class_docs.rs
@@ -67,20 +67,10 @@ impl GodotXmlDocs {
                         if let Some(property_name) = node.attribute("name") {
                             if !property_name.contains('/') {
                                 if node.has_attribute("setter") {
-                                    self.add_fn(
-                                        class,
-                                        &format!("set_{}", property_name),
-                                        desc,
-                                        &[],
-                                    );
+                                    self.add_fn(class, &format!("set_{property_name}"), desc, &[]);
                                 }
                                 if node.has_attribute("getter") {
-                                    self.add_fn(
-                                        class,
-                                        &format!("get_{}", property_name),
-                                        desc,
-                                        &[],
-                                    );
+                                    self.add_fn(class, &format!("get_{property_name}"), desc, &[]);
                                 }
                             }
                         }
@@ -187,10 +177,7 @@ impl GodotXmlDocs {
 
         // Info for GDScript blocks
         let godot_doc = if godot_doc.contains("[codeblock]") {
-            format!(
-                "_Sample code is GDScript unless otherwise noted._\n\n{}",
-                godot_doc
-            )
+            format!("_Sample code is GDScript unless otherwise noted._\n\n{godot_doc}")
         } else {
             godot_doc
         };
@@ -213,9 +200,9 @@ impl GodotXmlDocs {
             let text = &c[2];
 
             if text.is_empty() {
-                format!("<{url}>", url = url)
+                format!("<{url}>")
             } else {
-                format!("[{text}]({url})", text = text, url = url)
+                format!("[{text}]({url})")
             }
         });
 
@@ -247,11 +234,7 @@ impl GodotXmlDocs {
             let godot_ty = &c[2];
             let rust_ty = Self::translate_type(godot_ty);
 
-            format!(
-                "[`{godot_ty}`][{rust_ty}]",
-                godot_ty = godot_ty,
-                rust_ty = rust_ty
-            )
+            format!("[`{godot_ty}`][{rust_ty}]")
         });
 
         godot_doc.to_string()

diff --git a/bindings-generator/src/classes.rs b/bindings-generator/src/classes.rs
@@ -96,9 +96,6 @@ pub(crate) fn generate_class_constants(class: &GodotClass) -> TokenStream {
 }
 
 pub(crate) fn generate_enums(class: &GodotClass) -> TokenStream {
-    // TODO: check whether the start of the variant name is equal to the end of the enum name and if so, don't repeat it.
-    // For example ImageFormat::Rgb8 instead of ImageFormat::FormatRgb8.
-
     let mut enums: Vec<&Enum> = class.enums.iter().collect();
     enums.sort();
     let enums = enums.iter().map(|e| {

diff --git a/bindings-generator/src/documentation.rs b/bindings-generator/src/documentation.rs
@@ -149,7 +149,7 @@ fn list_base_classes(output: &mut impl Write, api: &Api, parent_name: &str) -> G
     if let Some(parent) = api.find_class(parent_name) {
         let class_link = class_doc_link(parent);
 
-        writeln!(output, " - {}", class_link)?;
+        writeln!(output, " - {class_link}")?;
 
         if !parent.base_class.is_empty() {
             list_base_classes(output, api, &parent.base_class)?;

diff --git a/bindings-generator/src/godot_version.rs b/bindings-generator/src/godot_version.rs
@@ -15,12 +15,7 @@ pub fn parse_godot_version(version_str: &str) -> Result<GodotVersion, Box<dyn Er
 
     let caps = regex.captures(version_str).ok_or("Regex capture failed")?;
 
-    let fail = || {
-        format!(
-            "Version substring could not be matched in '{}'",
-            version_str
-        )
-    };
+    let fail = || format!("Version substring could not be matched in '{version_str}'");
 
     Ok(GodotVersion {
         major: caps.get(1).ok_or_else(fail)?.as_str().parse::<u8>()?,

diff --git a/bindings-generator/src/methods.rs b/bindings-generator/src/methods.rs
@@ -200,7 +200,7 @@ pub fn generate_method_table(api: &Api, class: &GodotClass) -> TokenStream {
         } = m.get_name();
 
         let rust_ident = format_ident!("{}", rust_name);
-        let original_name = format!("{}\0", original_name);
+        let original_name = format!("{original_name}\0");
 
         if !skip_method(m, rust_name) {
             assert!(original_name.ends_with('\0'), "original_name must be null terminated");

diff --git a/bindings-generator/src/special_methods.rs b/bindings-generator/src/special_methods.rs
@@ -113,7 +113,7 @@ pub fn generate_singleton_getter(class: &GodotClass) -> TokenStream {
         class.name.as_ref()
     };
 
-    let singleton_name = format!("{}\0", s_name);
+    let singleton_name = format!("{s_name}\0");
 
     assert!(
         singleton_name.ends_with('\0'),

diff --git a/examples/dodge-the-creeps/src/player.rs b/examples/dodge-the-creeps/src/player.rs
@@ -82,8 +82,8 @@ impl Player {
         let change = velocity * delta;
         let position = owner.global_position() + change;
         let position = Vector2::new(
-            position.x.max(0.0).min(self.screen_size.x),
-            position.y.max(0.0).min(self.screen_size.y),
+            position.x.clamp(0.0, self.screen_size.x),
+            position.y.clamp(0.0, self.screen_size.y),
         );
         owner.set_global_position(position);
     }

diff --git a/examples/scene-create/src/lib.rs b/examples/scene-create/src/lib.rs
@@ -59,7 +59,7 @@ impl SceneCreate {
             Ok(spatial) => {
                 // Here is how you rename the child...
                 let key_str = format!("child_{}", self.children_spawned);
-                spatial.set_name(&key_str);
+                spatial.set_name(key_str);
 
                 let x = (self.children_spawned % 10) as f32;
                 let z = (self.children_spawned / 10) as f32;

diff --git a/gdnative-async/src/method.rs b/gdnative-async/src/method.rs
@@ -209,7 +209,7 @@ impl<C: NativeClass, F: AsyncMethod<C>> Method<C> for Async<F> {
                 Some(Err(err)) => {
                     log::error(
                         Self::site().unwrap_or_default(),
-                        format_args!("unable to spawn future: {}", err),
+                        format_args!("unable to spawn future: {err}"),
                     );
                     Variant::nil()
                 }

diff --git a/gdnative-async/src/rt.rs b/gdnative-async/src/rt.rs
@@ -107,8 +107,8 @@ pub fn register_runtime_with_prefix<S>(handle: &InitHandle, prefix: S)
 where
     S: Display,
 {
-    handle.add_class_as::<bridge::SignalBridge>(format!("{}SignalBridge", prefix));
-    handle.add_class_as::<func_state::FuncState>(format!("{}FuncState", prefix));
+    handle.add_class_as::<bridge::SignalBridge>(format!("{prefix}SignalBridge"));
+    handle.add_class_as::<func_state::FuncState>(format!("{prefix}FuncState"));
 }
 
 /// Releases all observers still in use. This should be called in the

diff --git a/gdnative-bindings/build.rs b/gdnative-bindings/build.rs
@@ -92,16 +92,15 @@ fn generate(
     for (class, code) in &binding_res.class_bindings {
         let mod_name = gen::module_name_from_class_name(&class.name);
 
-        let mod_path = out_path.join(format!("{}.rs", mod_name));
+        let mod_path = out_path.join(format!("{mod_name}.rs"));
         let mut mod_output = BufWriter::new(File::create(&mod_path).unwrap());
 
         write!(
             &mut mod_output,
             r#"
-            {content}
+            {code}
             use super::*;
             "#,
-            content = code,
         )
         .unwrap();
 
@@ -152,7 +151,7 @@ fn format_file_if_needed(output_rs: &Path) {
         Ok(_) => println!("Done."),
         Err(err) => {
             println!("Failed.");
-            println!("Error: {}", err);
+            println!("Error: {err}");
         }
     }
 }

diff --git a/gdnative-core/src/core_types/byte_array.rs b/gdnative-core/src/core_types/byte_array.rs
@@ -49,6 +49,6 @@ godot_test!(
 godot_test!(
     test_byte_array_debug {
         let arr = (0..8).collect::<ByteArray>();
-        assert_eq!(format!("{:?}", arr), "[0, 1, 2, 3, 4, 5, 6, 7]");
+        assert_eq!(format!("{arr:?}"), "[0, 1, 2, 3, 4, 5, 6, 7]");
     }
 );
diff --git a/gdnative-core/src/core_types/color_array.rs b/gdnative-core/src/core_types/color_array.rs
@@ -57,6 +57,6 @@ godot_test!(
             Color::from_rgb(0.0, 0.0, 1.0),
         ]);
 
-        assert_eq!(format!("{:?}", arr), "[Color { r: 1.0, g: 0.0, b: 0.0, a: 1.0 }, Color { r: 0.0, g: 1.0, b: 0.0, a: 1.0 }, Color { r: 0.0, g: 0.0, b: 1.0, a: 1.0 }]");
+        assert_eq!(format!("{arr:?}"), "[Color { r: 1.0, g: 0.0, b: 0.0, a: 1.0 }, Color { r: 0.0, g: 1.0, b: 0.0, a: 1.0 }, Color { r: 0.0, g: 0.0, b: 1.0, a: 1.0 }]");
     }
 );
diff --git a/gdnative-core/src/core_types/dictionary.rs b/gdnative-core/src/core_types/dictionary.rs
@@ -578,23 +578,21 @@ godot_test!(test_dictionary {
         assert_eq!(Some(value), dict.get(&key));
         assert!(
             iter_keys.insert(key.to_string()) ,
-            "key is already contained in set: {:?}",
-            key
+            "key is already contained in set: {key:?}"
         );
     }
     assert_eq!(expected_keys, iter_keys);
 });
 
-// TODO: clear dictionaries without affecting clones
-//godot_test!(test_dictionary_clone_clear {
-//    let foo = Variant::from_str("foo");
-//    let bar = Variant::from_str("bar");
-//    let mut dict = Dictionary::new();
-//
-//    dict.set(&foo, &bar);
-//    let dict_clone = dict.clone();
-//    dict.clear();
-//
-//    assert!(dict.is_empty());
-//    assert!(!dict_clone.is_empty());
-//});
+godot_test!(test_dictionary_clone_clear {
+    let foo = Variant::new("foo");
+    let bar = Variant::new("bar");
+    let dict = Dictionary::new();
+
+    dict.insert(&foo, &bar);
+    let dict_clone = dict.duplicate();
+    dict.clear();
+
+    assert!(dict.is_empty());
+    assert!(!dict_clone.is_empty());
+});
diff --git a/gdnative-core/src/core_types/error.rs b/gdnative-core/src/core_types/error.rs
@@ -77,7 +77,7 @@ impl GodotError {
 impl std::fmt::Display for GodotError {
     #[inline]
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "Godot API error: {:?}", self)
+        write!(f, "Godot API error: {self:?}")
     }
 }
 

diff --git a/gdnative-core/src/core_types/float32_array.rs b/gdnative-core/src/core_types/float32_array.rs
@@ -43,6 +43,6 @@ godot_test!(
 godot_test!(
     test_float32_array_debug {
         let arr = (0..8).map(|i| i as f32).collect::<Float32Array>();
-        assert_eq!(format!("{:?}", arr), "[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0]");
+        assert_eq!(format!("{arr:?}"), "[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0]");
     }
 );
diff --git a/gdnative-core/src/core_types/geom/plane.rs b/gdnative-core/src/core_types/geom/plane.rs
@@ -1,6 +1,6 @@
 use crate::core_types::{IsEqualApprox, Vector3};
 
-// TODO enforce invariants via setters, make fields private
+// TODO(#994) enforce invariants via setters, make fields private
 // Otherwise almost all methods need to panic
 // - normal.length() == 1
 // - d > 0
@@ -322,7 +322,7 @@ mod test {
         assert!(!p.contains_point(Vector3::new(6.562291, -0.186564, -0.101982)));
     }
 
-    // TODO contains_point_eps()
+    // TODO(#994) contains_point_eps()
 
     #[test]
     fn intersect_3() {