Fix invalidation when file changed during analysis

src/cli/lib.rs

@@ -686,7 +686,9 @@ fn do_fix(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(&cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(&cwd, config_path, &mut interner)
+            .ok();
     }
 
     config.allowed_issues = None;
@@ -704,6 +706,7 @@ fn do_fix(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((mut analysis_result, successfull_run_data)) = result {
@@ -737,7 +740,9 @@ fn do_remove_unused_fixmes(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -759,6 +764,7 @@ fn do_remove_unused_fixmes(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((mut analysis_result, successful_run_data)) = result {
@@ -814,7 +820,9 @@ fn do_add_fixmes(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -833,6 +841,7 @@ fn do_add_fixmes(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((mut analysis_result, successful_run_data)) = result {
@@ -880,7 +889,9 @@ fn do_migrate(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -921,6 +932,7 @@ fn do_migrate(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((mut analysis_result, successful_run_data)) = result {
@@ -969,7 +981,9 @@ fn do_migration_candidates(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -990,6 +1004,7 @@ fn do_migration_candidates(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok(result) = result {
@@ -1047,7 +1062,9 @@ fn do_codegen(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -1066,6 +1083,7 @@ fn do_codegen(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok(result) = result {
@@ -1159,7 +1177,9 @@ fn do_find_paths(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -1187,6 +1207,7 @@ fn do_find_paths(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((analysis_result, successful_run_data)) = result {
@@ -1224,7 +1245,9 @@ fn do_security_check(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
     config.allowed_issues = None;
 
@@ -1254,6 +1277,7 @@ fn do_security_check(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((analysis_result, successful_run_data)) = result {
@@ -1345,7 +1369,9 @@ fn do_analysis(
     let mut interner = Interner::default();
 
     if config_path.exists() {
-        config.update_from_file(cwd, config_path, &mut interner).ok();
+        config
+            .update_from_file(cwd, config_path, &mut interner)
+            .ok();
     }
 
     // do this after we've loaded from file, as they can be overridden
@@ -1374,6 +1400,7 @@ fn do_analysis(
         None,
         None,
         None,
+        || {},
     );
 
     if let Ok((analysis_result, successful_run_data)) = result {

src/cli/test_runners/test_runner.rs

@@ -255,6 +255,7 @@ impl TestRunner {
             previous_scan_data,
             previous_analysis_result,
             None,
+            || {},
         );
 
         if dir.contains("/migrations/")
@@ -445,14 +446,22 @@ impl TestRunner {
 
         let workdir_base = dir.clone() + "/workdir";
 
-        let mut folders = vec![dir.clone() + "/a", dir.clone() + "/b"];
+        let mut folders = vec![(dir.clone() + "/a", false)];
+
+        if Path::new(&(dir.clone() + "/a-before-analysis")).exists() {
+            folders[0].1 = true;
+        }
+
+        if Path::new(&(dir.clone() + "/b")).exists() {
+            folders.push((dir.clone() + "/b", false));
+        }
 
         if Path::new(&(dir.clone() + "/c")).exists() {
-            folders.push(dir.clone() + "/c");
+            folders.push((dir.clone() + "/c", false));
         }
 
         if Path::new(&(dir.clone() + "/d")).exists() {
-            folders.push(dir.clone() + "/d");
+            folders.push((dir.clone() + "/d", false));
         }
 
         let mut previous_scan_data = None;
@@ -469,7 +478,7 @@ impl TestRunner {
             stub_dirs.push(cwd.clone() + "/third-party/xhp-lib/src");
         }
 
-        for folder in folders {
+        for (folder, change_after_scan) in folders {
             copy_recursively(folder.clone(), workdir_base.clone()).unwrap();
 
             let run_result = hakana_workhorse::scan_and_analyze(
@@ -488,6 +497,12 @@ impl TestRunner {
                 previous_scan_data,
                 previous_analysis_result,
                 None,
+                || {
+                    if change_after_scan {
+                        copy_recursively(folder.clone() + "-before-analysis", workdir_base.clone())
+                            .unwrap();
+                    }
+                },
             );
 
             fs::remove_dir_all(&workdir_base).unwrap();

src/code_info/analysis_result.rs

@@ -6,7 +6,10 @@ use serde::Serialize;
 
 use crate::{
     code_location::FilePath,
-    data_flow::{graph::{DataFlowGraph, GraphKind}, node::DataFlowNodeId},
+    data_flow::{
+        graph::{DataFlowGraph, GraphKind},
+        node::DataFlowNodeId,
+    },
     function_context::FunctionLikeIdentifier,
     issue::{Issue, IssueKind},
     symbol_references::SymbolReferences,
@@ -35,6 +38,7 @@ pub struct AnalysisResult {
     pub time_in_analysis: Duration,
     pub functions_to_migrate: FxHashMap<FunctionLikeIdentifier, bool>,
     pub has_invalid_hack_files: bool,
+    pub changed_during_analysis_files: FxHashSet<FilePath>,
 }
 
 impl AnalysisResult {
@@ -55,6 +59,7 @@ impl AnalysisResult {
             functions_to_migrate: FxHashMap::default(),
             codegen: BTreeMap::default(),
             has_invalid_hack_files: false,
+            changed_during_analysis_files: FxHashSet::default(),
         }
     }
 
@@ -78,6 +83,7 @@ impl AnalysisResult {
         }
         self.functions_to_migrate.extend(other.functions_to_migrate);
         self.codegen.extend(other.codegen);
+        self.changed_during_analysis_files.extend(other.changed_during_analysis_files);
         self.has_invalid_hack_files = self.has_invalid_hack_files || other.has_invalid_hack_files;
     }
 

src/file_scanner_analyzer/analyzer.rs

@@ -1,3 +1,4 @@
+use crate::file::get_file_contents_hash;
 use crate::{get_aast_for_path, update_progressbar, SuccessfulScanData};
 use hakana_analyzer::config::Config;
 use hakana_analyzer::file_analyzer;
@@ -108,11 +109,7 @@ pub fn analyze_files(
                     file_analysis_time += analyze_file(
                         file_path,
                         str_path,
-                        scan_data
-                            .file_system
-                            .file_hashes_and_times
-                            .get(&file_path)
-                            .map(|k| k.1),
+                        scan_data.file_system.file_hashes_and_times.get(&file_path),
                         codebase,
                         interner,
                         &analysis_config,
@@ -155,7 +152,7 @@ pub fn analyze_files(
 fn analyze_file(
     file_path: FilePath,
     str_path: &String,
-    last_updated_time: Option<u64>,
+    last_hash_and_time: Option<&(u64, u64)>,
     codebase: &CodebaseInfo,
     interner: &Interner,
     config: &Arc<Config>,
@@ -173,9 +170,12 @@ fn analyze_file(
             .unwrap()
             .as_micros() as u64;
 
-        if let Some(last_updated_time) = last_updated_time {
-            if updated_time != last_updated_time {
+        if let Some((file_hash, last_updated_time)) = last_hash_and_time {
+            if updated_time != *last_updated_time
+                && get_file_contents_hash(&str_path).unwrap_or(0) != *file_hash
+            {
                 analysis_result.has_invalid_hack_files = true;
+                analysis_result.changed_during_analysis_files.insert(file_path);
                 analysis_result.emitted_issues.insert(
                     file_path,
                     vec![Issue::new(

src/file_scanner_analyzer/file.rs

@@ -22,13 +22,6 @@ pub struct VirtualFileSystem {
 }
 
 impl VirtualFileSystem {
-    pub fn get_contents_hash(&self, file_path: &String) -> Result<u64, std::io::Error> {
-        match fs::read_to_string(file_path) {
-            Ok(file_contents) => Ok(xxhash_rust::xxh3::xxh3_64(file_contents.as_bytes())),
-            Err(error) => Err(error),
-        }
-    }
-
     pub(crate) fn apply_language_server_changes(
         &mut self,
         language_server_changes: FxHashMap<String, FileStatus>,
@@ -265,15 +258,15 @@ impl VirtualFileSystem {
                             if old_update_time == &updated_time {
                                 *old_contents_hash
                             } else if calculate_file_hashes {
-                                self.get_contents_hash(&str_path).unwrap_or(0)
+                                get_file_contents_hash(&str_path).unwrap_or(0)
                             } else {
                                 0
                             }
                         } else {
                             0
                         }
                     } else if calculate_file_hashes {
-                        self.get_contents_hash(&str_path).unwrap_or(0)
+                        get_file_contents_hash(&str_path).unwrap_or(0)
                     } else {
                         0
                     };
@@ -297,3 +290,10 @@ impl VirtualFileSystem {
         }
     }
 }
+
+pub fn get_file_contents_hash(file_path: &String) -> Result<u64, std::io::Error> {
+    match fs::read_to_string(file_path) {
+        Ok(file_contents) => Ok(xxhash_rust::xxh3::xxh3_64(file_contents.as_bytes())),
+        Err(error) => Err(error),
+    }
+}