Migration tool fixes #8157
Migration tool fixes #8157
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 650d0c6da2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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)) |
There was a problem hiding this comment.
The dependency filter is keyed off d.source_loc, which is the location of the deprecated use (recorded via Builtin_attributes.check_deprecated at the call site). That means when a project file calls a deprecated API from a dependency, source_loc still points to the project file, so this filter won’t drop it and the migration will still rewrite user code. This contradicts the intended “skip dependency migrations” behavior (e.g., a project’s DeprecatedDep.oldThing() call will still be migrated). To suppress dependency-originated migrations you need to filter by where the deprecated item is defined, not the usage location.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
How would you suggest fixing this?
This fixes so that the migration tool a) does not touch files it does not apply any migrations to (it used to format them), and b) does not attempt to do migrations on dependencies.
Closes #8152, #8096