Enable create-sourcedirs by default in rewatch

cristianoc · cristianoc · commit 1b2651a9c874 · 2025-12-15T13:46:51.000+01:00
- Change --create-sourcedirs to default to true (always create .sourcedirs.json)
- Hide the flag from help since it's now always enabled
- Add deprecation warning when flag is explicitly used
- Fix package name mismatches in test projects:
  - deadcode rescript.json: sample-typescript-app -&gt; @tests/reanalyze-deadcode
  - rescript-react package.json: @tests/rescript-react -&gt; @rescript/react
diff --git a/analysis/reanalyze/src/CmtCache.ml b/analysis/reanalyze/src/CmtCache.ml
@@ -29,14 +29,10 @@ let clear () = Marshal_cache.clear ()
     The next read will re-load the file from disk. *)
 let invalidate path = Marshal_cache.invalidate path
 
+type stats = {entry_count: int; mapped_bytes: int}
 (** Cache statistics *)
-type stats = {
-  entry_count: int;
-  mapped_bytes: int;
-}
 
 (** Get cache statistics *)
 let stats () : stats =
   let s = Marshal_cache.stats () in
-  { entry_count = s.entry_count; mapped_bytes = s.mapped_bytes }
-
+  {entry_count = s.entry_count; mapped_bytes = s.mapped_bytes}
diff --git a/analysis/reanalyze/src/CmtCache.mli b/analysis/reanalyze/src/CmtCache.mli
@@ -17,12 +17,8 @@ val clear : unit -> unit
 val invalidate : string -> unit
 (** Invalidate a specific path in the cache. *)
 
-type stats = {
-  entry_count: int;
-  mapped_bytes: int;
-}
+type stats = {entry_count: int; mapped_bytes: int}
 (** Cache statistics *)
 
 val stats : unit -> stats
 (** Get cache statistics *)
-
diff --git a/analysis/reanalyze/src/ReactiveAnalysis.ml b/analysis/reanalyze/src/ReactiveAnalysis.ml
@@ -6,25 +6,25 @@
 
 [@@@alert "-unsafe"]
 
-(** Result of processing a single CMT file *)
 type cmt_file_result = {
   dce_data: DceFileProcessing.file_data option;
   exception_data: Exception.file_result option;
 }
+(** Result of processing a single CMT file *)
 
-(** Result of processing all CMT files *)
 type all_files_result = {
   dce_data_list: DceFileProcessing.file_data list;
   exception_results: Exception.file_result list;
 }
+(** Result of processing all CMT files *)
 
-(** Cached file_data for a single CMT file.
-    We cache the processed result, not just the raw CMT data. *)
 type cached_file = {
   path: string;
   file_data: DceFileProcessing.file_data option;
   exception_data: Exception.file_result option;
 }
+(** Cached file_data for a single CMT file.
+    We cache the processed result, not just the raw CMT data. *)
 
 (** The file cache - maps CMT paths to processed results *)
 let file_cache : (string, cached_file) Hashtbl.t = Hashtbl.create 1024
@@ -81,60 +81,62 @@ let process_cmt_infos ~config ~cmtFilePath cmt_infos : cmt_file_result option =
     Returns the cached result if the file hasn't changed since last access. *)
 let process_cmt_cached ~config cmtFilePath : cmt_file_result option =
   match CmtCache.read_cmt_if_changed cmtFilePath with
-  | None ->
+  | None -> (
     (* File unchanged - return cached result *)
-    (match Hashtbl.find_opt file_cache cmtFilePath with
-     | Some cached -> 
-       Some { dce_data = cached.file_data; exception_data = cached.exception_data }
-     | None ->
-       (* First time seeing this file - shouldn't happen, but handle gracefully *)
-       None)
+    match Hashtbl.find_opt file_cache cmtFilePath with
+    | Some cached ->
+      Some {dce_data = cached.file_data; exception_data = cached.exception_data}
+    | None ->
+      (* First time seeing this file - shouldn't happen, but handle gracefully *)
+      None)
   | Some cmt_infos ->
     (* File changed or new - process it *)
     let result = process_cmt_infos ~config ~cmtFilePath cmt_infos in
     (* Cache the result *)
     (match result with
-     | Some r ->
-       Hashtbl.replace file_cache cmtFilePath {
-         path = cmtFilePath;
-         file_data = r.dce_data;
-         exception_data = r.exception_data;
-       }
-     | None -> ());
+    | Some r ->
+      Hashtbl.replace file_cache cmtFilePath
+        {
+          path = cmtFilePath;
+          file_data = r.dce_data;
+          exception_data = r.exception_data;
+        }
+    | None -> ());
     result
 
 (** Process all files incrementally.
     First run processes all files. Subsequent runs only process changed files. *)
 let process_files_incremental ~config cmtFilePaths : all_files_result =
   Timing.time_phase `FileLoading (fun () ->
-    let dce_data_list = ref [] in
-    let exception_results = ref [] in
-    let processed = ref 0 in
-    let from_cache = ref 0 in
-    
-    cmtFilePaths |> List.iter (fun cmtFilePath ->
-      (* Check if file was in cache *before* processing *)
-      let was_cached = Hashtbl.mem file_cache cmtFilePath in
-      match process_cmt_cached ~config cmtFilePath with
-      | Some {dce_data; exception_data} ->
-        (match dce_data with
-         | Some data -> dce_data_list := data :: !dce_data_list
-         | None -> ());
-        (match exception_data with
-         | Some data -> exception_results := data :: !exception_results
-         | None -> ());
-        (* Track whether it was from cache *)
-        if was_cached then
-          incr from_cache
-        else
-          incr processed
-      | None -> ()
-    );
-    
-    if !Cli.timing then
-      Printf.eprintf "Reactive: %d files processed, %d from cache\n%!" !processed !from_cache;
-    
-    {dce_data_list = List.rev !dce_data_list; exception_results = List.rev !exception_results})
+      let dce_data_list = ref [] in
+      let exception_results = ref [] in
+      let processed = ref 0 in
+      let from_cache = ref 0 in
+
+      cmtFilePaths
+      |> List.iter (fun cmtFilePath ->
+             (* Check if file was in cache *before* processing *)
+             let was_cached = Hashtbl.mem file_cache cmtFilePath in
+             match process_cmt_cached ~config cmtFilePath with
+             | Some {dce_data; exception_data} ->
+               (match dce_data with
+               | Some data -> dce_data_list := data :: !dce_data_list
+               | None -> ());
+               (match exception_data with
+               | Some data -> exception_results := data :: !exception_results
+               | None -> ());
+               (* Track whether it was from cache *)
+               if was_cached then incr from_cache else incr processed
+             | None -> ());
+
+      if !Cli.timing then
+        Printf.eprintf "Reactive: %d files processed, %d from cache\n%!"
+          !processed !from_cache;
+
+      {
+        dce_data_list = List.rev !dce_data_list;
+        exception_results = List.rev !exception_results;
+      })
 
 (** Clear all cached file data *)
 let clear () =
@@ -146,4 +148,3 @@ let stats () =
   let file_count = Hashtbl.length file_cache in
   let cmt_stats = CmtCache.stats () in
   (file_count, cmt_stats)
-
diff --git a/analysis/reanalyze/src/Reanalyze.ml b/analysis/reanalyze/src/Reanalyze.ml
@@ -211,9 +211,14 @@ let processCmtFiles ~config ~cmtRoot : all_files_result =
   let cmtFilePaths = collectCmtFilePaths ~cmtRoot in
   (* Reactive mode: use incremental processing that skips unchanged files *)
   if !Cli.reactive then
-    let result = ReactiveAnalysis.process_files_incremental ~config cmtFilePaths in
-    {dce_data_list = result.dce_data_list; exception_results = result.exception_results}
-  else begin
+    let result =
+      ReactiveAnalysis.process_files_incremental ~config cmtFilePaths
+    in
+    {
+      dce_data_list = result.dce_data_list;
+      exception_results = result.exception_results;
+    }
+  else
     let numDomains =
       match !Cli.parallel with
       | n when n > 0 -> n
@@ -228,7 +233,6 @@ let processCmtFiles ~config ~cmtRoot : all_files_result =
           (List.length cmtFilePaths);
       processFilesParallel ~config ~numDomains cmtFilePaths)
     else processFilesSequential ~config cmtFilePaths
-  end
 
 (* Shuffle a list using Fisher-Yates algorithm *)
 let shuffle_list lst =
@@ -362,11 +366,10 @@ let runAnalysisAndReport ~cmtRoot =
     if numRuns > 1 && !Cli.timing then
       Printf.eprintf "\n=== Run %d/%d ===\n%!" run numRuns;
     runAnalysis ~dce_config ~cmtRoot;
-    if run = numRuns then begin
+    if run = numRuns then (
       (* Only report on last run *)
       Log_.Stats.report ~config:dce_config;
-      Log_.Stats.clear ()
-    end;
+      Log_.Stats.clear ());
     Timing.report ()
   done;
   if !Cli.json then EmitJson.finish ()
diff --git a/analysis/vendor/skip-lite/dune b/analysis/vendor/skip-lite/dune
@@ -1,7 +1,9 @@
 ; skip-lite vendor directory
+
 (dirs marshal_cache reactive_file_collection)
 
 ; Test executable for CMT file support
+
 (executable
  (name test_cmt)
  (modules test_cmt)
diff --git a/analysis/vendor/skip-lite/marshal_cache/dune b/analysis/vendor/skip-lite/marshal_cache/dune
@@ -3,5 +3,6 @@
  (foreign_stubs
   (language cxx)
   (names marshal_cache_stubs)
-  (flags (:standard -std=c++17)))
+  (flags
+   (:standard -std=c++17)))
  (c_library_flags (-lstdc++)))
diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs
@@ -197,9 +197,9 @@ pub struct AfterBuildArg {
 
 #[derive(Args, Debug, Clone, Copy)]
 pub struct CreateSourceDirsArg {
-    /// Create a source_dirs.json file at the root of the monorepo, needed for Reanalyze.
-    #[arg(short, long, default_value_t = false, num_args = 0..=1)]
-    pub create_sourcedirs: bool,
+    /// Deprecated: source_dirs.json is now always created.
+    #[arg(short, long, num_args = 0..=1, default_missing_value = "true", hide = true)]
+    pub create_sourcedirs: Option<bool>,
 }
 
 #[derive(Args, Debug, Clone, Copy)]
@@ -488,11 +488,10 @@ impl Deref for AfterBuildArg {
     }
 }
 
-impl Deref for CreateSourceDirsArg {
-    type Target = bool;
-
-    fn deref(&self) -> &Self::Target {
-        &self.create_sourcedirs
+impl CreateSourceDirsArg {
+    /// Returns true if the flag was explicitly passed on the command line.
+    pub fn was_explicitly_set(&self) -> bool {
+        self.create_sourcedirs.is_some()
     }
 }
 
diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs
@@ -46,12 +46,18 @@ fn main() -> Result<()> {
                 );
             }
 
+            if build_args.create_sourcedirs.was_explicitly_set() {
+                log::warn!(
+                    "`--create-sourcedirs` is deprecated: source_dirs.json is now always created. Please remove this flag from your command."
+                );
+            }
+
             match build::build(
                 &build_args.filter,
                 Path::new(&build_args.folder as &str),
                 show_progress,
                 build_args.no_timing,
-                *build_args.create_sourcedirs,
+                true, // create_sourcedirs is now always enabled
                 plain_output,
                 (*build_args.warn_error).clone(),
             ) {
@@ -76,12 +82,18 @@ fn main() -> Result<()> {
                 );
             }
 
+            if watch_args.create_sourcedirs.was_explicitly_set() {
+                log::warn!(
+                    "`--create-sourcedirs` is deprecated: source_dirs.json is now always created. Please remove this flag from your command."
+                );
+            }
+
             match watcher::start(
                 &watch_args.filter,
                 show_progress,
                 &watch_args.folder,
                 (*watch_args.after_build).clone(),
-                *watch_args.create_sourcedirs,
+                true, // create_sourcedirs is now always enabled
                 plain_output,
                 (*watch_args.warn_error).clone(),
             ) {
diff --git a/tests/analysis_tests/tests-reanalyze/deadcode/package.json b/tests/analysis_tests/tests-reanalyze/deadcode/package.json
@@ -2,8 +2,8 @@
   "name": "@tests/reanalyze-deadcode",
   "private": true,
   "scripts": {
-    "build": "rescript-legacy build",
-    "clean": "rescript-legacy clean"
+    "build": "rescript build",
+    "clean": "rescript clean"
   },
   "dependencies": {
     "@rescript/react": "link:../../../dependencies/rescript-react",
diff --git a/tests/analysis_tests/tests-reanalyze/deadcode/rescript.json b/tests/analysis_tests/tests-reanalyze/deadcode/rescript.json
@@ -5,7 +5,7 @@
     "unsuppress": [],
     "transitive": true
   },
-  "name": "sample-typescript-app",
+  "name": "@tests/reanalyze-deadcode",
   "jsx": { "version": 4 },
   "dependencies": ["@rescript/react"],
   "sources": [
diff --git a/tests/dependencies/rescript-react/package.json b/tests/dependencies/rescript-react/package.json
@@ -1,4 +1,4 @@
 {
-  "name": "@tests/rescript-react",
+  "name": "@rescript/react",
   "private": true
 }

Original file line number	Diff line number	Diff line change
`@@ -197,9 +197,9 @@ pub struct AfterBuildArg {`
`197`	`197`
`198`	`198`	`#[derive(Args, Debug, Clone, Copy)]`
`199`	`199`	`pub struct CreateSourceDirsArg {`
`200`		`- /// Create a source_dirs.json file at the root of the monorepo, needed for Reanalyze.`
`201`		`- #[arg(short, long, default_value_t = false, num_args = 0..=1)]`
`202`		`- pub create_sourcedirs: bool,`
	`200`	`+ /// Deprecated: source_dirs.json is now always created.`
	`201`	`+ #[arg(short, long, num_args = 0..=1, default_missing_value = "true", hide = true)]`
	`202`	`+ pub create_sourcedirs: Option<bool>,`
`203`	`203`	`}`
`204`	`204`
`205`	`205`	`#[derive(Args, Debug, Clone, Copy)]`
`@@ -488,11 +488,10 @@ impl Deref for AfterBuildArg {`
`488`	`488`	`}`
`489`	`489`	`}`
`490`	`490`
`491`		`-impl Deref for CreateSourceDirsArg {`
`492`		`- type Target = bool;`
`493`		`-`
`494`		`- fn deref(&self) -> &Self::Target {`
`495`		`- &self.create_sourcedirs`
	`491`	`+impl CreateSourceDirsArg {`
	`492`	`+ /// Returns true if the flag was explicitly passed on the command line.`
	`493`	`+ pub fn was_explicitly_set(&self) -> bool {`
	`494`	`+ self.create_sourcedirs.is_some()`
`496`	`495`	`}`
`497`	`496`	`}`
`498`	`497`