analysis: fix namespaced reference lookup

[benjamin-thomas]

Re-opening here following a probably bad workflow from my part on #8453 :)

---

Hello!

This is an attempt at fixing this problem:

2 problems were occurring when activating the namespace feature

• The generated namespace module's path would be searched by its "double name" (MyNamespace-MyNamespace)
• The references command (from rescript-editor-analysis.exe references) would try to find the namespaced module (MyModule1-MyNamespace) against the indexed "public paths" and so would fail.

This PR solves the problem by:

• not trying to find the module name via file URI resolution when we already have its name in the package module index.
• mapping the "internal paths" (MyModule1-MyNamespace) to their public path equivalent (MyNamespace.MyModule1)

It looks like this is a long standing issue. I found this old open issue on the vscode plugin repo.

[fhammerschmidt]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🚀

ℹ️ 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".

[codecov]

Codecov Report

❌ Patch coverage is 39.02439% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.68%. Comparing base (f0636e0) to head (af8e951).

Files with missing lines	Patch %	Lines
analysis/src/Cmt.ml	12.50%	21 Missing ⚠️
analysis/src/References.ml	60.00%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #8455   +/-   ##
=======================================
  Coverage   60.68%   60.68%           
=======================================
  Files         373      374    +1     
  Lines       54095    54117   +22     
=======================================
+ Hits        32826    32840   +14     
- Misses      21269    21277    +8     

Files with missing lines	Coverage Δ
...sts/ounit_tests/ounit_analysis_references_tests.ml	100.00% <100.00%> (ø)
tests/ounit_tests/ounit_tests_main.ml	100.00% <ø> (ø)
analysis/src/References.ml	1.73% <60.00%> (+1.73%)	⬆️
analysis/src/Cmt.ml	30.61% <12.50%> (-5.29%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8455

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8455

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8455

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8455

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8455

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8455

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8455

commit: af8e951

[fhammerschmidt]

Looks already great!

The existing tests/analysis_tests/tests-namespaced-references fixture covers the normal CMT path. It would be useful to also cover the new fullFromModule / fullsFromModule path when incremental typechecking is enabled, since that now prefers lib/bs/___incremental/<module>.cmt|cmti.

You could extend tests/analysis_tests/tests-namespaced-references/Makefile after yarn build to create incremental CMT (typedtree) artifacts from the normal build output:

build:
    yarn build
    mkdir -p lib/bs/___incremental
    cp lib/bs/src/MyModule1-MyNamespace.cmt lib/bs/___incremental/MyModule1-MyNamespace.cmt
    cp lib/bs/src/MyModule2-MyNamespace.cmt lib/bs/___incremental/MyModule2-MyNamespace.cmt
    cp lib/bs/MyNamespace.cmt lib/bs/___incremental/MyNamespace.cmt

Then add an incremental marker around one of the existing reference checks, for example in src/MyModule1.res:

// ^in+
// ^dv+
let myFunc1 = () => MyModule2.myFunc2()
//                            ^ref
// ^dv-
// ^in-

^in+ enables incremental typechecking mode in the analysis test runner. ^dv+ should make the expected output include "[cmt] Found incremental cmt", which proves the test actually hit the incremental CMT branch rather than silently falling back to the regular lib/bs CMTs.

Then run:

make -C tests/analysis_tests/tests-namespaced-references test