From f9e1c93f385e15b384e2aea9ef2f9079d5c7a81b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 7 Jan 2026 09:36:26 +0100 Subject: [PATCH 01/10] make migration tool not write files unless they have actual changes --- .../src/expected/NoOp.res.expected | 2 + tests/tools_tests/src/migrate/NoOp.res | 1 + tools/bin/main.ml | 27 ++++---- tools/src/migrate.ml | 64 ++++++++++++------- tools/src/migrate.mli | 2 +- 5 files changed, 56 insertions(+), 40 deletions(-) create mode 100644 tests/tools_tests/src/expected/NoOp.res.expected create mode 100644 tests/tools_tests/src/migrate/NoOp.res diff --git a/tests/tools_tests/src/expected/NoOp.res.expected b/tests/tools_tests/src/expected/NoOp.res.expected new file mode 100644 index 00000000000..01895b7ba3c --- /dev/null +++ b/tests/tools_tests/src/expected/NoOp.res.expected @@ -0,0 +1,2 @@ +let meaningOfLife = 42 + diff --git a/tests/tools_tests/src/migrate/NoOp.res b/tests/tools_tests/src/migrate/NoOp.res new file mode 100644 index 00000000000..50802237b05 --- /dev/null +++ b/tests/tools_tests/src/migrate/NoOp.res @@ -0,0 +1 @@ +let meaningOfLife = 42 diff --git a/tools/bin/main.ml b/tools/bin/main.ml index cd810b53095..ed86b622536 100644 --- a/tools/bin/main.ml +++ b/tools/bin/main.ml @@ -71,12 +71,14 @@ let main () = | "migrate" :: file :: opts -> ( let isStdout = List.mem "--stdout" opts in let outputMode = if isStdout then `Stdout else `File in - match - (Tools.Migrate.migrate ~entryPointFile:file ~outputMode, outputMode) - with - | Ok content, `Stdout -> print_endline content - | result, `File -> logAndExit result - | Error e, _ -> logAndExit (Error e)) + let base = Filename.basename file in + match Tools.Migrate.migrate ~entryPointFile:file ~outputMode with + | Ok (`Changed content) when isStdout -> print_endline content + | Ok (`Unchanged content) when isStdout -> print_endline content + | Ok (`Changed _) -> logAndExit (Ok (base ^ ": File migrated successfully")) + | Ok (`Unchanged _) -> + logAndExit (Ok (base ^ ": File did not need migration")) + | Error e -> logAndExit (Error e)) | "migrate-all" :: root :: _opts -> ( let rootPath = if Filename.is_relative root then Unix.realpath root else root @@ -113,17 +115,10 @@ let main () = let migrated, unchanged, failures = results |> List.fold_left - (fun (migrated, unchanged, failures) (file, res) -> + (fun (migrated, unchanged, failures) (_file, res) -> match res with - | Ok msg -> - let base = Filename.basename file in - if msg = base ^ ": File migrated successfully" then - (migrated + 1, unchanged, failures) - else if msg = base ^ ": File did not need migration" then - (migrated, unchanged + 1, failures) - else - (* Unknown OK message, count as unchanged *) - (migrated, unchanged + 1, failures) + | Ok (`Changed _) -> (migrated + 1, unchanged, failures) + | Ok (`Unchanged _) -> (migrated, unchanged + 1, failures) | Error _ -> (migrated, unchanged, failures + 1)) (0, 0, 0) in diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index b6251dd2464..db588855cb3 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -744,10 +744,6 @@ let migrate ~entryPointFile ~outputMode = in let result = if Filename.check_suffix path ".res" then - let parser = - Res_driver.parsing_engine.parse_implementation ~for_printer:true - in - let {Res_driver.parsetree; comments; source} = parser ~filename:path in match Cmt.loadCmtInfosFromPath ~path with | None -> Error @@ -755,7 +751,17 @@ let migrate ~entryPointFile ~outputMode = "error: failed to run migration for %s because build artifacts \ could not be found. try to build the project" path) + | Some {cmt_extra_info = {deprecated_used = []}} -> ( + match outputMode with + | `Stdout -> + let source = Res_io.read_file ~filename:path in + Ok (`Unchanged source) + | `File -> Ok (`Unchanged "")) | Some {cmt_extra_info = {deprecated_used}} -> + let parser = + Res_driver.parsing_engine.parse_implementation ~for_printer:true + in + let {Res_driver.parsetree; comments; source} = parser ~filename:path in let mapper = makeMapper deprecated_used in let astMapped = mapper.structure mapper parsetree in (* Second pass: apply any post-migration transforms signaled via @apply.transforms *) @@ -769,18 +775,13 @@ let migrate ~entryPointFile ~outputMode = let astTransformed = apply_transforms.structure apply_transforms astMapped in - Ok - ( Res_printer.print_implementation - ~width:Res_printer.default_print_width astTransformed ~comments, - source ) + let contents = + Res_printer.print_implementation + ~width:Res_printer.default_print_width astTransformed ~comments + in + if contents = source then Ok (`Unchanged source) + else Ok (`Changed contents) else if Filename.check_suffix path ".resi" then - let parser = - Res_driver.parsing_engine.parse_interface ~for_printer:true - in - let {Res_driver.parsetree = signature; comments; source} = - parser ~filename:path - in - match Cmt.loadCmtInfosFromPath ~path with | None -> Error @@ -788,10 +789,25 @@ let migrate ~entryPointFile ~outputMode = "error: failed to run migration for %s because build artifacts \ could not be found. try to build the project" path) + | Some {cmt_extra_info = {deprecated_used = []}} -> ( + match outputMode with + | `Stdout -> + let source = Res_io.read_file ~filename:path in + Ok (`Unchanged source) + | `File -> Ok (`Unchanged "")) | Some {cmt_extra_info = {deprecated_used}} -> + let parser = + Res_driver.parsing_engine.parse_interface ~for_printer:true + in + let {Res_driver.parsetree = signature; comments; source} = + parser ~filename:path + in + let mapper = makeMapper deprecated_used in let astMapped = mapper.signature mapper signature in - Ok (Res_printer.print_interface astMapped ~comments, source) + let contents = Res_printer.print_interface astMapped ~comments in + if contents = source then Ok (`Unchanged source) + else Ok (`Changed contents) else Error (Printf.sprintf @@ -800,15 +816,17 @@ let migrate ~entryPointFile ~outputMode = in match result with | Error e -> Error e - | Ok (contents, source) when contents <> source -> ( + | Ok (`Unchanged source) -> ( match outputMode with - | `Stdout -> Ok contents + | `Stdout -> Ok (`Unchanged source) + | `File -> + Ok (`Unchanged (Filename.basename path ^ ": File did not need migration")) + ) + | Ok (`Changed contents) -> ( + match outputMode with + | `Stdout -> Ok (`Changed contents) | `File -> let oc = open_out path in Printf.fprintf oc "%s" contents; close_out oc; - Ok (Filename.basename path ^ ": File migrated successfully")) - | Ok (contents, _) -> ( - match outputMode with - | `Stdout -> Ok contents - | `File -> Ok (Filename.basename path ^ ": File did not need migration")) + Ok (`Changed (Filename.basename path ^ ": File migrated successfully"))) diff --git a/tools/src/migrate.mli b/tools/src/migrate.mli index 1831a90b30c..565e6b4a44b 100644 --- a/tools/src/migrate.mli +++ b/tools/src/migrate.mli @@ -1,4 +1,4 @@ val migrate : entryPointFile:string -> outputMode:[`File | `Stdout] -> - (string, string) result + ([`Changed of string | `Unchanged of string], string) result From 2eaf010b3e4d2351020d55b699d30e3559acd908 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 7 Jan 2026 19:04:54 +0100 Subject: [PATCH 02/10] ensure migrations are not attempted on anything but project files --- tests/dependencies/deprecatedlib/package.json | 11 ++ .../dependencies/deprecatedlib/rescript.json | 6 + .../deprecatedlib/src/DeprecatedDep.res | 7 ++ tests/tools_tests/package.json | 1 + tests/tools_tests/rescript.json | 2 +- .../DependencyDeprecation.res.expected | 2 + .../src/migrate/DependencyDeprecation.res | 1 + tests/tools_tests/test.sh | 3 +- tools/src/migrate.ml | 113 +++++++++++------- tools/src/migrate.mli | 5 + 10 files changed, 106 insertions(+), 45 deletions(-) create mode 100644 tests/dependencies/deprecatedlib/package.json create mode 100644 tests/dependencies/deprecatedlib/rescript.json create mode 100644 tests/dependencies/deprecatedlib/src/DeprecatedDep.res create mode 100644 tests/tools_tests/src/expected/DependencyDeprecation.res.expected create mode 100644 tests/tools_tests/src/migrate/DependencyDeprecation.res diff --git a/tests/dependencies/deprecatedlib/package.json b/tests/dependencies/deprecatedlib/package.json new file mode 100644 index 00000000000..4a95698ff7d --- /dev/null +++ b/tests/dependencies/deprecatedlib/package.json @@ -0,0 +1,11 @@ +{ + "name": "deprecatedlib", + "version": "0.0.1", + "scripts": { + "build": "rescript-legacy build", + "clean": "rescript clean" + }, + "dependencies": { + "rescript": "workspace:^" + } +} diff --git a/tests/dependencies/deprecatedlib/rescript.json b/tests/dependencies/deprecatedlib/rescript.json new file mode 100644 index 00000000000..b6367f07e2d --- /dev/null +++ b/tests/dependencies/deprecatedlib/rescript.json @@ -0,0 +1,6 @@ +{ + "name": "deprecatedlib", + "sources": { "dir": "src", "subdirs": true }, + "package-specs": { "module": "commonjs", "in-source": true }, + "suffix": ".res.js" +} diff --git a/tests/dependencies/deprecatedlib/src/DeprecatedDep.res b/tests/dependencies/deprecatedlib/src/DeprecatedDep.res new file mode 100644 index 00000000000..26733f0f0c5 --- /dev/null +++ b/tests/dependencies/deprecatedlib/src/DeprecatedDep.res @@ -0,0 +1,7 @@ +@deprecated({ + reason: "Use `newThing` instead.", + migrate: DeprecatedDep.newThing(), +}) +let oldThing = () => 1 + +let newThing = () => 2 diff --git a/tests/tools_tests/package.json b/tests/tools_tests/package.json index 46209fd6de9..57abb22bbb0 100644 --- a/tests/tools_tests/package.json +++ b/tests/tools_tests/package.json @@ -8,6 +8,7 @@ }, "dependencies": { "@rescript/react": "link:../dependencies/rescript-react", + "deprecatedlib": "link:../dependencies/deprecatedlib", "rescript": "workspace:^" } } diff --git a/tests/tools_tests/rescript.json b/tests/tools_tests/rescript.json index 3ed41da9887..75312e0b220 100644 --- a/tests/tools_tests/rescript.json +++ b/tests/tools_tests/rescript.json @@ -9,5 +9,5 @@ "in-source": true }, "suffix": ".res.js", - "dependencies": ["@rescript/react"] + "dependencies": ["@rescript/react", "deprecatedlib"] } diff --git a/tests/tools_tests/src/expected/DependencyDeprecation.res.expected b/tests/tools_tests/src/expected/DependencyDeprecation.res.expected new file mode 100644 index 00000000000..acaf9fdc88c --- /dev/null +++ b/tests/tools_tests/src/expected/DependencyDeprecation.res.expected @@ -0,0 +1,2 @@ +let value = DeprecatedDep.oldThing() + diff --git a/tests/tools_tests/src/migrate/DependencyDeprecation.res b/tests/tools_tests/src/migrate/DependencyDeprecation.res new file mode 100644 index 00000000000..6d8716c8be8 --- /dev/null +++ b/tests/tools_tests/src/migrate/DependencyDeprecation.res @@ -0,0 +1 @@ +let value = DeprecatedDep.oldThing() diff --git a/tests/tools_tests/test.sh b/tests/tools_tests/test.sh index 4e44f421708..100d97d9110 100755 --- a/tests/tools_tests/test.sh +++ b/tests/tools_tests/test.sh @@ -36,7 +36,8 @@ done # Test migrate command for file in src/migrate/*.{res,resi}; do output="src/expected/$(basename $file).expected" - ../../_build/install/default/bin/rescript-tools migrate "$file" --stdout > $output + # Capture stderr too so warnings would surface in expected output if they occur. + ../../_build/install/default/bin/rescript-tools migrate "$file" --stdout 2>&1 > $output if [ "$RUNNER_OS" == "Windows" ]; then perl -pi -e 's/\r\n/\n/g' -- $output fi diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index db588855cb3..97ff200ad01 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -3,6 +3,23 @@ open Analysis module StringMap = Map.Make (String) module StringSet = Set.Make (String) module IntSet = Set.Make (Int) +module FileSet = SharedTypes.FileSet + +let filter_deprecations_for_project ~entryPointFile deprecated_used = + let canonical p = try Unix.realpath p with _ -> p in + let uri = Uri.fromPath entryPointFile in + match Packages.getPackage ~uri with + | None -> deprecated_used + | Some package -> + let dependency_paths = + FileSet.fold + (fun p acc -> acc |> StringSet.add p |> StringSet.add (canonical p)) + package.dependenciesFiles StringSet.empty + in + deprecated_used + |> List.filter (fun (d : Cmt_utils.deprecated_used) -> + let loc_path = canonical d.source_loc.Location.loc_start.pos_fname in + not (StringSet.mem loc_path dependency_paths)) (* Public API: migrate ~entryPointFile ~outputMode *) @@ -751,36 +768,42 @@ let migrate ~entryPointFile ~outputMode = "error: failed to run migration for %s because build artifacts \ could not be found. try to build the project" path) - | Some {cmt_extra_info = {deprecated_used = []}} -> ( - match outputMode with - | `Stdout -> - let source = Res_io.read_file ~filename:path in - Ok (`Unchanged source) - | `File -> Ok (`Unchanged "")) | Some {cmt_extra_info = {deprecated_used}} -> - let parser = - Res_driver.parsing_engine.parse_implementation ~for_printer:true + let deprecated_used = + filter_deprecations_for_project ~entryPointFile:path deprecated_used in - let {Res_driver.parsetree; comments; source} = parser ~filename:path in - let mapper = makeMapper deprecated_used in - let astMapped = mapper.structure mapper parsetree in - (* Second pass: apply any post-migration transforms signaled via @apply.transforms *) - let apply_transforms = - let expr mapper (e : Parsetree.expression) = - let e = Ast_mapper.default_mapper.expr mapper e in - MapperUtils.ApplyTransforms.apply_on_self e + if Ext_list.is_empty deprecated_used then + match outputMode with + | `Stdout -> + let source = Res_io.read_file ~filename:path in + Ok (`Unchanged source) + | `File -> Ok (`Unchanged "") + else + let parser = + Res_driver.parsing_engine.parse_implementation ~for_printer:true in - {Ast_mapper.default_mapper with expr} - in - let astTransformed = - apply_transforms.structure apply_transforms astMapped - in - let contents = - Res_printer.print_implementation - ~width:Res_printer.default_print_width astTransformed ~comments - in - if contents = source then Ok (`Unchanged source) - else Ok (`Changed contents) + let {Res_driver.parsetree; comments; source} = + parser ~filename:path + in + let mapper = makeMapper deprecated_used in + let astMapped = mapper.structure mapper parsetree in + (* Second pass: apply any post-migration transforms signaled via @apply.transforms *) + let apply_transforms = + let expr mapper (e : Parsetree.expression) = + let e = Ast_mapper.default_mapper.expr mapper e in + MapperUtils.ApplyTransforms.apply_on_self e + in + {Ast_mapper.default_mapper with expr} + in + let astTransformed = + apply_transforms.structure apply_transforms astMapped + in + let contents = + Res_printer.print_implementation + ~width:Res_printer.default_print_width astTransformed ~comments + in + if contents = source then Ok (`Unchanged source) + else Ok (`Changed contents) else if Filename.check_suffix path ".resi" then match Cmt.loadCmtInfosFromPath ~path with | None -> @@ -789,25 +812,29 @@ let migrate ~entryPointFile ~outputMode = "error: failed to run migration for %s because build artifacts \ could not be found. try to build the project" path) - | Some {cmt_extra_info = {deprecated_used = []}} -> ( - match outputMode with - | `Stdout -> - let source = Res_io.read_file ~filename:path in - Ok (`Unchanged source) - | `File -> Ok (`Unchanged "")) | Some {cmt_extra_info = {deprecated_used}} -> - let parser = - Res_driver.parsing_engine.parse_interface ~for_printer:true - in - let {Res_driver.parsetree = signature; comments; source} = - parser ~filename:path + let deprecated_used = + filter_deprecations_for_project ~entryPointFile:path deprecated_used in + if Ext_list.is_empty deprecated_used then + match outputMode with + | `Stdout -> + let source = Res_io.read_file ~filename:path in + Ok (`Unchanged source) + | `File -> Ok (`Unchanged "") + else + let parser = + Res_driver.parsing_engine.parse_interface ~for_printer:true + in + let {Res_driver.parsetree = signature; comments; source} = + parser ~filename:path + in - let mapper = makeMapper deprecated_used in - let astMapped = mapper.signature mapper signature in - let contents = Res_printer.print_interface astMapped ~comments in - if contents = source then Ok (`Unchanged source) - else Ok (`Changed contents) + let mapper = makeMapper deprecated_used in + let astMapped = mapper.signature mapper signature in + let contents = Res_printer.print_interface astMapped ~comments in + if contents = source then Ok (`Unchanged source) + else Ok (`Changed contents) else Error (Printf.sprintf diff --git a/tools/src/migrate.mli b/tools/src/migrate.mli index 565e6b4a44b..d38f7751b58 100644 --- a/tools/src/migrate.mli +++ b/tools/src/migrate.mli @@ -2,3 +2,8 @@ val migrate : entryPointFile:string -> outputMode:[`File | `Stdout] -> ([`Changed of string | `Unchanged of string], string) result + +val filter_deprecations_for_project : + entryPointFile:string -> + Cmt_utils.deprecated_used list -> + Cmt_utils.deprecated_used list From 9de4952b3d8ee804584884ca00a76ca31371617a Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 7 Jan 2026 19:07:35 +0100 Subject: [PATCH 03/10] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8924a442d2..ea0a69780c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ - Rewatch: enable `--create-sourcedirs` by default (now deprecated when explicitly used). https://github.com/rescript-lang/rescript/pull/8092 - Rewatch: check if filename case for interface and implementation matches. https://github.com/rescript-lang/rescript/pull/8144 +- Migration tool: Do not rewrite or format files unless there are actual migrations performed. https://github.com/rescript-lang/rescript/pull/8157 +- Migration tool: Do not attempt to run migrations on dependencies. https://github.com/rescript-lang/rescript/pull/8157 #### :house: Internal From 72cef3e9a35a71f7c0daa81ca1e555738a523efd Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 7 Jan 2026 19:08:47 +0100 Subject: [PATCH 04/10] update lockfile --- yarn.lock | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/yarn.lock b/yarn.lock index 2624cbc9892..30123bce081 100644 --- a/yarn.lock +++ b/yarn.lock @@ -783,6 +783,7 @@ __metadata: resolution: "@tests/tools@workspace:tests/tools_tests" dependencies: "@rescript/react": "link:../dependencies/rescript-react" + deprecatedlib: "link:../dependencies/deprecatedlib" rescript: "workspace:^" languageName: unknown linkType: soft @@ -1213,6 +1214,20 @@ __metadata: languageName: node linkType: hard +"deprecatedlib@link:../dependencies/deprecatedlib::locator=%40tests%2Ftools%40workspace%3Atests%2Ftools_tests": + version: 0.0.0-use.local + resolution: "deprecatedlib@link:../dependencies/deprecatedlib::locator=%40tests%2Ftools%40workspace%3Atests%2Ftools_tests" + languageName: node + linkType: soft + +"deprecatedlib@workspace:tests/dependencies/deprecatedlib": + version: 0.0.0-use.local + resolution: "deprecatedlib@workspace:tests/dependencies/deprecatedlib" + dependencies: + rescript: "workspace:^" + languageName: unknown + linkType: soft + "diff@npm:^7.0.0": version: 7.0.0 resolution: "diff@npm:7.0.0" From f44f5e45f45667445c37d07f2a7bda6c760acb3b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 8 Jan 2026 10:50:02 +0100 Subject: [PATCH 05/10] reuse package --- tools/bin/main.ml | 8 ++++---- tools/src/migrate.ml | 16 ++++++++++++---- tools/src/migrate.mli | 4 +++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/tools/bin/main.ml b/tools/bin/main.ml index ed86b622536..43afc737e44 100644 --- a/tools/bin/main.ml +++ b/tools/bin/main.ml @@ -72,9 +72,9 @@ let main () = let isStdout = List.mem "--stdout" opts in let outputMode = if isStdout then `Stdout else `File in let base = Filename.basename file in - match Tools.Migrate.migrate ~entryPointFile:file ~outputMode with - | Ok (`Changed content) when isStdout -> print_endline content - | Ok (`Unchanged content) when isStdout -> print_endline content + match Tools.Migrate.migrate file ~outputMode with + | Ok (`Changed content | `Unchanged content) when isStdout -> + print_endline content | Ok (`Changed _) -> logAndExit (Ok (base ^ ": File migrated successfully")) | Ok (`Unchanged _) -> logAndExit (Ok (base ^ ": File did not need migration")) @@ -109,7 +109,7 @@ let main () = if total = 0 then logAndExit (Ok "No source files found to migrate") else let process_one file = - (file, Tools.Migrate.migrate ~entryPointFile:file ~outputMode:`File) + (file, Tools.Migrate.migrate file ~package ~outputMode:`File) in let results = List.map process_one files in let migrated, unchanged, failures = diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index 97ff200ad01..b2a00d2b54c 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -5,10 +5,16 @@ module StringSet = Set.Make (String) module IntSet = Set.Make (Int) module FileSet = SharedTypes.FileSet -let filter_deprecations_for_project ~entryPointFile deprecated_used = +let filter_deprecations_for_project ~entryPointFile ?package deprecated_used = let canonical p = try Unix.realpath p with _ -> p in - let uri = Uri.fromPath entryPointFile in - match Packages.getPackage ~uri with + let package = + match package with + | Some p -> Some p + | None -> + let uri = Uri.fromPath entryPointFile in + Packages.getPackage ~uri + in + match package with | None -> deprecated_used | Some package -> let dependency_paths = @@ -753,7 +759,7 @@ let makeMapper (deprecated_used : Cmt_utils.deprecated_used list) = in mapper -let migrate ~entryPointFile ~outputMode = +let migrate ~outputMode ?package entryPointFile = let path = match Filename.is_relative entryPointFile with | true -> Unix.realpath entryPointFile @@ -771,6 +777,7 @@ let migrate ~entryPointFile ~outputMode = | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = filter_deprecations_for_project ~entryPointFile:path deprecated_used + ?package in if Ext_list.is_empty deprecated_used then match outputMode with @@ -815,6 +822,7 @@ let migrate ~entryPointFile ~outputMode = | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = filter_deprecations_for_project ~entryPointFile:path deprecated_used + ?package in if Ext_list.is_empty deprecated_used then match outputMode with diff --git a/tools/src/migrate.mli b/tools/src/migrate.mli index d38f7751b58..2e8a7e85761 100644 --- a/tools/src/migrate.mli +++ b/tools/src/migrate.mli @@ -1,9 +1,11 @@ val migrate : - entryPointFile:string -> outputMode:[`File | `Stdout] -> + ?package:Analysis.SharedTypes.package -> + string -> ([`Changed of string | `Unchanged of string], string) result val filter_deprecations_for_project : entryPointFile:string -> + ?package:Analysis.SharedTypes.package -> Cmt_utils.deprecated_used list -> Cmt_utils.deprecated_used list From 650d0c6da2aa275e26c508ce274a914b2dcdef9a Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 8 Jan 2026 11:06:19 +0100 Subject: [PATCH 06/10] precompute --- tools/bin/main.ml | 15 +++++++++++++-- tools/src/migrate.ml | 24 ++++++++++++++---------- tools/src/migrate.mli | 8 +++++--- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/tools/bin/main.ml b/tools/bin/main.ml index 43afc737e44..41f2148df2c 100644 --- a/tools/bin/main.ml +++ b/tools/bin/main.ml @@ -72,7 +72,7 @@ let main () = let isStdout = List.mem "--stdout" opts in let outputMode = if isStdout then `Stdout else `File in let base = Filename.basename file in - match Tools.Migrate.migrate file ~outputMode with + match Tools.Migrate.migrate ~outputMode file with | Ok (`Changed content | `Unchanged content) when isStdout -> print_endline content | Ok (`Changed _) -> logAndExit (Ok (base ^ ": File migrated successfully")) @@ -108,8 +108,19 @@ let main () = let total = List.length files in if total = 0 then logAndExit (Ok "No source files found to migrate") else + let canonical p = try Unix.realpath p with _ -> p in + let dependency_paths = + Analysis.SharedTypes.FileSet.fold + (fun p acc -> + acc + |> Analysis.SharedTypes.FileSet.add p + |> Analysis.SharedTypes.FileSet.add (canonical p)) + package.dependenciesFiles Analysis.SharedTypes.FileSet.empty + in let process_one file = - (file, Tools.Migrate.migrate file ~package ~outputMode:`File) + ( file, + Tools.Migrate.migrate ~package ~dependency_paths ~outputMode:`File + file ) in let results = List.map process_one files in let migrated, unchanged, failures = diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index b2a00d2b54c..1de4f66df9d 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -5,7 +5,8 @@ module StringSet = Set.Make (String) module IntSet = Set.Make (Int) module FileSet = SharedTypes.FileSet -let filter_deprecations_for_project ~entryPointFile ?package deprecated_used = +let filter_deprecations_for_project ?package ?dependency_paths ~entryPointFile + ~deprecated_used = let canonical p = try Unix.realpath p with _ -> p in let package = match package with @@ -18,14 +19,17 @@ let filter_deprecations_for_project ~entryPointFile ?package deprecated_used = | None -> deprecated_used | Some package -> let dependency_paths = - FileSet.fold - (fun p acc -> acc |> StringSet.add p |> StringSet.add (canonical p)) - package.dependenciesFiles StringSet.empty + match dependency_paths with + | Some paths -> paths + | None -> + FileSet.fold + (fun p acc -> acc |> FileSet.add p |> FileSet.add (canonical p)) + package.dependenciesFiles FileSet.empty in deprecated_used |> List.filter (fun (d : Cmt_utils.deprecated_used) -> let loc_path = canonical d.source_loc.Location.loc_start.pos_fname in - not (StringSet.mem loc_path dependency_paths)) + not (FileSet.mem loc_path dependency_paths)) (* Public API: migrate ~entryPointFile ~outputMode *) @@ -759,7 +763,7 @@ let makeMapper (deprecated_used : Cmt_utils.deprecated_used list) = in mapper -let migrate ~outputMode ?package entryPointFile = +let migrate ?package ?dependency_paths ~outputMode entryPointFile = let path = match Filename.is_relative entryPointFile with | true -> Unix.realpath entryPointFile @@ -776,8 +780,8 @@ let migrate ~outputMode ?package entryPointFile = path) | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = - filter_deprecations_for_project ~entryPointFile:path deprecated_used - ?package + filter_deprecations_for_project ~entryPointFile:path ~deprecated_used + ?package ?dependency_paths in if Ext_list.is_empty deprecated_used then match outputMode with @@ -821,8 +825,8 @@ let migrate ~outputMode ?package entryPointFile = path) | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = - filter_deprecations_for_project ~entryPointFile:path deprecated_used - ?package + filter_deprecations_for_project ~entryPointFile:path ~deprecated_used + ?package ?dependency_paths in if Ext_list.is_empty deprecated_used then match outputMode with diff --git a/tools/src/migrate.mli b/tools/src/migrate.mli index 2e8a7e85761..cd93b8ee7e9 100644 --- a/tools/src/migrate.mli +++ b/tools/src/migrate.mli @@ -1,11 +1,13 @@ val migrate : - outputMode:[`File | `Stdout] -> ?package:Analysis.SharedTypes.package -> + ?dependency_paths:Analysis.SharedTypes.FileSet.t -> + outputMode:[`File | `Stdout] -> string -> ([`Changed of string | `Unchanged of string], string) result val filter_deprecations_for_project : - entryPointFile:string -> ?package:Analysis.SharedTypes.package -> - Cmt_utils.deprecated_used list -> + ?dependency_paths:Analysis.SharedTypes.FileSet.t -> + entryPointFile:string -> + deprecated_used:Cmt_utils.deprecated_used list -> Cmt_utils.deprecated_used list From 0d98a2244d3da024591ad2639ca0b21069232bf8 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 12 Jan 2026 21:50:37 +0100 Subject: [PATCH 07/10] fix dependency path filter --- tools/src/migrate.ml | 29 ++++++++++++++++++++--------- tools/src/migrate.mli | 6 +++--- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index 1de4f66df9d..8d49b891844 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -5,8 +5,8 @@ module StringSet = Set.Make (String) module IntSet = Set.Make (Int) module FileSet = SharedTypes.FileSet -let filter_deprecations_for_project ?package ?dependency_paths ~entryPointFile - ~deprecated_used = +let filter_deprecations_for_project ?dependency_paths ?package ~deprecated_used + entryPointFile = let canonical p = try Unix.realpath p with _ -> p in let package = match package with @@ -26,10 +26,21 @@ let filter_deprecations_for_project ?package ?dependency_paths ~entryPointFile (fun p acc -> acc |> FileSet.add p |> FileSet.add (canonical p)) package.dependenciesFiles FileSet.empty in + let is_dependency_path p = + FileSet.mem p dependency_paths + || FileSet.mem (canonical p) dependency_paths + in + let def_loc_path (d : Cmt_utils.deprecated_used) = + let loc_of_expr e = e.Parsetree.pexp_loc.Location.loc_start.pos_fname in + match d.migration_template with + | Some e -> Some (loc_of_expr e) + | None -> Option.map loc_of_expr d.migration_in_pipe_chain_template + in deprecated_used |> List.filter (fun (d : Cmt_utils.deprecated_used) -> - let loc_path = canonical d.source_loc.Location.loc_start.pos_fname in - not (FileSet.mem loc_path dependency_paths)) + match def_loc_path d with + | Some def_path when is_dependency_path def_path -> false + | _ -> true) (* Public API: migrate ~entryPointFile ~outputMode *) @@ -763,7 +774,7 @@ let makeMapper (deprecated_used : Cmt_utils.deprecated_used list) = in mapper -let migrate ?package ?dependency_paths ~outputMode entryPointFile = +let migrate ?dependency_paths ?package ~outputMode entryPointFile = let path = match Filename.is_relative entryPointFile with | true -> Unix.realpath entryPointFile @@ -780,8 +791,8 @@ let migrate ?package ?dependency_paths ~outputMode entryPointFile = path) | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = - filter_deprecations_for_project ~entryPointFile:path ~deprecated_used - ?package ?dependency_paths + filter_deprecations_for_project ~deprecated_used ?package + ?dependency_paths path in if Ext_list.is_empty deprecated_used then match outputMode with @@ -825,8 +836,8 @@ let migrate ?package ?dependency_paths ~outputMode entryPointFile = path) | Some {cmt_extra_info = {deprecated_used}} -> let deprecated_used = - filter_deprecations_for_project ~entryPointFile:path ~deprecated_used - ?package ?dependency_paths + filter_deprecations_for_project ~deprecated_used ?package + ?dependency_paths path in if Ext_list.is_empty deprecated_used then match outputMode with diff --git a/tools/src/migrate.mli b/tools/src/migrate.mli index cd93b8ee7e9..1ff89c244c2 100644 --- a/tools/src/migrate.mli +++ b/tools/src/migrate.mli @@ -1,13 +1,13 @@ val migrate : - ?package:Analysis.SharedTypes.package -> ?dependency_paths:Analysis.SharedTypes.FileSet.t -> + ?package:Analysis.SharedTypes.package -> outputMode:[`File | `Stdout] -> string -> ([`Changed of string | `Unchanged of string], string) result val filter_deprecations_for_project : - ?package:Analysis.SharedTypes.package -> ?dependency_paths:Analysis.SharedTypes.FileSet.t -> - entryPointFile:string -> + ?package:Analysis.SharedTypes.package -> deprecated_used:Cmt_utils.deprecated_used list -> + string -> Cmt_utils.deprecated_used list From 9435093d113197ba6d6fdbb1b200d73f8ac452c7 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 12 Jan 2026 22:16:13 +0100 Subject: [PATCH 08/10] fix --- tests/tools_tests/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tools_tests/test.sh b/tests/tools_tests/test.sh index 100d97d9110..c91e22e5d01 100755 --- a/tests/tools_tests/test.sh +++ b/tests/tools_tests/test.sh @@ -37,7 +37,7 @@ done for file in src/migrate/*.{res,resi}; do output="src/expected/$(basename $file).expected" # Capture stderr too so warnings would surface in expected output if they occur. - ../../_build/install/default/bin/rescript-tools migrate "$file" --stdout 2>&1 > $output + ../../_build/install/default/bin/rescript-tools migrate "$file" --stdout > $output 2>&1 if [ "$RUNNER_OS" == "Windows" ]; then perl -pi -e 's/\r\n/\n/g' -- $output fi From aaf832cc95105edfb5e44e273e4189b401da32f5 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 13 Jan 2026 09:44:11 +0100 Subject: [PATCH 09/10] fix logic --- tools/src/migrate.ml | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/tools/src/migrate.ml b/tools/src/migrate.ml index 8d49b891844..793c49cf0c9 100644 --- a/tools/src/migrate.ml +++ b/tools/src/migrate.ml @@ -26,21 +26,18 @@ let filter_deprecations_for_project ?dependency_paths ?package ~deprecated_used (fun p acc -> acc |> FileSet.add p |> FileSet.add (canonical p)) package.dependenciesFiles FileSet.empty in - let is_dependency_path p = - FileSet.mem p dependency_paths - || FileSet.mem (canonical p) dependency_paths - in - let def_loc_path (d : Cmt_utils.deprecated_used) = - let loc_of_expr e = e.Parsetree.pexp_loc.Location.loc_start.pos_fname in - match d.migration_template with - | Some e -> Some (loc_of_expr e) - | None -> Option.map loc_of_expr d.migration_in_pipe_chain_template - in deprecated_used |> List.filter (fun (d : Cmt_utils.deprecated_used) -> - match def_loc_path d with - | Some def_path when is_dependency_path def_path -> false - | _ -> true) + let loc_path = canonical d.source_loc.Location.loc_start.pos_fname in + not + (FileSet.mem loc_path dependency_paths + || FileSet.mem (canonical loc_path) dependency_paths)) + +let migratable_deprecations (deprecated_used : Cmt_utils.deprecated_used list) = + deprecated_used + |> List.filter (fun (d : Cmt_utils.deprecated_used) -> + Option.is_some d.migration_template + || Option.is_some d.migration_in_pipe_chain_template) (* Public API: migrate ~entryPointFile ~outputMode *) @@ -794,7 +791,8 @@ let migrate ?dependency_paths ?package ~outputMode entryPointFile = filter_deprecations_for_project ~deprecated_used ?package ?dependency_paths path in - if Ext_list.is_empty deprecated_used then + let migratable_deprecated = migratable_deprecations deprecated_used in + if Ext_list.is_empty migratable_deprecated then match outputMode with | `Stdout -> let source = Res_io.read_file ~filename:path in @@ -807,7 +805,7 @@ let migrate ?dependency_paths ?package ~outputMode entryPointFile = let {Res_driver.parsetree; comments; source} = parser ~filename:path in - let mapper = makeMapper deprecated_used in + let mapper = makeMapper migratable_deprecated in let astMapped = mapper.structure mapper parsetree in (* Second pass: apply any post-migration transforms signaled via @apply.transforms *) let apply_transforms = @@ -839,7 +837,8 @@ let migrate ?dependency_paths ?package ~outputMode entryPointFile = filter_deprecations_for_project ~deprecated_used ?package ?dependency_paths path in - if Ext_list.is_empty deprecated_used then + let migratable_deprecated = migratable_deprecations deprecated_used in + if Ext_list.is_empty migratable_deprecated then match outputMode with | `Stdout -> let source = Res_io.read_file ~filename:path in @@ -853,7 +852,7 @@ let migrate ?dependency_paths ?package ~outputMode entryPointFile = parser ~filename:path in - let mapper = makeMapper deprecated_used in + let mapper = makeMapper migratable_deprecated in let astMapped = mapper.signature mapper signature in let contents = Res_printer.print_interface astMapped ~comments in if contents = source then Ok (`Unchanged source) From cef32da1d795baff469eb89c60b9f7dc72ea29fe Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 13 Jan 2026 09:50:25 +0100 Subject: [PATCH 10/10] update output --- .../expected/StdlibMigration_Js_Undefined.res.expected | 10 +++++----- .../migrated/Migrated_StdlibMigration_Js_Undefined.res | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/tools_tests/src/expected/StdlibMigration_Js_Undefined.res.expected b/tests/tools_tests/src/expected/StdlibMigration_Js_Undefined.res.expected index 19289cbc746..9fc0ae05099 100644 --- a/tests/tools_tests/src/expected/StdlibMigration_Js_Undefined.res.expected +++ b/tests/tools_tests/src/expected/StdlibMigration_Js_Undefined.res.expected @@ -27,10 +27,10 @@ let toOption2 = Nullable.toOption(Nullable.make(3)) let to_opt1 = Nullable.make(4)->Nullable.toOption let to_opt2 = Nullable.toOption(Nullable.make(4)) -let test1 = Js.Undefined.empty->Js.Undefined.test -let test2 = Js.Undefined.test(Js.Undefined.empty) -let test3 = Js.Undefined.return(5)->Js.Undefined.bind(v => v)->Js.Undefined.test +let test1 = Nullable.undefined->Js.Undefined.test +let test2 = Js.Undefined.test(Nullable.undefined) +let test3 = Nullable.make(5)->Nullable.map(v => v)->Js.Undefined.test -let testAny1 = Js.Undefined.testAny(Js.Undefined.empty) -let testAny2 = Js.Undefined.empty->Js.Undefined.testAny +let testAny1 = Js.Undefined.testAny(Nullable.undefined) +let testAny2 = Nullable.undefined->Js.Undefined.testAny diff --git a/tests/tools_tests/src/migrate/migrated/Migrated_StdlibMigration_Js_Undefined.res b/tests/tools_tests/src/migrate/migrated/Migrated_StdlibMigration_Js_Undefined.res index 47a7a236879..82b5a2ebf10 100644 --- a/tests/tools_tests/src/migrate/migrated/Migrated_StdlibMigration_Js_Undefined.res +++ b/tests/tools_tests/src/migrate/migrated/Migrated_StdlibMigration_Js_Undefined.res @@ -29,9 +29,9 @@ let toOption2 = Nullable.toOption(Nullable.make(3)) let to_opt1 = Nullable.make(4)->Nullable.toOption let to_opt2 = Nullable.toOption(Nullable.make(4)) -let test1 = Js.Undefined.empty->Js.Undefined.test -let test2 = Js.Undefined.test(Js.Undefined.empty) -let test3 = Js.Undefined.return(5)->Js.Undefined.bind(v => v)->Js.Undefined.test +let test1 = Nullable.undefined->Js.Undefined.test +let test2 = Js.Undefined.test(Nullable.undefined) +let test3 = Nullable.make(5)->Nullable.map(v => v)->Js.Undefined.test -let testAny1 = Js.Undefined.testAny(Js.Undefined.empty) -let testAny2 = Js.Undefined.empty->Js.Undefined.testAny +let testAny1 = Js.Undefined.testAny(Nullable.undefined) +let testAny2 = Nullable.undefined->Js.Undefined.testAny