Skip to content

Commit

Permalink
[flow][get-def] Use the same heuristic for get-def in require expression
Browse files Browse the repository at this point in the history
Summary:
This diff applies the same heuristics in the previous diff to require expression. The def loc is recorded in the reason of the literal expression.

Changelog: [ide] Go-to-definition for `require('...')` expression will now jump to the default export of an ESM module if available, or the first export of an ESM module.

Reviewed By: panagosg7

Differential Revision: D67875257

fbshipit-source-id: 3d1a47555d448c3872063ee6d2c349e3c64ebd17
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Jan 7, 2025
1 parent 8237ea2 commit 5f0fb3c
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 35 deletions.
10 changes: 8 additions & 2 deletions src/services/get_def/get_def_process_location.ml
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,18 @@ class virtual ['T] searcher _cx ~is_local_use ~is_legit_require ~covers_target ~
Call.callee = (_, Identifier (_, { Identifier.name = "require"; _ }));
arguments =
( _,
{ ArgList.arguments = [Expression (source_annot, StringLiteral _)]; comments = _ }
{
ArgList.arguments =
[Expression (source_annot, (StringLiteral _ | TemplateLiteral _))];
comments = _;
}
);
_;
}
when this#is_legit_require source_annot ->
let annot = (this#loc_of_annot annot, this#type_from_enclosing_node annot) in
let annot =
(this#loc_of_annot source_annot, this#type_from_enclosing_node source_annot)
in
this#request (Get_def_request.Type { annot; name = None })
| _ -> super#expression (annot, expr)
else
Expand Down
32 changes: 15 additions & 17 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3655,21 +3655,7 @@ module Make
comments;
}
)
) ->
let (_def_loc_opt, t) =
Import_export.get_module_t
cx
(source_loc, module_name)
~perform_platform_validation:true
~import_kind_for_untyped_import_validation:(Some ImportValue)
|> Import_export.cjs_require_type
cx
(mk_reason (RModule module_name) loc)
~namespace_symbol:(mk_module_symbol ~name:module_name ~def_loc:loc)
~standard_cjs_esm_interop:false
~legacy_interop:false
in
(t, (args_loc, { ArgList.arguments = [Expression (expression cx lit_exp)]; comments }))
)
| ( None,
( args_loc,
{
Expand Down Expand Up @@ -3699,7 +3685,7 @@ module Make
}
)
) ->
let (_def_loc_opt, t) =
let (def_loc_opt, require_t) =
Import_export.get_module_t
cx
(source_loc, module_name)
Expand All @@ -3712,7 +3698,19 @@ module Make
~standard_cjs_esm_interop:false
~legacy_interop:false
in
(t, (args_loc, { ArgList.arguments = [Expression (expression cx lit_exp)]; comments }))
let lit_exp =
let ((l, t), e) = expression cx lit_exp in
let t =
TypeUtil.mod_reason_of_t
(fun _ ->
match def_loc_opt with
| Some def_loc -> mk_reason (RModule module_name) def_loc |> repos_reason loc
| None -> TypeUtil.reason_of_t require_t |> repos_reason loc)
t
in
((l, t), e)
in
(require_t, (args_loc, { ArgList.arguments = [Expression lit_exp]; comments }))
| (Some _, arguments) ->
ignore (arg_list cx arguments);
Flow.add_output
Expand Down
36 changes: 20 additions & 16 deletions tests/get_def/get_def.exp
Original file line number Diff line number Diff line change
Expand Up @@ -224,67 +224,71 @@ require.js:12:10
Flags:
[LIB] libs/lib.js:18:27,18:32

require.js:16:9
require.js:15:11
Flags:
[LIB] libs/lib.js:2:22,2:24

require.js:18:9
require.js:19:9
Flags:
[LIB] libs/lib.js:2:22,2:24

require.js:20:14
require.js:21:9
Flags:
[LIB] libs/lib.js:2:22,2:24

require.js:22:9
require.js:23:14
Flags:
[LIB] libs/lib.js:2:22,2:24

require.js:25:9
Flags:
[LIB] libs/lib.js:3:22,3:24

require.js:24:9
require.js:27:9
Flags:
[LIB] libs/lib.js:3:22,3:24

require.js:26:14
require.js:29:14
Flags:
[LIB] libs/lib.js:3:22,3:24

require.js:28:15
require.js:31:15
Flags:
[LIB] libs/lib.js:3:28,3:30

require.js:30:20
require.js:33:20
Flags:
[LIB] libs/lib.js:3:28,3:30

require.js:35:9
require.js:38:9
Flags:
[LIB] libs/lib.js:8:5,8:7

require.js:37:9
require.js:40:9
Flags:
[LIB] libs/lib.js:8:5,8:7

require.js:39:14
require.js:42:14
Flags:
[LIB] libs/lib.js:8:5,8:7

require.js:41:9
require.js:44:9
Flags:
[LIB] libs/lib.js:9:5,9:7

require.js:43:9
require.js:46:9
Flags:
[LIB] libs/lib.js:9:5,9:7

require.js:45:14
require.js:48:14
Flags:
[LIB] libs/lib.js:9:5,9:7

require.js:47:15
require.js:50:15
Flags:
[LIB] libs/lib.js:9:11,9:13

require.js:49:20
require.js:52:20
Flags:
[LIB] libs/lib.js:9:11,9:13

Expand Down
3 changes: 3 additions & 0 deletions tests/get_def/require.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ require("Untyped");
require("Untyped_LibDeclared");
// ^

require('test_lib');
// ^

{
const {foo} = require('test_lib');
// ^
Expand Down

0 comments on commit 5f0fb3c

Please sign in to comment.