Access content lazily instead of requiring an eager value

Author: Hendrik van Antwerpen

tree-sitter-stack-graphs/CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - A new `CancelAfterDuration` implementation of `CancellationFlag` that cancels the computation after a certain amount of time.
 
+#### Changed
+
+- The `LanguageConfiguration::matches_file` method takes a `ContentProvider` instead of an `Option<&str>` value. This allows lazy file reading *after* the filename is checked, instead of the unconditional loading required before. To give content readers the opportunity to cache read values, a mutable reference is required. The return type has changed to `std::io::Result` to propagate possible errors from content providers. A `FileReader` implementation that caches the last read file is provided as well.
+
 ### CLI
 
 #### Added

tree-sitter-stack-graphs/src/cli/analyze.rs

@@ -23,6 +23,7 @@ use walkdir::WalkDir;
 use crate::cli::util::duration_from_seconds_str;
 use crate::cli::util::map_parse_errors;
 use crate::cli::util::path_exists;
+use crate::loader::FileReader;
 use crate::loader::Loader;
 use crate::CancelAfterDuration;
 use crate::CancellationFlag;
@@ -77,24 +78,11 @@ impl AnalyzeArgs {
                     .filter(|e| e.file_type().is_file())
                 {
                     let source_path = source_entry.path();
-                    if let Err(e) = self.analyze_file_with_context(source_root, source_path, loader)
-                    {
-                        eprintln!(
-                            "Skipping file {} due to analysis failure {}",
-                            source_path.display(),
-                            e.to_string()
-                        );
-                    }
+                    self.analyze_file_with_context(source_root, source_path, loader)?;
                 }
             } else {
                 let source_root = source_path.parent().unwrap();
-                if let Err(e) = self.analyze_file_with_context(source_root, source_path, loader) {
-                    eprintln!(
-                        "Skipping file {} due to analysis failure {}",
-                        source_path.display(),
-                        e.to_string()
-                    );
-                };
+                self.analyze_file_with_context(source_root, source_path, loader)?;
             }
         }
         Ok(())
@@ -119,8 +107,8 @@ impl AnalyzeArgs {
     ) -> anyhow::Result<()> {
         let mut file_status = FileStatusLogger::new(source_path, self.verbose);
 
-        let source = std::fs::read_to_string(source_path)?;
-        let lc = match loader.load_for_file(source_path, Some(&source), &NoCancellation) {
+        let mut file_reader = FileReader::new();
+        let lc = match loader.load_for_file(source_path, &mut file_reader, &NoCancellation) {
             Ok(Some(sgl)) => sgl,
             Ok(None) => return Ok(()),
             Err(crate::loader::LoadError::Cancelled(_)) => {
@@ -129,6 +117,7 @@ impl AnalyzeArgs {
             }
             Err(e) => return Err(e.into()),
         };
+        let source = file_reader.get(source_path)?;
 
         let mut cancellation_flag: Arc<dyn CancellationFlag> = Arc::new(NoCancellation);
         if let Some(max_file_time) = self.max_file_time {

tree-sitter-stack-graphs/src/cli/parse.rs

@@ -15,6 +15,7 @@ use tree_sitter::Parser;
 use tree_sitter_graph::parse_error::ParseError;
 
 use crate::cli::util::path_exists;
+use crate::loader::FileReader;
 use crate::loader::Loader;
 use crate::LoadError;
 
@@ -34,11 +35,12 @@ impl ParseArgs {
     }
 
     fn parse_file(&self, file_path: &Path, loader: &mut Loader) -> anyhow::Result<()> {
-        let source = std::fs::read_to_string(file_path)?;
-        let lang = match loader.load_tree_sitter_language_for_file(file_path, Some(&source))? {
+        let mut file_reader = FileReader::new();
+        let lang = match loader.load_tree_sitter_language_for_file(file_path, &mut file_reader)? {
             Some(sgl) => sgl,
             None => return Err(anyhow!("No stack graph language found")),
         };
+        let source = file_reader.get(file_path)?;
 
         let mut parser = Parser::new();
         parser.set_language(lang)?;

tree-sitter-stack-graphs/src/cli/test.rs

@@ -24,6 +24,7 @@ use walkdir::WalkDir;
 use crate::cli::util::map_parse_errors;
 use crate::cli::util::path_exists;
 use crate::cli::util::PathSpec;
+use crate::loader::FileReader;
 use crate::loader::LanguageConfiguration;
 use crate::loader::Loader;
 use crate::test::Test;
@@ -193,8 +194,8 @@ impl TestArgs {
         test_path: &Path,
         loader: &mut Loader,
     ) -> anyhow::Result<TestResult> {
-        let source = std::fs::read_to_string(test_path)?;
-        let lc = match loader.load_for_file(test_path, Some(&source), &NoCancellation)? {
+        let mut file_reader = FileReader::new();
+        let lc = match loader.load_for_file(test_path, &mut file_reader, &NoCancellation)? {
             Some(sgl) => sgl,
             None => {
                 if self.show_ignored {
@@ -203,6 +204,7 @@ impl TestArgs {
                 return Ok(TestResult::new());
             }
         };
+        let source = file_reader.get(test_path)?;
         let default_fragment_path = test_path.strip_prefix(test_root).unwrap();
         let mut test = Test::from_source(&test_path, &source, default_fragment_path)?;
         self.load_builtins_into(&lc, &mut test.graph)
@@ -224,7 +226,10 @@ impl TestArgs {
                     &test_fragment.globals,
                     &NoCancellation,
                 )?;
-            } else if lc.matches_file(&test_fragment.path, Some(&test_fragment.source)) {
+            } else if lc.matches_file(
+                &test_fragment.path,
+                &mut Some(test_fragment.source.as_ref()),
+            )? {
                 globals.clear();
                 test_fragment.add_globals_to(&mut globals);
                 self.build_fragment_stack_graph_into(

tree-sitter-stack-graphs/src/loader.rs

@@ -86,8 +86,12 @@ impl LanguageConfiguration {
         })
     }
 
-    pub fn matches_file(&self, path: &Path, content: Option<&str>) -> bool {
-        matches_file(&self.file_types, &self.content_regex, path, content).is_some()
+    pub fn matches_file(
+        &self,
+        path: &Path,
+        content: &mut dyn ContentProvider,
+    ) -> std::io::Result<bool> {
+        matches_file(&self.file_types, &self.content_regex, path, content).map(|l| l.is_some())
     }
 }
 
@@ -205,7 +209,7 @@ impl Loader {
     pub fn load_tree_sitter_language_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
     ) -> Result<Option<tree_sitter::Language>, LoadError> {
         match &mut self.0 {
             LoaderImpl::Paths(loader) => loader.load_tree_sitter_language_for_file(path, content),
@@ -219,7 +223,7 @@ impl Loader {
     pub fn load_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
         cancellation_flag: &dyn CancellationFlag,
     ) -> Result<Option<&LanguageConfiguration>, LoadError> {
         match &mut self.0 {
@@ -328,34 +332,28 @@ impl LanguageConfigurationsLoader {
     pub fn load_tree_sitter_language_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
     ) -> Result<Option<tree_sitter::Language>, LoadError> {
-        let configuration = match self
-            .configurations
-            .iter()
-            .find(|l| l.matches_file(path, content))
-        {
-            Some(language) => language,
-            None => return Ok(None),
-        };
-        Ok(Some(configuration.language))
+        for configuration in self.configurations.iter() {
+            if configuration.matches_file(path, content)? {
+                return Ok(Some(configuration.language));
+            }
+        }
+        Ok(None)
     }
 
     /// Load a stack graph language for the given file. Loading is based on the loader configuration and the given file path.
     pub fn load_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
     ) -> Result<Option<&LanguageConfiguration>, LoadError> {
-        let language = match self
-            .configurations
-            .iter()
-            .find(|l| l.matches_file(path, content))
-        {
-            Some(language) => language,
-            None => return Ok(None),
-        };
-        Ok(Some(language))
+        for language in self.configurations.iter() {
+            if language.matches_file(path, content)? {
+                return Ok(Some(language));
+            }
+        }
+        Ok(None)
     }
 }
 
@@ -400,7 +398,7 @@ impl PathLoader {
     pub fn load_tree_sitter_language_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
     ) -> Result<Option<tree_sitter::Language>, LoadError> {
         if let Some(selected_language) = self.select_language_for_file(path, content)? {
             return Ok(Some(selected_language.language));
@@ -411,7 +409,7 @@ impl PathLoader {
     pub fn load_for_file(
         &mut self,
         path: &Path,
-        content: Option<&str>,
+        content: &mut dyn ContentProvider,
         cancellation_flag: &dyn CancellationFlag,
     ) -> Result<Option<&LanguageConfiguration>, LoadError> {
         let selected_language = self.select_language_for_file(path, content)?;
@@ -457,7 +455,7 @@ impl PathLoader {
     fn select_language_for_file(
         &mut self,
         file_path: &Path,
-        file_content: Option<&str>,
+        file_content: &mut dyn ContentProvider,
     ) -> Result<Option<&SupplementedLanguage>, LoadError> {
         // The borrow checker is not smart enough to realize that the early returns
         // ensure any references from the self.select_* call (which require a mutable
@@ -494,7 +492,7 @@ impl PathLoader {
         &mut self,
         language_path: &Path,
         file_path: &Path,
-        file_content: Option<&str>,
+        file_content: &mut dyn ContentProvider,
     ) -> Result<Option<&SupplementedLanguage>, LoadError> {
         let scope = self.scope.as_deref();
         let languages = self.loader.languages_at_path(language_path, scope)?;
@@ -506,7 +504,7 @@ impl PathLoader {
             )));
         }
         if let Some(language) =
-            SupplementedLanguage::best_for_file(languages, file_path, file_content)
+            SupplementedLanguage::best_for_file(languages, file_path, file_content)?
         {
             return Ok(Some(language));
         };
@@ -641,27 +639,31 @@ impl SupplementedLanguage {
     }
 
     // Extracted from tree_sitter_loader::Loader::language_configuration_for_file_name
-    pub fn matches_file(&self, path: &Path, content: Option<&str>) -> Option<isize> {
+    pub fn matches_file(
+        &self,
+        path: &Path,
+        content: &mut dyn ContentProvider,
+    ) -> std::io::Result<Option<isize>> {
         matches_file(&self.file_types, &self.content_regex, path, content)
     }
 
     // Extracted from tree_sitter_loader::Loader::language_configuration_for_file_name
     pub fn best_for_file<'a>(
         languages: Vec<&'a SupplementedLanguage>,
         path: &Path,
-        content: Option<&str>,
-    ) -> Option<&'a SupplementedLanguage> {
+        content: &mut dyn ContentProvider,
+    ) -> std::io::Result<Option<&'a SupplementedLanguage>> {
         let mut best_score = -1isize;
         let mut best = None;
         for language in languages {
-            if let Some(score) = language.matches_file(path, content) {
+            if let Some(score) = language.matches_file(path, content)? {
                 if score > best_score {
                     best_score = score;
                     best = Some(language);
                 }
             }
         }
-        best
+        Ok(best)
     }
 }
 
@@ -682,28 +684,64 @@ pub fn matches_file(
     file_types: &Vec<String>,
     content_regex: &Option<Regex>,
     path: &Path,
-    content: Option<&str>,
-) -> Option<isize> {
+    content: &mut dyn ContentProvider,
+) -> std::io::Result<Option<isize>> {
     // Check path extension
     if !path
         .extension()
         .and_then(OsStr::to_str)
         .map_or(false, |ext| file_types.iter().any(|ft| ft == ext))
     {
-        return None;
+        return Ok(None);
     }
 
     // Apply content regex
+    let content = content.get(path)?;
     if let (Some(file_content), Some(content_regex)) = (content, &content_regex) {
         // If the language configuration has a content regex, assign
         // a score based on the length of the first match.
         if let Some(mat) = content_regex.find(&file_content) {
             let score = (mat.end() - mat.start()) as isize;
-            return Some(score);
+            return Ok(Some(score));
         } else {
-            return None;
+            return Ok(None);
         }
     }
 
-    Some(0isize)
+    Ok(Some(0isize))
+}
+
+pub trait ContentProvider {
+    fn get(&mut self, path: &Path) -> std::io::Result<Option<&str>>;
+}
+
+/// FileReader reads files from the filesystem and caches the most recently read file.
+pub struct FileReader {
+    cache: Option<(PathBuf, String)>,
+}
+
+impl FileReader {
+    pub fn new() -> Self {
+        Self { cache: None }
+    }
+
+    pub fn get(&mut self, path: &Path) -> std::io::Result<&str> {
+        if self.cache.as_ref().map_or(true, |(p, _)| p != path) {
+            let content = std::fs::read_to_string(path)?;
+            self.cache = Some((path.to_path_buf(), content));
+        }
+        Ok(&self.cache.as_ref().unwrap().1)
+    }
+}
+
+impl ContentProvider for FileReader {
+    fn get(&mut self, path: &Path) -> std::io::Result<Option<&str>> {
+        self.get(path).map(|c| Some(c))
+    }
+}
+
+impl ContentProvider for Option<&str> {
+    fn get(&mut self, _path: &Path) -> std::io::Result<Option<&str>> {
+        Ok(self.clone())
+    }
 }

tree-sitter-stack-graphs/tests/it/loader.rs

@@ -40,12 +40,12 @@ fn can_load_from_provided_language_configuration() {
         Loader::from_language_configurations(vec![lc], None).expect("Expected loader to succeed");
 
     let tsl = loader
-        .load_tree_sitter_language_for_file(&PATH, None)
+        .load_tree_sitter_language_for_file(&PATH, &mut None)
         .expect("Expected loading tree-sitter language to succeed");
     assert_eq!(tsl, Some(language));
 
     let lc = loader
-        .load_for_file(&PATH, None, &NoCancellation)
+        .load_for_file(&PATH, &mut None, &NoCancellation)
         .expect("Expected loading stack graph language to succeed");
     assert_eq!(lc.map(|lc| lc.language), Some(language));
 }