Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
106 changes: 85 additions & 21 deletions lib/resolver/resolver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,25 @@ let module_name_to_filename name =
(** Stdlib module names — used only to SUPPRESS "module not found" errors
for imports that resolve from the bundled stdlib. A user file with the
same conventional filename always wins (it is found first), so listing
a name here never shadows user code. *)
a name here never shadows user code.

Every entry must name a module that [stdlib/] actually declares. Nine
Depot* names (Depot, Depot.Gate, DepotForm, DepotGate, DepotSchema,
DepotRepo, DepotQuery, DepotMigration, DepotTest) were listed but ship
in the third-party `depot` package, not here, so they only suppressed
genuine diagnostics for depot's own modules. "URI" was likewise never
a module — [stdlib/uri.march] declares `mod Uri`, and the mismatched
casing meant a plain `import Uri` reported "not found". *)
let stdlib_module_names =
[ "List"; "Map"; "Set"; "Array"; "Queue"; "String"; "Option"; "Result"
; "Math"; "Enum"; "BigInt"; "Decimal"; "DateTime"; "Duration"; "Bytes"; "Json"
; "Regex"; "Csv"; "File"; "Dir"; "Path"; "Http"; "HttpClient"
; "HttpServer"; "HttpTransport"; "WebSocket"; "Process"; "Logger"
; "Flow"; "Actor"; "Sort"; "Hamt"; "Seq"; "Iterable"; "IOList"
; "Random"; "Stats"; "Plot"; "Prelude"; "DataFrame"; "Test"
; "Vault"; "URI"
; "Depot"; "Depot.Gate"
; "Vault"; "Uri"
; "Config"; "Crypto"; "Env"; "Channel"; "PubSub"
; "ChannelServer"; "ChannelSocket"; "Presence"; "Tls"; "Uuid"
; "DepotForm"; "DepotGate"; "DepotSchema"; "DepotRepo"
; "DepotQuery"; "DepotMigration"; "DepotTest"
; "Tuple"; "Char"; "OrderedMap"; "SortedSet"; "Range" ]

(** All non-empty dotted prefixes of [names], LONGEST first, down to the
Expand Down Expand Up @@ -106,7 +111,9 @@ let rec has_global_effect_decl decls =
List.exists (function
| March_ast.Ast.DImpl _ | March_ast.Ast.DInterface _
| March_ast.Ast.DDeriving _ | March_ast.Ast.DTransitions _
| March_ast.Ast.DProtocol _ | March_ast.Ast.DExtern _ -> true
| March_ast.Ast.DProtocol _ | March_ast.Ast.DExtern _
| March_ast.Ast.DDescribe _ | March_ast.Ast.DTest _
| March_ast.Ast.DSetup _ | March_ast.Ast.DSetupAll _ -> true
| March_ast.Ast.DMod (_, _, inner, _) -> has_global_effect_decl inner
| _ -> false) decls

Expand Down Expand Up @@ -283,12 +290,56 @@ let resolve_imports ?(extra_lib_paths = []) ?(auto_discover = true)
Hashtbl.add loaded_paths canonical_source ();
note_user_file source_file canonical_source;

(* Does [path] declare `mod [mod_name]` at the start of a line? A cheap
textual check — enough to locate the file, which is then parsed
properly by [load]. *)
let file_declares path mod_name =
match (try Some (read_file path) with Sys_error _ -> None) with
| None -> false
| Some src ->
let needle = "mod " ^ mod_name in
let nlen = String.length needle in
let slen = String.length src in
let rec scan i =
if i + nlen > slen then false
else if (i = 0 || src.[i - 1] = '\n')
&& String.sub src i nlen = needle
&& (i + nlen = slen
|| (match src.[i + nlen] with
| ' ' | '\t' | '\n' | '\r' -> true
| _ -> false))
then true
else scan (i + 1)
in
scan 0
in

(* The conventional dotted->path mapping first ("MyApp.Router" ->
"my_app/router.march"), then a scan for the file that actually DECLARES
the module. The scan matters because a module's file need not follow
the convention: step 2's auto-discovery keys off the `mod` declaration,
not the path, so such a module was always reachable — just not by
explicit `import`. depot declares `mod Depot.Query` in
lib/api/depot_query.march, and until [import_refs] began yielding
dotted candidates that import resolved only because "Depot" sat
(wrongly) in [stdlib_module_names] and suppressed the lookup entirely.
Scanning only on a miss keeps the common path a single [Sys.file_exists]
and preserves the precise "looked for <path>" diagnostic for a module
that genuinely is not there. *)
let find_file mod_name =
let fname = module_name_to_filename mod_name in
List.find_map (fun dir ->
let p = Filename.concat dir fname in
if Sys.file_exists p then Some p else None
) search_path
match
List.find_map (fun dir ->
let p = Filename.concat dir fname in
if Sys.file_exists p then Some p else None
) search_path
with
| Some p -> Some p
| None ->
List.find_map (fun dir ->
List.find_opt (fun p -> file_declares p mod_name)
(collect_lib_files dir)
) search_path
in

let rec load mod_name ~from_span =
Expand Down Expand Up @@ -420,7 +471,7 @@ let resolve_imports ?(extra_lib_paths = []) ?(auto_discover = true)
(* Phase 1: parse + desugar every un-loaded discovered file across ALL
lib dirs, keeping its source text for reachability analysis. *)
let all_parsed =
List.concat_map (fun lib_dir ->
List.concat_map (fun (dir_idx, lib_dir) ->
let files = collect_lib_files lib_dir in
List.filter_map (fun file_path ->
let canon_fp =
Expand All @@ -434,11 +485,11 @@ let resolve_imports ?(extra_lib_paths = []) ?(auto_discover = true)
| Error msg ->
Printf.eprintf "[lib] %s\n%!" msg; None
| Ok ast ->
Some (canon_fp, file_path,
Some (dir_idx, canon_fp, file_path,
March_desugar.Desugar.desugar_module ~is_entry:false ast,
src)
) files
) all_lib_paths
) (List.mapi (fun i d -> (i, d)) all_lib_paths)
in
(* Reachability pruning. An auto-discovered library module is only
built when the entry can actually reach it — directly or transitively,
Expand All @@ -465,7 +516,7 @@ let resolve_imports ?(extra_lib_paths = []) ?(auto_discover = true)
anchors that keep it (module name + provided type/ctor names), and
the module-name tokens its own source references (transitive spread). *)
let discovered =
List.map (fun (_, _, ast, src) ->
List.map (fun (_, _, _, ast, src) ->
let mn = mod_name_of ast in
let anchors = mn :: provided_anchor_names ast.March_ast.Ast.mod_decls in
(mn, anchors, referenced_name_tokens src))
Expand All @@ -491,20 +542,33 @@ let resolve_imports ?(extra_lib_paths = []) ?(auto_discover = true)
|| Hashtbl.mem reachable (mod_name_of ast)
|| has_global_effect_decl ast.March_ast.Ast.mod_decls
in
(* Sort: more dot-segments in mod name → load first (namespace leaves).
Alphabetical tiebreak keeps things deterministic. *)
let sorted = List.sort (fun (_, _, a, _) (_, _, b, _) ->
let da = dot_count (mod_name_of a) and db = dot_count (mod_name_of b) in
if db <> da then compare db da
else compare (mod_name_of a) (mod_name_of b)
(* Sort: SEARCH-PATH ORDER first, then more dot-segments in mod name
(namespace leaves), then alphabetically for determinism.

The search-path key is what keeps the caller's ordering intent
intact. `forge test` places lib/ before test/ on MARCH_LIB_PATH
precisely because test modules call into lib modules and never the
reverse: a lib function still bound to its pass-1 Mono placeholder,
pinned first by a TEST call site, makes every later call site with
a different record shape fail to unify. Sorting on the module name
alone silently undid that — `Forgepm.Test.*` and `Forgepm.Web.*`
have equal dot-counts, so "Test" sorted ahead of "Web" and the test
modules were checked first, yielding a wall of `expected () but got
Unit`. Within one directory the dot-count rule still applies. *)
let sorted = List.sort (fun (ia, _, _, a, _) (ib, _, _, b, _) ->
if ia <> ib then compare ia ib
else
let da = dot_count (mod_name_of a) and db = dot_count (mod_name_of b) in
if db <> da then compare db da
else compare (mod_name_of a) (mod_name_of b)
) all_parsed in
(* Phase 2: build DMods in sorted order for KEPT modules only. Emit
transitive imports as top-level siblings (not nested) to avoid
name-mangling collisions. [canon_fp] keys the dedup; [orig_path] is
the string the file was parsed under (= what its spans carry),
recorded for user_files. A pruned module is skipped entirely — never
noted as a user file, never emitted, never typechecked. *)
List.concat_map (fun (canon_fp, orig_path, ast, _src) ->
List.concat_map (fun (_dir_idx, canon_fp, orig_path, ast, _src) ->
if Hashtbl.mem loaded_paths canon_fp then []
else if not (keep ast) then []
else begin
Expand Down
92 changes: 92 additions & 0 deletions test/test_compiler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5409,6 +5409,92 @@ end
"unrelated-named sibling dotted module still resolves (regression control)"
false errored

(* A user module whose dotted name's LEADING SEGMENT collides with a stdlib
module ("Depot" is in [stdlib_module_names], as is "Env", "Ast", …) must
still resolve from the search path. Before dotted candidates existed
[import_refs] yielded only the first segment, so `use Depot.Query` was
skipped as stdlib and step 2's auto-discovery picked the user file up;
once candidates were filtered against the stdlib list, the bare "Depot"
was dropped and the surviving "Depot.Query" — a real user module, but not
at `depot/query.march` — became a hard "Module not found". That broke
depot itself, whose own Depot.Query / Depot.Url / Depot.Transaction /
Depot.Migration live at lib/api/depot_*.march. *)
(* `forge test` compiles ONE test file as the entry and lets auto-discovery
pull in the siblings. Reachability pruning (48cf9263) keeps a discovered
module only when the entry references it or it carries a global-effect
decl — and nothing references a test module by name, which is what a test
module IS. Every sibling was therefore dropped, [collect_tests] never saw
its bodies, and the run reported a PASS over whatever survived: forgepm
went from 544 tests to 21, exit 0. Silent test loss is the worst
available failure mode, so pin it. *)
let test_sibling_test_module_is_not_pruned () =
let sibling_src = {|mod T.Sibling do
import Test

describe "sibling" do
test "runs" do
Test.assert_true(true, "holds")
end
end
end
|} in
let entry_src = {|mod T.Entry do
import Test

describe "entry" do
test "runs" do
Test.assert_true(true, "holds")
end
end
end
|} in
let dir = Filename.temp_file "march_test_prune_" "" in
Sys.remove dir;
Unix.mkdir dir 0o755;
Fun.protect ~finally:(fun () ->
try ignore (Sys.command (Printf.sprintf "rm -rf %s" (Filename.quote dir)))
with _ -> ())
(fun () ->
let oc = open_out (Filename.concat dir "sibling_test.march") in
output_string oc sibling_src; close_out oc;
(* The entry must be a REAL file: [resolve_imports] disables pruning
entirely when it cannot read [source_file], so a synthetic path
would make this test vacuous (it passes either way). *)
let entry_path = Filename.concat dir "entry_test.march" in
let oc = open_out entry_path in
output_string oc entry_src; close_out oc;
Unix.putenv "MARCH_LIB_PATH" dir;
Fun.protect ~finally:(fun () -> Unix.putenv "MARCH_LIB_PATH" "") (fun () ->
let m = parse_and_desugar entry_src in
let (_errs, extra_decls, _files) =
March_resolver.Resolver.resolve_imports ~source_file:entry_path m in
let mentions_sibling =
List.exists (function
| March_ast.Ast.DMod (n, _, _, _) -> n.March_ast.Ast.txt = "T.Sibling"
| _ -> false) extra_decls
in
Alcotest.(check bool)
"an unreferenced sibling test module survives reachability pruning"
true mentions_sibling))

let test_stdlib_prefixed_user_module_resolves () =
let sibling_src = {|mod Depot.Query do
fn answer() : Int do 42 end
end
|} in
let entry_src = {|mod App do
import Depot.Query

fn main() : Int do
Depot.Query.answer()
end
end
|} in
let errored = check_entry_with_lib_dir [("depot_query.march", sibling_src)] entry_src in
Alcotest.(check bool)
"user module under a stdlib-named prefix resolves from the search path"
false errored

let test_self_prefix_nested_submodule_still_strips () =
(* Control for the [strip_entry_self_qual] fix itself: a GENUINE nested
submodule (declared directly inside the entry, not a separate sibling
Expand Down Expand Up @@ -9894,6 +9980,12 @@ let compiler_suites =
Alcotest.test_case
"genuine nested submodule self-qualification still strips"
`Quick test_self_prefix_nested_submodule_still_strips;
Alcotest.test_case
"user module under a stdlib-named dotted prefix resolves"
`Quick test_stdlib_prefixed_user_module_resolves;
Alcotest.test_case
"unreferenced sibling test module is not pruned"
`Quick test_sibling_test_module_is_not_pruned;
] );
( "diagnostic dedup",
[
Expand Down
Loading