From 0b8662690cdfa1e5107c17c2c889108549bd3d76 Mon Sep 17 00:00:00 2001
From: Kenny Kerr <kenny@kennykerr.ca>
Date: Mon, 28 Aug 2023 14:28:24 -0500
Subject: [PATCH] Detect unused `bindgen`/`riddle` filters (#2634)

---
 crates/libs/bindgen/src/error.rs        |  8 --------
 crates/libs/bindgen/src/winmd/verify.rs | 12 ++++++++++++
 crates/libs/metadata/src/filter.rs      |  4 ++++
 crates/libs/metadata/src/lib.rs         | 23 +++++++++++++++++++++++
 crates/tests/metadata/tests/unused.rs   | 14 ++++++++++++++
 crates/tests/riddle/src/lib.rs          |  3 +++
 crates/tests/standalone/build.rs        |  2 ++
 7 files changed, 58 insertions(+), 8 deletions(-)
 create mode 100644 crates/tests/metadata/tests/unused.rs

diff --git a/crates/libs/bindgen/src/error.rs b/crates/libs/bindgen/src/error.rs
index 6d783d0844..16122bbf69 100644
--- a/crates/libs/bindgen/src/error.rs
+++ b/crates/libs/bindgen/src/error.rs
@@ -44,12 +44,4 @@ impl Error {
     pub(crate) fn with_path(self, path: &str) -> Self {
         Self { path: path.to_string(), ..self }
     }
-
-    // pub(crate) fn with_span(self, span: proc_macro2::Span) -> Self {
-    //     let start = span.start();
-    //     Self {
-    //         span: Some((start.line, start.column)),
-    //         ..self
-    //     }
-    // }
 }
diff --git a/crates/libs/bindgen/src/winmd/verify.rs b/crates/libs/bindgen/src/winmd/verify.rs
index f10bd65246..42b1fc32b8 100644
--- a/crates/libs/bindgen/src/winmd/verify.rs
+++ b/crates/libs/bindgen/src/winmd/verify.rs
@@ -2,6 +2,18 @@ use super::*;
 use metadata::RowReader;
 
 pub fn verify(reader: &metadata::Reader, filter: &metadata::Filter) -> crate::Result<()> {
+    let unused: Vec<&str> = filter.unused(reader).collect();
+
+    if !unused.is_empty() {
+        let mut message = "unused filters".to_string();
+
+        for unused in unused {
+            message.push_str(&format!("\n  {unused}"));
+        }
+
+        return Err(crate::Error::new(&message));
+    }
+
     for item in reader.items(filter) {
         // TODO: cover all variants
         let metadata::Item::Type(def) = item else {
diff --git a/crates/libs/metadata/src/filter.rs b/crates/libs/metadata/src/filter.rs
index c13afda44a..4b2d229f82 100644
--- a/crates/libs/metadata/src/filter.rs
+++ b/crates/libs/metadata/src/filter.rs
@@ -70,6 +70,10 @@ impl<'a> Filter<'a> {
     fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
+
+    pub fn unused(&self, reader: &'a Reader) -> impl Iterator<Item = &str> + '_ {
+        self.0.iter().filter_map(|(name, _)| if reader.unused(name) { Some(*name) } else { None })
+    }
 }
 
 fn match_type_name(rule: &str, namespace: &str, name: &str) -> bool {
diff --git a/crates/libs/metadata/src/lib.rs b/crates/libs/metadata/src/lib.rs
index 70a85c5b10..1aca2157fa 100644
--- a/crates/libs/metadata/src/lib.rs
+++ b/crates/libs/metadata/src/lib.rs
@@ -130,6 +130,29 @@ impl<'a> Reader<'a> {
         self.items.get_key_value(namespace).into_iter().flat_map(move |(namespace, items)| items.iter().filter(move |(name, _)| filter.includes_type_name(TypeName::new(namespace, name)))).flat_map(move |(_, items)| items).cloned()
     }
 
+    fn unused(&self, filter: &str) -> bool {
+        // Match namespaces
+        if self.items.contains_key(filter) {
+            return false;
+        }
+
+        // Match type names
+        if let Some((namespace, name)) = filter.rsplit_once('.') {
+            if self.items.get(namespace).is_some_and(|items| items.contains_key(name)) {
+                return false;
+            }
+        }
+
+        // Match empty parent namespaces
+        for namespace in self.items.keys() {
+            if namespace.len() > filter.len() && namespace.starts_with(filter) && namespace.as_bytes()[filter.len()] == b'.' {
+                return false;
+            }
+        }
+
+        true
+    }
+
     fn get_item(&self, type_name: TypeName) -> impl Iterator<Item = Item> + '_ {
         if let Some(items) = self.items.get(type_name.namespace) {
             if let Some(items) = items.get(type_name.name) {
diff --git a/crates/tests/metadata/tests/unused.rs b/crates/tests/metadata/tests/unused.rs
new file mode 100644
index 0000000000..bda800581f
--- /dev/null
+++ b/crates/tests/metadata/tests/unused.rs
@@ -0,0 +1,14 @@
+use metadata::*;
+
+#[test]
+fn test() {
+    let files = tool_lib::default_metadata();
+    let reader = &Reader::new(&files);
+    let filter = Filter::new(
+        &["Windows", "BadNamespace", "Windows.AI"],
+        &["Windows.Foundation.Rect", "Windows.Foundation.BadType"],
+    );
+    let unused: Vec<&str> = filter.unused(reader).collect();
+
+    assert_eq!(unused, ["Windows.Foundation.BadType", "BadNamespace"]);
+}
diff --git a/crates/tests/riddle/src/lib.rs b/crates/tests/riddle/src/lib.rs
index 9b0ebcfece..c731ee6aa4 100644
--- a/crates/tests/riddle/src/lib.rs
+++ b/crates/tests/riddle/src/lib.rs
@@ -18,6 +18,7 @@ pub fn run_riddle(name: &str, dialect: &str, etc: &[&str]) -> Vec<windows_metada
     let before = std::fs::read_to_string(&rdl).expect("Failed to read input");
 
     // Convert .rdl to .winmd
+    _ = std::fs::remove_file(&winmd);
     let mut command = Command::new("cargo");
     command.args([
         "run", "-p", "riddle", "--", "--in", &rdl, "--out", &winmd, "--filter", "Test",
@@ -25,6 +26,7 @@ pub fn run_riddle(name: &str, dialect: &str, etc: &[&str]) -> Vec<windows_metada
     assert!(command.status().unwrap().success());
 
     // Convert .winmd back to .rdl
+    std::fs::remove_file(&rdl).expect("Failed to delete output");
     let mut command = Command::new("cargo");
     command.args([
         "run", "-p", "riddle", "--", "--in", &winmd, "--out", &rdl, "--filter", "Test", "--config",
@@ -37,6 +39,7 @@ pub fn run_riddle(name: &str, dialect: &str, etc: &[&str]) -> Vec<windows_metada
     assert_eq!(before, after, "no equal {}", rdl);
 
     // Convert .rdl to .rs
+    std::fs::remove_file(&rs).expect("Failed to delete output");
     let mut command = Command::new("cargo");
     command.args([
         "run", "-p", "riddle", "--", "--in", &rdl, "--out", &rs, "--filter", "Test",
diff --git a/crates/tests/standalone/build.rs b/crates/tests/standalone/build.rs
index ce6e908ce3..82dbacf9f4 100644
--- a/crates/tests/standalone/build.rs
+++ b/crates/tests/standalone/build.rs
@@ -150,6 +150,8 @@ fn write_std(output: &str, filter: &[&str]) {
 }
 
 fn riddle(output: &str, filter: &[&str], config: &[&str]) {
+    std::fs::remove_file(output).expect("Failed to delete output");
+
     let mut command = std::process::Command::new("cargo");
 
     command.args([