extension_cli: Validate that all snippets can be parsed (#47649)

Finn Evers created

With this, we can now ensure that snippet extensions we publish actually
only contain valid snippets.

Release Notes:

- N/A

Change summary

Cargo.lock                              |  3 
crates/extension_cli/Cargo.toml         |  2 
crates/extension_cli/src/main.rs        | 36 ++++++++++++++++
crates/snippet_provider/Cargo.toml      |  1 
crates/snippet_provider/src/lib.rs      | 56 +++++++++++++++-----------
crates/snippet_provider/src/registry.rs |  5 +
6 files changed, 74 insertions(+), 29 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -6010,6 +6010,8 @@ dependencies = [
  "rpc",
  "serde",
  "serde_json",
+ "serde_json_lenient",
+ "snippet_provider",
  "theme",
  "tokio",
  "toml 0.8.23",
@@ -15420,7 +15422,6 @@ dependencies = [
  "futures 0.3.31",
  "gpui",
  "indoc",
- "log",
  "parking_lot",
  "paths",
  "schemars",

crates/extension_cli/Cargo.toml 🔗

@@ -25,6 +25,8 @@ reqwest_client.workspace = true
 rpc.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+serde_json_lenient.workspace = true
+snippet_provider.workspace = true
 theme.workspace = true
 tokio = { workspace = true, features = ["full"] }
 toml.workspace = true

crates/extension_cli/src/main.rs 🔗

@@ -5,13 +5,15 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use ::fs::{CopyOptions, Fs, RealFs, copy_recursive};
-use anyhow::{Context as _, Result, bail};
+use anyhow::{Context as _, Result, anyhow, bail};
 use clap::Parser;
-use extension::ExtensionManifest;
 use extension::extension_builder::{CompileExtensionOptions, ExtensionBuilder};
+use extension::{ExtensionManifest, ExtensionSnippets};
 use language::LanguageConfig;
 use reqwest_client::ReqwestClient;
 use rpc::ExtensionProvides;
+use snippet_provider::file_to_snippets;
+use snippet_provider::format::VsSnippetsFile;
 use tokio::process::Command;
 use tree_sitter::{Language, Query, WasmStore};
 
@@ -79,6 +81,7 @@ async fn main() -> Result<()> {
     let grammars = test_grammars(&manifest, &extension_path, &mut wasm_store)?;
     test_languages(&manifest, &extension_path, &grammars)?;
     test_themes(&manifest, &extension_path, fs.clone()).await?;
+    test_snippets(&manifest, &extension_path, fs.clone()).await?;
 
     let archive_dir = output_dir.join("archive");
     fs::remove_dir_all(&archive_dir).ok();
@@ -423,3 +426,32 @@ async fn test_themes(
 
     Ok(())
 }
+
+async fn test_snippets(
+    manifest: &ExtensionManifest,
+    extension_path: &Path,
+    fs: Arc<dyn Fs>,
+) -> Result<()> {
+    for relative_snippet_path in manifest
+        .snippets
+        .as_ref()
+        .map(ExtensionSnippets::paths)
+        .into_iter()
+        .flatten()
+    {
+        let snippet_path = extension_path.join(relative_snippet_path);
+        let snippets_content = fs.load_bytes(&snippet_path).await?;
+        let snippets_file = serde_json_lenient::from_slice::<VsSnippetsFile>(&snippets_content)
+            .with_context(|| anyhow!("Failed to load snippet file at {snippet_path:?}"))?;
+        let snippet_errors = file_to_snippets(snippets_file, &snippet_path)
+            .flat_map(Result::err)
+            .collect::<Vec<_>>();
+
+        anyhow::ensure!(
+            snippet_errors.len() == 0,
+            "Could not parse all snippets in file {snippet_path:?}: {snippet_errors:?}"
+        );
+    }
+
+    Ok(())
+}

crates/snippet_provider/Cargo.toml 🔗

@@ -18,7 +18,6 @@ extension.workspace = true
 fs.workspace = true
 futures.workspace = true
 gpui.workspace = true
-log.workspace = true
 parking_lot.workspace = true
 paths.workspace = true
 serde.workspace = true

crates/snippet_provider/src/lib.rs 🔗

@@ -15,6 +15,7 @@ use fs::Fs;
 use futures::stream::StreamExt;
 use gpui::{App, AppContext as _, AsyncApp, Context, Entity, Task, WeakEntity};
 pub use registry::*;
+use util::ResultExt;
 
 pub fn init(cx: &mut App) {
     SnippetRegistry::init_global(cx);
@@ -31,30 +32,36 @@ fn file_stem_to_key(stem: &str) -> SnippetKind {
     }
 }
 
-fn file_to_snippets(file_contents: VsSnippetsFile, source: &Path) -> Vec<Arc<Snippet>> {
-    let mut snippets = vec![];
-    for (name, snippet) in file_contents.snippets {
-        let snippet_name = name.clone();
-        let prefixes = snippet
-            .prefix
-            .map_or_else(move || vec![snippet_name], |prefixes| prefixes.into());
-        let description = snippet
-            .description
-            .map(|description| description.to_string());
-        let body = snippet.body.to_string();
-        if let Err(e) = snippet::Snippet::parse(&body) {
-            log::error!("Invalid snippet name '{name}' in {source:?}: {e:#}");
-            continue;
-        }
-        snippets.push(Arc::new(Snippet {
-            body,
-            prefix: prefixes,
-            description,
-            name,
-        }));
-    }
-    snippets
+pub fn file_to_snippets(
+    file_contents: VsSnippetsFile,
+    source: &Path,
+) -> impl Iterator<Item = Result<Arc<Snippet>>> {
+    file_contents
+        .snippets
+        .into_iter()
+        .map(move |(name, snippet)| {
+            let snippet_name = name.clone();
+            let prefixes = snippet
+                .prefix
+                .map_or_else(move || vec![snippet_name], |prefixes| prefixes.into());
+            let description = snippet
+                .description
+                .map(|description| description.to_string());
+            let body = snippet.body.to_string();
+            match snippet::Snippet::parse(&body) {
+                Ok(_) => Ok(Arc::new(Snippet {
+                    body,
+                    prefix: prefixes,
+                    description,
+                    name,
+                })),
+                Err(e) => Err(anyhow::anyhow!(
+                    "Invalid snippet '{name}' in {source:?}: {e:#}"
+                )),
+            }
+        })
 }
+
 // Snippet with all of the metadata
 #[derive(Debug)]
 pub struct Snippet {
@@ -106,7 +113,8 @@ async fn process_updates(
                     return;
                 };
                 let snippets = file_to_snippets(as_json, entry_path.as_path());
-                *snippets_of_kind.entry(entry_path).or_default() = snippets;
+                *snippets_of_kind.entry(entry_path).or_default() =
+                    snippets.filter_map(Result::log_err).collect();
             } else {
                 snippets_of_kind.remove(&entry_path);
             }

crates/snippet_provider/src/registry.rs 🔗

@@ -4,6 +4,7 @@ use anyhow::Result;
 use collections::HashMap;
 use gpui::{App, Global, ReadGlobal, UpdateGlobal};
 use parking_lot::RwLock;
+use util::ResultExt;
 
 use crate::{Snippet, SnippetKind, file_stem_to_key};
 
@@ -43,7 +44,9 @@ impl SnippetRegistry {
             .file_stem()
             .and_then(|stem| stem.to_str().and_then(file_stem_to_key));
         let snippets = crate::file_to_snippets(snippets_in_file, file_path);
-        self.snippets.write().insert(kind, snippets);
+        self.snippets
+            .write()
+            .insert(kind, snippets.filter_map(Result::log_err).collect());
 
         Ok(())
     }