Leave files untouched when no migration applies

changes/migrate-only-changed-files.fix.md

@@ -0,0 +1 @@
+- Make `rescript-tools migrate` leave files untouched when no migration applies instead of reformatting them.

scripts/format.sh

@@ -8,7 +8,7 @@ echo Formatting OCaml code...
 opam exec -- dune build @fmt --auto-promote
 
 echo Formatting ReScript code...
-files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
+files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
 ./cli/rescript.js format $files
 
 echo Formatting JS code...

scripts/format_check.sh

@@ -17,7 +17,7 @@ case "$(uname -s)" in
     fi
 
     echo "Checking ReScript code formatting..."
-    files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
+    files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
     if ./cli/rescript.js format --check $files; then
       printf "${successGreen}✅ ReScript code formatting ok.${reset}\n"
     else

tests/tools_tests/src/expected/ApplyTransforms.res.expected

@@ -11,4 +11,3 @@ let make = executor => makeTask((resolve, reject) => executor(~resolve, ~reject)
 let sum = (~a, ~b) => a + b
 
 let add = (~x, ~y, ()) => sum(~a=x, ~b=y)
-

tests/tools_tests/src/expected/ApplyTransforms.resi.expected

@@ -19,4 +19,3 @@ let makeTask: ((int => unit, string => unit) => unit) => task<int>
 let add: (~x: int, ~y: int, unit) => int
 
 let sum: (~a: int, ~b: int) => int
-

tests/tools_tests/src/expected/ApplyTransformsUsage.res.expected

@@ -1,4 +1,3 @@
 let task = ApplyTransforms.makeTask((resolve, _) => resolve(1))
 
 let total = ApplyTransforms.sum(~a=1, ~b=2)
-

tests/tools_tests/src/expected/DeprecatedStuff.res.expected

@@ -9,4 +9,3 @@ module Constants = {
 }
 
 let deprecatedThing = [1, 2]
-

tests/tools_tests/src/expected/DeprecatedStuff.resi.expected

@@ -24,4 +24,3 @@ module Constants: {
   migrate: DeprecatedStuff.Constants.otherThing,
 })
 let deprecatedThing: array<int>
-

tests/tools_tests/src/expected/FileToMigrate.res.expected

@@ -8,4 +8,3 @@ let shift1 = Array.shift([1, 2, 3])
 let shift2 = [1, 2, 3]->Array.shift
 
 let deprecatedThing1 = DeprecatedStuff.Constants.otherThing
-

tests/tools_tests/src/expected/NoMigration.res.expected

@@ -0,0 +1,7 @@
+// No migrations apply here, so the file must stay byte-identical
+// even though it is not formatted the way the printer would format it.
+let   add = (a, b) =>   a + b
+
+
+let numbers = [1,2,3]->Array.map(x => add(x, 1))
+let other = DeprecatedStuff.Constants.otherThing

tests/tools_tests/src/expected/NoMigration.resi.expected

@@ -0,0 +1,5 @@
+let   add: (int, int) =>   int
+
+
+let numbers: array<int>
+let other: array<int>

tests/tools_tests/src/expected/OptionalArgRename.res.expected

@@ -9,4 +9,3 @@ let _ = Target.doStuff
 external doStuff: (~oldName: option<int>=?) => unit = "doStuff"
 
 /* Intentionally no usage here; this test exercises migration config parsing and ensures optional label rename is preserved and doesn’t crash. */
-

tests/tools_tests/src/expected/OptionalArgRename.resi.expected

@@ -3,4 +3,3 @@
   migrate: OptionalArgRename.Target.doStuff(~newName=%insert.labelledArgument("oldName")),
 })
 external doStuff: (~oldName: option<int>=?) => unit = "doStuff"
-

tests/tools_tests/src/expected/PipeAndRecursive.res.expected

@@ -37,4 +37,3 @@ let many = PipeAndRecursive.Target.a(
     ),
   ),
 )
-

tests/tools_tests/src/expected/StdlibMigration_DataView.res.expected

@@ -1,3 +1,2 @@
 let a = DataView.fromBuffer(ArrayBuffer.make(8), ~byteOffset=2)
 let b = DataView.fromBuffer(ArrayBuffer.make(8), ~byteOffset=2, ~length=4)
-

tests/tools_tests/src/expected/StdlibMigration_Error.res.expected

@@ -32,4 +32,3 @@ let uriErr = JsError.URIError.make("uri error")
 
 let ignore1 = err->JsError.ignore
 let ignore2 = JsError.ignore(err)
-

tests/tools_tests/src/expected/StdlibMigration_Exn.res.expected

@@ -48,4 +48,3 @@ let catchingExnErrorWithSwitch = switch someThrowingJsCode() {
 let catchingExnErrorWithTryCatch = try {someThrowingJsCode()} catch {
 | JsExn(e) => e->JsExn.message
 }
-

tests/tools_tests/src/expected/StdlibMigration_Int.res.expected

@@ -5,4 +5,3 @@ let result4 = Int.shiftLeft(1, 2)
 let result5 = Int.shiftRightUnsigned(1, 2)
 let result6 = Int.shiftRight(1, 2)
 let result7 = Int.bitwiseNot(0)
-

tests/tools_tests/src/expected/StdlibMigration_JSON_Stdlib.res.expected

@@ -12,4 +12,3 @@ let a2 = JSON.stringifyAnyWithReplacer(1, (_k, v) => v)
 let a3 = JSON.stringifyAnyWithReplacerAndIndent(1, (_k, v) => v, 2)
 let a4 = JSON.stringifyAnyWithFilter(1, ["a"])
 let a5 = JSON.stringifyAnyWithFilterAndIndent(1, ["a"], 2)
-

tests/tools_tests/src/expected/StdlibMigration_Pervasives.res.expected

@@ -1,2 +1 @@
 throw(Failure("test"))
-

tests/tools_tests/src/expected/StdlibMigration_StdlibArray.res.expected

@@ -18,4 +18,3 @@ let f1 = [1, 2, 3, 4]->Array.slice(~start=2)
 let g1 = [1, 2]->Array.lastIndexOf(1, ~from=1)
 
 let h1 = [1, 2]->Array.getUnsafe(1)
-

tests/tools_tests/src/expected/StdlibMigration_StdlibListLazyPromise.res.expected

@@ -11,4 +11,3 @@ let b1 = Lazy.isEvaluated(lazy1)
 
 let p = Promise.resolve(5)
 let _ = Promise.ignore(p)
-

tests/tools_tests/src/expected/StdlibMigration_StdlibOptionResultNull.res.expected

@@ -13,4 +13,3 @@ let n3 = Null.mapOr(Null.make(3), 0, x => x)
 let nb1 = Nullable.getOrThrow(Nullable.make(3))
 let nb2 = Nullable.getOr(Nullable.null, 0)
 let nb3 = Nullable.mapOr(Nullable.make(3), 0, x => x)
-

tests/tools_tests/src/expected/StdlibMigration_StdlibPrimitives.res.expected

@@ -16,4 +16,3 @@ let buf = ArrayBuffer.make(8)
 let ab1 = buf->ArrayBuffer.slice(~start=2)
 
 let re1 = RegExp.fromString("\\w+", ~flags="g")
-

tests/tools_tests/src/expected/StdlibMigration_StdlibString.res.expected

@@ -35,4 +35,3 @@ let r3 =
     ~offset as _,
     ~input as _,
   ) => group1 ++ group2 ++ group3)
-

tests/tools_tests/src/expected/StdlibMigration_TypedArray_Operations.res.expected

@@ -4,4 +4,3 @@ let a1 = ta->TypedArray.copyWithin(~target=1, ~start=2)
 let a2 = ta->TypedArray.fill(9, ~start=1)
 let a3 = ta->TypedArray.slice(~start=1)
 let a4 = ta->TypedArray.subarray(~start=1)
-

tests/tools_tests/src/expected/StdlibMigration_TypedArray_Stdlib.res.expected

@@ -1,4 +1,3 @@
 let a = Uint8Array.fromBuffer(ArrayBuffer.make(8), ~byteOffset=2)
 let b = Uint8Array.fromBuffer(ArrayBuffer.make(8), ~byteOffset=2, ~length=2)
 let c = Uint8Array.fromArrayLikeOrIterable([1, 2], ~map=(v, _i) => v)
-

tests/tools_tests/src/migrate/NoMigration.res

@@ -0,0 +1,7 @@
+// No migrations apply here, so the file must stay byte-identical
+// even though it is not formatted the way the printer would format it.
+let   add = (a, b) =>   a + b
+
+
+let numbers = [1,2,3]->Array.map(x => add(x, 1))
+let other = DeprecatedStuff.Constants.otherThing

tests/tools_tests/src/migrate/NoMigration.resi

@@ -0,0 +1,5 @@
+let   add: (int, int) =>   int
+
+
+let numbers: array<int>
+let other: array<int>

tests/tools_tests/test.sh

@@ -62,6 +62,21 @@ for file in src/migrate/*.{res,resi}; do
   fi
 done
 
+# Migrating a file in place must leave it untouched when no migration applies,
+# and --stdout must print it byte for byte
+for file in src/migrate/NoMigration.{res,resi}; do
+  if ! cmp -s "$file" "src/expected/$(basename $file).expected"; then
+    echo "Expected migrate --stdout to print $file unchanged" >&2
+    exit 1
+  fi
+  before="$(cksum < "$file")"
+  ../../_build/install/default/bin/rescript-tools migrate "$file" > /dev/null
+  if [ "$(cksum < "$file")" != "$before" ]; then
+    echo "Expected migrate to leave $file unchanged" >&2
+    exit 1
+  fi
+done
+
 # Move migrated files to expected directory so they can be compiled in the project
 for file in src/migrate/StdlibMigration_*.res; do
   expected_file="src/expected/$(basename $file).expected"

tools/bin/main.ml

@@ -75,7 +75,7 @@ let main () =
     match
       (Tools.Migrate.migrate ~entry_point_file:file ~output_mode, output_mode)
     with
-    | Ok content, `Stdout -> print_endline content
+    | Ok content, `Stdout -> print_string content
     | result, `File -> log_and_exit result
     | Error e, _ -> log_and_exit (Error e))
   | "migrate-all" :: root :: _opts -> (

tools/src/migrate.md

@@ -7,6 +7,7 @@ This document captures what the migration framework currently supports, based on
 - Targets: `*.res` and `*.resi` files.
 - Requires build artifacts for the file’s module (CMT/CMTI). If not found, migration aborts with an error message asking to build the project.
 - Output modes: write back to file or print to stdout.
+- Files where no migration applies are left untouched (and printed as is to stdout); only migrated files are reprinted.
 
 ## Deprecation Sources
 

tools/src/migrate.ml

@@ -774,10 +774,16 @@ let migrate ~entry_point_file ~output_mode =
         let ast_transformed =
           apply_transforms.structure apply_transforms ast_mapped
         in
-        Ok
-          ( Res_printer.print_implementation
-              ~width:Res_printer.default_print_width ast_transformed ~comments,
-            source )
+        (* Only reprint when a migration changed the AST, so that files
+           without migrations keep their formatting. *)
+        if ast_transformed = parsetree then Ok (None, source)
+        else
+          Ok
+            ( Some
+                (Res_printer.print_implementation
+                   ~width:Res_printer.default_print_width ast_transformed
+                   ~comments),
+              source )
     else if Filename.check_suffix path ".resi" then
       let parser = Res_driver.parsing_engine.parse_interface in
       let {Res_driver.parsetree = signature; comments; source} =
@@ -794,7 +800,8 @@ let migrate ~entry_point_file ~output_mode =
       | Some {cmt_extra_info = {deprecated_used}} ->
         let mapper = make_mapper deprecated_used in
         let ast_mapped = mapper.signature mapper signature in
-        Ok (Res_printer.print_interface ast_mapped ~comments, source)
+        if ast_mapped = signature then Ok (None, source)
+        else Ok (Some (Res_printer.print_interface ast_mapped ~comments), source)
     else
       Error
         (Printf.sprintf
@@ -803,15 +810,15 @@ let migrate ~entry_point_file ~output_mode =
   in
   match result with
   | Error e -> Error e
-  | Ok (contents, source) when contents <> source -> (
+  | Ok (Some contents, source) when contents <> source -> (
     match output_mode with
     | `Stdout -> Ok 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, _) -> (
+  | Ok (_, source) -> (
     match output_mode with
-    | `Stdout -> Ok contents
+    | `Stdout -> Ok source
     | `File -> Ok (Filename.basename path ^ ": File did not need migration"))