diff --git a/analysis/src/Cmt.ml b/analysis/src/Cmt.ml index ac1d5ae595..58b49ec364 100644 --- a/analysis/src/Cmt.ml +++ b/analysis/src/Cmt.ml @@ -8,6 +8,30 @@ let fullForCmt ~moduleName ~package ~uri cmt = let extra = ProcessExtra.getExtra ~file ~infos in Some {file; extra; package} +let fullForIncrementalCmt ~package ~moduleName ~uri = + if !Cfg.inIncrementalTypecheckingMode then + let path = Uri.toPath uri in + let incrementalCmtPath = + package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName + ^ + match Files.classifySourceFile path with + | Resi -> ".cmti" + | _ -> ".cmt" + in + match fullForCmt ~moduleName ~package ~uri incrementalCmtPath with + | Some cmtInfo -> + if Debug.verbose () then Printf.printf "[cmt] Found incremental cmt\n"; + Some cmtInfo + | None -> None + else None + +let fullFromModuleUri ~package ~moduleName ~uri ~paths = + match fullForIncrementalCmt ~package ~moduleName ~uri with + | Some cmtInfo -> Some cmtInfo + | None -> + let cmt = getCmtPath ~uri paths in + fullForCmt ~moduleName ~package ~uri cmt + let fullFromUri ~uri = let path = Uri.toPath uri in match Packages.getPackage ~uri with @@ -16,22 +40,8 @@ let fullFromUri ~uri = let moduleName = BuildSystem.namespacedName package.namespace (FindFiles.getName path) in - let incremental = - if !Cfg.inIncrementalTypecheckingMode then - let incrementalCmtPath = - package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName - ^ - match Files.classifySourceFile path with - | Resi -> ".cmti" - | _ -> ".cmt" - in - fullForCmt ~moduleName ~package ~uri incrementalCmtPath - else None - in - match incremental with - | Some cmtInfo -> - if Debug.verbose () then Printf.printf "[cmt] Found incremental cmt\n"; - Some cmtInfo + match fullForIncrementalCmt ~package ~moduleName ~uri with + | Some cmtInfo -> Some cmtInfo | None -> ( match Hashtbl.find_opt package.pathsForModule moduleName with | Some paths -> @@ -41,12 +51,20 @@ let fullFromUri ~uri = prerr_endline ("can't find module " ^ moduleName); None)) +let fullFromModule ~package ~moduleName = + Option.bind (Hashtbl.find_opt package.pathsForModule moduleName) + @@ fun paths -> + let uri = getUri paths in + fullFromModuleUri ~package ~moduleName ~uri ~paths + let fullsFromModule ~package ~moduleName = - if Hashtbl.mem package.pathsForModule moduleName then - let paths = Hashtbl.find package.pathsForModule moduleName in + match Hashtbl.find_opt package.pathsForModule moduleName with + | None -> [] + | Some paths -> let uris = getUris paths in - uris |> List.filter_map (fun uri -> fullFromUri ~uri) - else [] + uris + |> List.filter_map (fun uri -> + fullFromModuleUri ~package ~moduleName ~uri ~paths) let loadFullCmtFromPath ~path = let uri = Uri.fromPath path in diff --git a/analysis/src/References.ml b/analysis/src/References.ml index e047a2ba18..142cbfa2f8 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -18,6 +18,20 @@ let locItemsForPos ~extra pos = let lineColToCmtLoc ~pos:(line, col) = (line + 1, col) +(** External references in namespaced projects are indexed by the public + * namespace path, e.g. MyNamespace.MyModule1.myFunc1, while definitions live + * in hidden compiled modules like MyModule1-MyNamespace. + * We return the lookup key pair used by the external reference index. + *) +let normalizeExternalReferenceKey ~namespace ~moduleName ~path = + match namespace with + | Some namespace when Utils.endsWith moduleName ("-" ^ namespace) -> + let suffixLen = String.length namespace + 1 in + let sourceModuleLen = String.length moduleName - suffixLen in + let sourceModule = String.sub moduleName 0 sourceModuleLen in + (namespace, sourceModule :: path) + | _ -> (moduleName, path) + let getLocItem ~full ~pos ~debug = let log n msg = if debug then Printf.printf "getLocItem #%d: %s\n" n msg in let pos = lineColToCmtLoc ~pos in @@ -485,6 +499,10 @@ let forLocalStamp ~full:{file; extra; package} stamp (tip : Tip.t) = in maybeLog ("Now checking path " ^ pathToString path); let thisModuleName = file.moduleName in + let normalizedModuleName, normalizedPath = + normalizeExternalReferenceKey ~namespace:package.namespace + ~moduleName:thisModuleName ~path + in let externals = package.projectFiles |> FileSet.elements |> List.filter (fun name -> name <> file.moduleName) @@ -493,14 +511,15 @@ let forLocalStamp ~full:{file; extra; package} stamp (tip : Tip.t) = |> List.map (fun {file; extra} -> match Hashtbl.find_opt extra.externalReferences - thisModuleName + normalizedModuleName with | None -> [] | Some refs -> let locs = refs |> Utils.filterMap (fun (p, t, locs) -> - if p = path && t = tip then Some locs + if p = normalizedPath && t = tip then + Some locs else None) in locs @@ -522,10 +541,8 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = | TopLevelModule moduleName -> let otherModulesReferences = package.projectFiles |> FileSet.elements - |> Utils.filterMap (fun name -> - match ProcessCmt.fileForModule ~package name with - | None -> None - | Some file -> Cmt.fullFromUri ~uri:file.uri) + |> Utils.filterMap (fun moduleName -> + Cmt.fullFromModule ~package ~moduleName) |> List.map (fun full -> match Hashtbl.find_opt full.extra.fileReferences moduleName with | None -> [] @@ -563,7 +580,7 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = match exportedForTip ~env ~path ~package ~tip with | None -> [] | Some (env, _name, stamp) -> ( - match Cmt.fullFromUri ~uri:env.file.uri with + match Cmt.fullFromModule ~package ~moduleName:env.file.moduleName with | None -> [] | Some full -> maybeLog diff --git a/tests/analysis_tests/Makefile b/tests/analysis_tests/Makefile index 0fd0226e6a..741c90403e 100644 --- a/tests/analysis_tests/Makefile +++ b/tests/analysis_tests/Makefile @@ -4,6 +4,7 @@ test-analysis-binary: make -C tests test make -C tests-generic-jsx-transform test make -C tests-incremental-typechecking test + make -C tests-namespaced-references test make -C tests-sourcedirs-dependency test test-reanalyze: @@ -15,6 +16,7 @@ clean: make -C tests clean make -C tests-generic-jsx-transform clean make -C tests-incremental-typechecking clean + make -C tests-namespaced-references clean make -C tests-sourcedirs-dependency clean make -C tests-reanalyze clean diff --git a/tests/analysis_tests/tests-namespaced-references/.gitignore b/tests/analysis_tests/tests-namespaced-references/.gitignore new file mode 100644 index 0000000000..3b4e8dcf93 --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/.gitignore @@ -0,0 +1,2 @@ +lib/ +node_modules/ diff --git a/tests/analysis_tests/tests-namespaced-references/Makefile b/tests/analysis_tests/tests-namespaced-references/Makefile new file mode 100644 index 0000000000..4d26df85bb --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/Makefile @@ -0,0 +1,14 @@ +SHELL = /bin/bash + +build: + yarn build + +test: build + ./test.sh + +clean: + yarn clean + +.DEFAULT_GOAL := test + +.PHONY: clean test diff --git a/tests/analysis_tests/tests-namespaced-references/package.json b/tests/analysis_tests/tests-namespaced-references/package.json new file mode 100644 index 0000000000..596b324a42 --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/package.json @@ -0,0 +1,11 @@ +{ + "name": "@tests/namespaced-references", + "private": true, + "scripts": { + "build": "rescript build", + "clean": "rescript clean" + }, + "dependencies": { + "rescript": "workspace:^" + } +} diff --git a/tests/analysis_tests/tests-namespaced-references/rescript.json b/tests/analysis_tests/tests-namespaced-references/rescript.json new file mode 100644 index 0000000000..283f693473 --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/rescript.json @@ -0,0 +1,17 @@ +{ + "name": "@tests/namespaced-references", + "namespace": "my-namespace", + "sources": [ + { + "dir": "src", + "subdirs": true + } + ], + "package-specs": [ + { + "module": "commonjs", + "in-source": false + } + ], + "suffix": ".res.js" +} diff --git a/tests/analysis_tests/tests-namespaced-references/src/MyModule1.res b/tests/analysis_tests/tests-namespaced-references/src/MyModule1.res new file mode 100644 index 0000000000..88e16f534e --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/src/MyModule1.res @@ -0,0 +1,2 @@ +let myFunc1 = () => MyModule2.myFunc2() +// ^ref diff --git a/tests/analysis_tests/tests-namespaced-references/src/MyModule2.res b/tests/analysis_tests/tests-namespaced-references/src/MyModule2.res new file mode 100644 index 0000000000..4e7d3930e1 --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/src/MyModule2.res @@ -0,0 +1,2 @@ +let myFunc2 = () => 42 +// ^ref diff --git a/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule1.res.txt b/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule1.res.txt new file mode 100644 index 0000000000..71273ca46b --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule1.res.txt @@ -0,0 +1,18 @@ +References src/MyModule1.res 0:30 +[ + { + "range": { + "end": { "character": 37, "line": 0 }, + "start": { "character": 30, "line": 0 } + }, + "uri": "file:///MyModule1.res" + }, + { + "range": { + "end": { "character": 11, "line": 0 }, + "start": { "character": 4, "line": 0 } + }, + "uri": "file:///MyModule2.res" + } +] + diff --git a/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule2.res.txt b/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule2.res.txt new file mode 100644 index 0000000000..4cb76f012e --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/src/expected/MyModule2.res.txt @@ -0,0 +1,18 @@ +References src/MyModule2.res 0:4 +[ + { + "range": { + "end": { "character": 37, "line": 0 }, + "start": { "character": 30, "line": 0 } + }, + "uri": "file:///MyModule1.res" + }, + { + "range": { + "end": { "character": 11, "line": 0 }, + "start": { "character": 4, "line": 0 } + }, + "uri": "file:///MyModule2.res" + } +] + diff --git a/tests/analysis_tests/tests-namespaced-references/test.sh b/tests/analysis_tests/tests-namespaced-references/test.sh new file mode 100755 index 0000000000..c58c8e70b9 --- /dev/null +++ b/tests/analysis_tests/tests-namespaced-references/test.sh @@ -0,0 +1,21 @@ +for file in src/*.res; do + output="$(dirname $file)/expected/$(basename $file).txt" + ../../../_build/install/default/bin/rescript-editor-analysis test $file &> $output + # CI. We use LF, and the CI OCaml fork prints CRLF. Convert. + if [ "$RUNNER_OS" == "Windows" ]; then + perl -pi -e 's/\r\n/\n/g' -- $output + fi +done + +warningYellow='\033[0;33m' +successGreen='\033[0;32m' +reset='\033[0m' + +diff=$(git ls-files --modified src/expected) +if [[ $diff = "" ]]; then + printf "${successGreen}✅ No unstaged tests difference.${reset}\n" +else + printf "${warningYellow}⚠️ There are unstaged differences in tests/! Did you break a test?\n${diff}\n${reset}" + git --no-pager diff src/expected + exit 1 +fi diff --git a/tests/ounit_tests/ounit_analysis_references_tests.ml b/tests/ounit_tests/ounit_analysis_references_tests.ml new file mode 100644 index 0000000000..a81e69764e --- /dev/null +++ b/tests/ounit_tests/ounit_analysis_references_tests.ml @@ -0,0 +1,25 @@ +open OUnit + +let show_reference (module_name, path) = + Printf.sprintf "%s:%s" module_name (String.concat "." path) + +let assert_ref_key_eq ~expected ~actual = + assert_equal ~printer:show_reference expected actual + +let suites = + __FILE__ + >::: [ + ( "without namespace, external reference is unchanged" >:: fun _ -> + assert_ref_key_eq ~expected:("MyModule2", ["myFunc2"]) + ~actual: + (Analysis.References.normalizeExternalReferenceKey + ~namespace:None ~moduleName:"MyModule2" ~path:["myFunc2"]) ); + ( "with namespace, hidden module resolves to public namespace path" + >:: fun _ -> + assert_ref_key_eq + ~expected:("MyNamespace", ["MyModule2"; "myFunc2"]) + ~actual: + (Analysis.References.normalizeExternalReferenceKey + ~namespace:(Some "MyNamespace") + ~moduleName:"MyModule2-MyNamespace" ~path:["myFunc2"]) ); + ] diff --git a/tests/ounit_tests/ounit_tests_main.ml b/tests/ounit_tests/ounit_tests_main.ml index 05d374732b..7147844c6e 100644 --- a/tests/ounit_tests/ounit_tests_main.ml +++ b/tests/ounit_tests/ounit_tests_main.ml @@ -20,6 +20,7 @@ let suites = Ounit_js_analyzer_tests.suites; Ounit_jsx_loc_tests.suites; Ounit_analysis_config_tests.suites; + Ounit_analysis_references_tests.suites; ] let _ = OUnit.run_test_tt_main suites diff --git a/yarn.lock b/yarn.lock index a92a69c167..1eeb1b12d8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1133,6 +1133,14 @@ __metadata: languageName: unknown linkType: soft +"@tests/namespaced-references@workspace:tests/analysis_tests/tests-namespaced-references": + version: 0.0.0-use.local + resolution: "@tests/namespaced-references@workspace:tests/analysis_tests/tests-namespaced-references" + dependencies: + rescript: "workspace:^" + languageName: unknown + linkType: soft + "@tests/reanalyze-benchmark@workspace:tests/analysis_tests/tests-reanalyze/deadcode-benchmark": version: 0.0.0-use.local resolution: "@tests/reanalyze-benchmark@workspace:tests/analysis_tests/tests-reanalyze/deadcode-benchmark"