Skip to content

Commit c02cb7f

Browse files
benjamin-thomasbenjamin-thomas
committed
analysis: fix namespaced reference lookup
Signed-off-by: Benjamin Thomas <benjamin.guy.thomas@gmail.com>
1 parent 32f0ddb commit c02cb7f

15 files changed

Lines changed: 179 additions & 27 deletions

File tree

‎analysis/src/Cmt.ml‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,30 @@ let fullForCmt ~moduleName ~package ~uri cmt =
88
let extra = ProcessExtra.getExtra ~file ~infos in
99
Some {file; extra; package}
1010

11+
let fullForIncrementalCmt ~package ~moduleName ~uri =
12+
if !Cfg.inIncrementalTypecheckingMode then
13+
let path = Uri.toPath uri in
14+
let incrementalCmtPath =
15+
package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName
16+
^
17+
match Files.classifySourceFile path with
18+
| Resi -> ".cmti"
19+
| _ -> ".cmt"
20+
in
21+
match fullForCmt ~moduleName ~package ~uri incrementalCmtPath with
22+
| Some cmtInfo ->
23+
if Debug.verbose () then Printf.printf "[cmt] Found incremental cmt\n";
24+
Some cmtInfo
25+
| None -> None
26+
else None
27+
28+
let fullFromModuleUri ~package ~moduleName ~uri ~paths =
29+
match fullForIncrementalCmt ~package ~moduleName ~uri with
30+
| Some cmtInfo -> Some cmtInfo
31+
| None ->
32+
let cmt = getCmtPath ~uri paths in
33+
fullForCmt ~moduleName ~package ~uri cmt
34+
1135
let fullFromUri ~uri =
1236
let path = Uri.toPath uri in
1337
match Packages.getPackage ~uri with
@@ -16,22 +40,8 @@ let fullFromUri ~uri =
1640
let moduleName =
1741
BuildSystem.namespacedName package.namespace (FindFiles.getName path)
1842
in
19-
let incremental =
20-
if !Cfg.inIncrementalTypecheckingMode then
21-
let incrementalCmtPath =
22-
package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName
23-
^
24-
match Files.classifySourceFile path with
25-
| Resi -> ".cmti"
26-
| _ -> ".cmt"
27-
in
28-
fullForCmt ~moduleName ~package ~uri incrementalCmtPath
29-
else None
30-
in
31-
match incremental with
32-
| Some cmtInfo ->
33-
if Debug.verbose () then Printf.printf "[cmt] Found incremental cmt\n";
34-
Some cmtInfo
43+
match fullForIncrementalCmt ~package ~moduleName ~uri with
44+
| Some cmtInfo -> Some cmtInfo
3545
| None -> (
3646
match Hashtbl.find_opt package.pathsForModule moduleName with
3747
| Some paths ->
@@ -41,12 +51,20 @@ let fullFromUri ~uri =
4151
prerr_endline ("can't find module " ^ moduleName);
4252
None))
4353

54+
let fullFromModule ~package ~moduleName =
55+
Option.bind (Hashtbl.find_opt package.pathsForModule moduleName)
56+
@@ fun paths ->
57+
let uri = getUri paths in
58+
fullFromModuleUri ~package ~moduleName ~uri ~paths
59+
4460
let fullsFromModule ~package ~moduleName =
45-
if Hashtbl.mem package.pathsForModule moduleName then
46-
let paths = Hashtbl.find package.pathsForModule moduleName in
61+
match Hashtbl.find_opt package.pathsForModule moduleName with
62+
| None -> []
63+
| Some paths ->
4764
let uris = getUris paths in
48-
uris |> List.filter_map (fun uri -> fullFromUri ~uri)
49-
else []
65+
uris
66+
|> List.filter_map (fun uri ->
67+
fullFromModuleUri ~package ~moduleName ~uri ~paths)
5068

5169
let loadFullCmtFromPath ~path =
5270
let uri = Uri.fromPath path in

‎analysis/src/References.ml‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@ let locItemsForPos ~extra pos =
1818

1919
let lineColToCmtLoc ~pos:(line, col) = (line + 1, col)
2020

21+
(** External references in namespaced projects are indexed by the public
22+
* namespace path, e.g. MyNamespace.MyModule1.myFunc1, while definitions live
23+
* in hidden compiled modules like MyModule1-MyNamespace.
24+
* We return the lookup key pair used by the external reference index.
25+
*)
26+
let normalizeExternalReferenceKey ~namespace ~moduleName ~path =
27+
match namespace with
28+
| Some namespace when Utils.endsWith moduleName ("-" ^ namespace) ->
29+
let suffixLen = String.length namespace + 1 in
30+
let sourceModuleLen = String.length moduleName - suffixLen in
31+
let sourceModule = String.sub moduleName 0 sourceModuleLen in
32+
(namespace, sourceModule :: path)
33+
| _ -> (moduleName, path)
34+
2135
let getLocItem ~full ~pos ~debug =
2236
let log n msg = if debug then Printf.printf "getLocItem #%d: %s\n" n msg in
2337
let pos = lineColToCmtLoc ~pos in
@@ -485,6 +499,10 @@ let forLocalStamp ~full:{file; extra; package} stamp (tip : Tip.t) =
485499
in
486500
maybeLog ("Now checking path " ^ pathToString path);
487501
let thisModuleName = file.moduleName in
502+
let normalizedModuleName, normalizedPath =
503+
normalizeExternalReferenceKey ~namespace:package.namespace
504+
~moduleName:thisModuleName ~path
505+
in
488506
let externals =
489507
package.projectFiles |> FileSet.elements
490508
|> List.filter (fun name -> name <> file.moduleName)
@@ -493,14 +511,15 @@ let forLocalStamp ~full:{file; extra; package} stamp (tip : Tip.t) =
493511
|> List.map (fun {file; extra} ->
494512
match
495513
Hashtbl.find_opt extra.externalReferences
496-
thisModuleName
514+
normalizedModuleName
497515
with
498516
| None -> []
499517
| Some refs ->
500518
let locs =
501519
refs
502520
|> Utils.filterMap (fun (p, t, locs) ->
503-
if p = path && t = tip then Some locs
521+
if p = normalizedPath && t = tip then
522+
Some locs
504523
else None)
505524
in
506525
locs
@@ -522,10 +541,8 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem =
522541
| TopLevelModule moduleName ->
523542
let otherModulesReferences =
524543
package.projectFiles |> FileSet.elements
525-
|> Utils.filterMap (fun name ->
526-
match ProcessCmt.fileForModule ~package name with
527-
| None -> None
528-
| Some file -> Cmt.fullFromUri ~uri:file.uri)
544+
|> Utils.filterMap (fun moduleName ->
545+
Cmt.fullFromModule ~package ~moduleName)
529546
|> List.map (fun full ->
530547
match Hashtbl.find_opt full.extra.fileReferences moduleName with
531548
| None -> []
@@ -563,7 +580,7 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem =
563580
match exportedForTip ~env ~path ~package ~tip with
564581
| None -> []
565582
| Some (env, _name, stamp) -> (
566-
match Cmt.fullFromUri ~uri:env.file.uri with
583+
match Cmt.fullFromModule ~package ~moduleName:env.file.moduleName with
567584
| None -> []
568585
| Some full ->
569586
maybeLog

‎tests/analysis_tests/Makefile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ test-analysis-binary:
44
make -C tests test
55
make -C tests-generic-jsx-transform test
66
make -C tests-incremental-typechecking test
7+
make -C tests-namespaced-references test
78
make -C tests-sourcedirs-dependency test
89

910
test-reanalyze:
@@ -15,6 +16,7 @@ clean:
1516
make -C tests clean
1617
make -C tests-generic-jsx-transform clean
1718
make -C tests-incremental-typechecking clean
19+
make -C tests-namespaced-references clean
1820
make -C tests-sourcedirs-dependency clean
1921
make -C tests-reanalyze clean
2022

‎tests/analysis_tests/tests-namespaced-references/.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lib/
2+
node_modules/

‎tests/analysis_tests/tests-namespaced-references/Makefile‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
SHELL = /bin/bash
2+
3+
build:
4+
yarn build
5+
6+
test: build
7+
./test.sh
8+
9+
clean:
10+
yarn clean
11+
12+
.DEFAULT_GOAL := test
13+
14+
.PHONY: clean test

‎tests/analysis_tests/tests-namespaced-references/package.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"name": "@tests/namespaced-references",
3+
"private": true,
4+
"scripts": {
5+
"build": "rescript build",
6+
"clean": "rescript clean"
7+
},
8+
"dependencies": {
9+
"rescript": "workspace:^"
10+
}
11+
}

‎tests/analysis_tests/tests-namespaced-references/rescript.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"name": "@tests/namespaced-references",
3+
"namespace": "my-namespace",
4+
"sources": [
5+
{
6+
"dir": "src",
7+
"subdirs": true
8+
}
9+
],
10+
"package-specs": [
11+
{
12+
"module": "commonjs",
13+
"in-source": false
14+
}
15+
],
16+
"suffix": ".res.js"
17+
}

‎tests/analysis_tests/tests-namespaced-references/src/MyModule1.res‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
let myFunc1 = () => MyModule2.myFunc2()
2+
// ^ref

‎tests/analysis_tests/tests-namespaced-references/src/MyModule2.res‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
let myFunc2 = () => 42
2+
// ^ref

‎tests/analysis_tests/tests-namespaced-references/src/expected/MyModule1.res.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
References src/MyModule1.res 0:30
2+
[
3+
{"uri": "MyModule1.res", "range": {"start": {"line": 0, "character": 30}, "end": {"line": 0, "character": 37}}},
4+
{"uri": "MyModule2.res", "range": {"start": {"line": 0, "character": 4}, "end": {"line": 0, "character": 11}}}
5+
]
6+

0 commit comments

Comments
 (0)