Skip to content

Handle absolute paths in typegen #7104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 21, 2024
Merged

Handle absolute paths in typegen #7104

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c5dd009
refactor: extract fun from process_cmt_file
Bushuo Oct 17, 2024
678e6d4
refactor: read the .cmt earlier
Bushuo Oct 17, 2024
3accab4
feat: handle paths via source_file instead of cmt_file
Bushuo Oct 17, 2024
8d22164
add project root to default config
Bushuo Oct 17, 2024
f62fd61
refactor: extract funs to remove duplication
Bushuo Oct 18, 2024
3b6ad82
fix: consider file seperators
Bushuo Oct 18, 2024
11f9c42
fix: please static checker
Bushuo Oct 19, 2024
6f9d118
chore: update changelog
Bushuo Oct 20, 2024
8da295e
make find source file return only absolute paths
Bushuo Oct 20, 2024
5bb3db9
refactor: cleanup get_output_file and get_output_file_relative
Bushuo Oct 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: handle paths via source_file instead of cmt_file
  • Loading branch information
@Bushuo
Bushuo committed Oct 20, 2024
commit 3accab44960c233af798c62babdf6e0043ac0a8f
6 changes: 4 additions & 2 deletions compiler/gentype/GenTypeMain.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@ let process_cmt_file cmt =
if !Debug.basic then Log_.item "Cmt %s\n" cmt;
let cmt_file = cmt |> Paths.get_cmt_file in
if cmt_file <> "" then
let output_file = cmt |> Paths.get_output_file ~config in
let output_file_relative = cmt |> Paths.get_output_file_relative ~config in
let file_name = cmt |> Paths.get_module_name in
let is_interface = Filename.check_suffix cmt_file ".cmti" in
let input_cmt, has_gentype_annotations =
Expand All @@ -155,6 +153,10 @@ let process_cmt_file cmt =
| true -> ".resi"
| false -> ".res")
in
let output_file = source_file |> Paths.get_output_file ~config in
let output_file_relative =
source_file |> Paths.get_output_file_relative ~config
in
let resolver =
ModuleResolver.create_lazy_resolver ~config
~extensions:[".res"; ".shim.ts"] ~exclude_file:(fun fname ->
Expand Down
36 changes: 31 additions & 5 deletions compiler/gentype/Paths.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,43 @@ let find_name_space cmt =
cmt |> Filename.basename |> (Filename.chop_extension [@doesNotRaise])
|> keep_after_dash

let get_output_file_relative ~config cmt =
(cmt |> handle_namespace) ^ ModuleExtension.ts_input_file_suffix ~config
let remove_project_root_from_absolute_path ~(config : Config.t) source_path =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bit that I haven't really reviewed properly yet.
Might not be a robust way to get a relative path. Especially Windows vs linux etc.
Can't remember if the code already does this somewhere else.
Or, poking around in the Filename module, there's more one can use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored the function to use Filename.concat for appending the directory separator when missing. Additionally, it now verifies if the prefix exists and returns the original path if not. I also renamed the function for better clarity.

let i = String.length config.project_root + 1 in
let n = String.length source_path - i in
(String.sub source_path i n [@doesNotRaise])

let get_output_file ~(config : Config.t) cmt =
Filename.concat config.project_root (get_output_file_relative ~config cmt)
let get_output_file_relative ~config source_path =
if Filename.is_relative source_path then
(source_path |> handle_namespace)
^ ModuleExtension.ts_input_file_suffix ~config
else
let relative_path =
remove_project_root_from_absolute_path ~config source_path
in
(relative_path |> handle_namespace)
^ ModuleExtension.ts_input_file_suffix ~config

let get_output_file ~(config : Config.t) sourcePath =
if Filename.is_relative sourcePath then
(* assuming a relative path from the project root *)
Filename.concat config.project_root
(get_output_file_relative ~config sourcePath)
else
(* we want to place the output beside the source file *)
let relative_path =
remove_project_root_from_absolute_path ~config sourcePath
in
Filename.concat config.project_root
(get_output_file_relative ~config relative_path)

let get_module_name cmt =
cmt |> handle_namespace |> Filename.basename |> ModuleName.from_string_unsafe

let get_cmt_file cmt =
let path_cmt = Filename.concat (Sys.getcwd ()) cmt in
let path_cmt =
if Filename.is_relative cmt then Filename.concat (Sys.getcwd ()) cmt
else cmt
in
let cmt_file =
if Filename.check_suffix path_cmt ".cmt" then
let path_cmt_lower_case =
Expand Down