Add opt-in --file-order-auto+ for dependency-based file ordering#19647
Open
bryancostanich wants to merge 34 commits intodotnet:mainfrom
Open
Add opt-in --file-order-auto+ for dependency-based file ordering#19647bryancostanich wants to merge 34 commits intodotnet:mainfrom
bryancostanich wants to merge 34 commits intodotnet:mainfrom
Conversation
Contributor
❗ Release notes required
|
bryancostanich
added a commit
to bryancostanich/fsharp
that referenced
this pull request
Apr 27, 2026
Author
|
@dotnet-policy-service agree |
Implement the foundation for order-independent compilation: SymbolCollection.fs: New "Enter" phase that scans all parsed files and collects top-level declarations (types, values, modules, opens, exceptions) into Entity shell stubs. These stubs are folded into TcEnv via the existing AddLocalRootModuleOrNamespace path, pre-populating the name resolution environment before type checking begins. Compiler flag: --file-order-auto+/- (default off) gates the pre-pass. When enabled, the enter phase runs between GetInitialTcState and CheckClosedInputSet in the fsc pipeline. Key design decisions: - Eager stubs via ModuleOrNamespaceType (not lazy completers) for Level A - Entity shells have names/arities but TNoRepr (no type representations) - Architecture supports future Level B lazy completers for cycle groups - Pre-pass is embarrassingly parallel (Array.Parallel.mapi) This is Track 01 of the order-independent compilation project. Track 02 (auto dependency graph) will use the FileDeclarations output to compute topological file ordering, completing the end-to-end feature. Includes end-to-end test project verifying no regressions with the flag. Builds clean: 0 errors, 0 warnings (validated in Docker/Linux).
…ck 02) When --file-order-auto+ is enabled, the compiler now automatically determines the correct file compilation order from dependency analysis: 1. The symbol collection pre-pass (Track 01) gathers all declarations 2. FileContentMapping walks the full AST for qualified name references 3. An export map links module/type names to their defining files 4. A dependency graph is built from each file's references 5. Kahn's algorithm produces a deterministic topological sort 6. Files are reordered before being passed to CheckClosedInputSet The .fsproj file order becomes irrelevant. Files can be listed in any order and the compiler will figure out the correct compilation sequence. Cycle detection via Tarjan-style in-degree tracking: if files form a circular dependency, a clear error is reported (Level A behavior). Level B (cycle group resolution) is architecturally supported but deferred. Moved SymbolCollection after GraphChecking in the fsproj to enable reuse of FileContentMapping's full AST identifier extraction. End-to-end test: files deliberately listed in wrong order (C, B, A) compile successfully with --file-order-auto+. All 4 tests pass.
…ck 04 Phase 1) Fix three dependency resolution bugs found by the inference test corpus: 1. False cycles from auto-generated files (AssemblyInfo.fs, buildproperties.fs, AssemblyAttributes.fs from obj/) — fixed by detecting obj/ paths and giving them empty dependency sets. 2. False cycles from shared namespace prefixes — when multiple files define modules under the same namespace (e.g., MyApp.Types, MyApp.Logic), the shared prefix "MyApp" was creating false dependencies between all files. Fixed by tracking which names are defined by multiple files and skipping them during dependency resolution. 3. [<EntryPoint>] must-be-last violation — after reordering, the IsLastCompiland flag on ParsedImplFileInput still pointed to the originally-last file. Fixed by updating the flag on the reordered sequence so only the actual last file has isLastCompiland=true. Inference sensitivity test corpus validates 4 patterns with wrong file order: - SRTP (statically resolved type parameters) with custom operators - Record field disambiguation across files - Union case discrimination across files - Operator overload resolution with custom types All 8 tests pass (4 core + 4 inference). Moved SymbolCollection after GraphChecking in fsproj to enable reuse of FileContentMapping AST walker.
…fallback
Three fixes to dependency resolution found by testing against Fantomas (31 files):
1. .fsi/.fs pairing: Signature files must immediately precede their
implementation. The topological sort now detects sig/impl pairs and
enforces sig-before-impl ordering in the final sequence.
2. Opens vs identifier refs: Open statements ("open Utils") now always
create dependencies regardless of shared prefixes — they're explicit
imports. Only PrefixedIdentifier refs (qualified names from expressions)
skip shared namespace prefixes. This prevents false "not defined" errors
when files use open to import sibling modules.
3. Cycle graceful fallback: When circular dependencies are detected (Level A),
instead of crashing with failwith, the compiler warns and falls back to
the original file order. This allows the build to proceed with normal F#
ordering errors rather than an unhandled exception.
Adds test-real-project.sh for testing against real F# projects by shuffling
their file order and building with --file-order-auto+.
Results: Fantomas.Core detects a genuine cycle (3 files with mutual deps) and
correctly falls back. This is expected Level A behavior — Level B cycle groups
will handle this in Track 03.
When --file-order-auto is active and mutually recursive types use the 'and' keyword, warning FS3885 is emitted: "The 'and' keyword for mutually recursive types is unnecessary when using --file-order-auto. Consider placing types in separate declarations. This keyword may be removed in a future version." Implementation: - Added fileOrderAuto field to TcFileState (CheckBasics.fs/fsi) - Warning emitted in CheckDeclarations.fs when SynModuleDecl.Types has multiple typeDefs (the 'and' case) and cenv.fileOrderAuto is true - Warning 3885 added to FSComp.txt resource strings The fileOrderAuto field is currently hardcoded to false in cenv.Create calls — threading it from TcConfig through CheckOneImplFile/CheckOneSigFile is deferred to Track 06 (migration tooling) to avoid a deep refactor of the type checking pipeline signatures.
…Track 06)
MSBuild integration:
- Added <FSharpAutoFileOrder> property to Microsoft.FSharp.NetSdk.props
(defaults to false). Users can enable auto file ordering in their .fsproj:
<FSharpAutoFileOrder>true</FSharpAutoFileOrder>
- Added FileOrderAuto property to the Fsc MSBuild task (Fsc.fs)
- Wired property through Microsoft.FSharp.Targets to pass --file-order-auto+
Localized help text:
- Added optsFileOrderAuto resource string to FSComp.txt
- Updated CompilerOptions.fs to use FSComp.SR.optsFileOrderAuto
Threading fileOrderAuto through the type checker:
- Added fileOrderAuto parameter to CheckOneImplFile and CheckOneSigFile
- Threaded from TcConfig through ParseAndCheckInputs.fs into the cenv
- The and keyword deprecation warning (FS3885) now fires when the flag
is active and mutually recursive types use the and keyword
All 8 tests pass (4 core + 4 inference).
Fix: normalize path separators (\ vs /) in buildSigImplPairs so Windows-style paths in .fsproj files (e.g. 'Facilities\AsyncMemoize.fsi') correctly match Unix-style paths from ParsedInput.FileName on Linux/macOS. Without this, sig/impl pairs weren't detected and .fsi files could land after .fs files, triggering FS0239 "implementation already given". Also adds tests/file-order-auto-test/self-host-test.sh — shuffles the 369 Compile items in FSharp.Compiler.Service.fsproj and rebuilds with --file-order-auto+ to verify the compiler can compile itself with randomized file order. XLF translation files auto-updated from FSComp.txt additions (chkAndKeywordDeprecatedWithFileOrderAuto + optsFileOrderAuto).
When --file-order-auto detects a cycle group (multiple files with mutual dependencies), the compiler now synthesizes a single ParsedImplFileInput that wraps the group's content in a `namespace rec <commonPrefix>` and hands it to the existing F# type checker. This reuses F#'s mature mutual-recursion infrastructure rather than duplicating it. Architecture (B4 design from conductor/tracks/03): - SymbolCollection: Tarjan's SCC algorithm (computeSCCs) - New CompilationUnit type: SingleFile | CycleGroup - computeCompilationUnits returns dependency-ordered units, partitioning auto-generated files first and preserving .fsi-before-.fs pairing - CycleGroupProcessing.synthesizeCycleGroupImpl detects the common namespace prefix, wraps modules as nested decls inside `namespace rec` - fsc.fs TypeCheck pipeline: replaces order-only reordering with compilation-unit dispatch (single files unchanged, cycle groups synthesized before reaching CheckClosedInputSet) Validation: - Cross-file mutual record test (Tree/Forest with mutual references) compiles AND runs correctly end-to-end - Inference tests still pass (4/4) - Memory bounded — Tarjan is O(V+E), synthesis is light AST manipulation Known limitation: - Cycles only detected when refs use `open` or fully-qualified paths. Namespace-relative refs (e.g., sibling module access from inside `namespace Fantomas.Core`) still go undetected. Dependency analyzer needs to try namespace-relative paths — separate enhancement. Other fixes in this commit: - Tarjan emits in topological order; removed incorrect Seq.rev - Memory dedup: bound IdentifierRefs growth to prevent OOM on large projects - Two-level retry for cycles: full refs → opens-only → fallback - Path normalization for sig/impl pairing across platforms
Two improvements to Level B cycle group processing:
1. Namespace-relative dependency resolution
When a file declared as `module Foo.Bar.Baz` references `Sibling.X`,
the analyzer now tries `Foo.Sibling.X`, `Foo.Bar.Sibling.X`, and
`Foo.Bar.Baz.Sibling.X` against the export map (in addition to the
literal `Sibling.X`). This catches cycles between sibling modules
in shared namespaces.
2. Cycle group sig-pairing handling
- SCC-detected cycle groups now pull in any .fsi files paired with
their .fs members (so sig+impl stay together)
- Cycle groups containing any .fsi file fall back to original order
(Level A behavior) — the synthetic impl wrapper changes structure
in ways the standalone sig file can't match. Future work: figure
out how to coordinate sig+impl in synthesized groups.
Cycle test (Tree/Forest with namespace-relative refs) now passes with
no fully-qualified workarounds. Fantomas.Core falls back cleanly due
to its sig files; needs future sig-coordination work.
The symbol collection pre-pass populates TcEnv with Entity stubs (TNoRepr). For most projects this is fine: the stubs get shadowed by real entities as the type checker processes each file. But FSharp.Core defines primitive types like `string`, `int`, etc. The stubs we create for these end up shadowing the real definitions during the early self-bootstrap phase, breaking compilation with errors like: error FS0193: The module/namespace 'Microsoft.FSharp.Core' from compilation unit 'FSharp.Core' did not contain the namespace, module or type 'string' FSharp.Core is bootstrap code — its file order is hand-curated by maintainers, no cycles exist, and auto-ordering provides no value. Skipping the enter phase when tcConfig.compilingFSharpCore is true is the correct guard. Verified: - FSharp.Core compiles cleanly with FSharpAutoFileOrder=true (was 200 errors) - Cross-file cycle test still works - F# compiler self-host: now fails on cycle-with-sig groups (expected Level B limitation, not a regression)
Verifies that --file-order-auto+ produces identical error messages to manual mode for common F# error categories. Tests: - undefined_name (typo on a value) - undefined_module (typo on a module) - type_mismatch (wrong type in let binding) - missing_field (record field that doesn't exist) - wrong_arity (extra argument to function) - missing_open (function used without open statement) diff-errors.sh runs each project in both modes, normalizes error output, and diffs. Result: 6/6 identical — no error message regressions.
The synthesis+reorder pipeline previously lived inline in fsc.fs's TypeCheck. Lift it into CycleGroupProcessing.applyAutoFileOrder so it can be called from any compiler entry point — fsc.fs (already wired) and FCS's BackgroundCompiler/IncrementalBuilder (next step, requires hooking before the per-file incremental processing kicks in). This is Phase 1 of Track 05 (FCS API adaptation). The flag itself is already accepted by FCS via OtherOptions (no FSharpProjectOptions change needed). What's still needed: have BackgroundCompiler call applyAutoFileOrder before constructing the IncrementalBuilder so its file list is dependency-ordered. All existing tests pass: cycle test, inference suite, error corpus.
Adds Level-A reordering to FCS so projects checked via FSharpChecker (IDE, Ionide, etc.) honour --file-order-auto+ in OtherOptions, matching build-time behaviour for the common no-cycle case. - CycleGroupProcessing.computeReorderedFileNames: lightweight reorder that pairs collectFileDeclarations with FileContentMapping enrichment so qualified cross-file references (`Test.B.value`) resolve correctly, mirroring runEnterPhase. Cycle groups stay in original position (Level B synthesis remains build-only). - IncrementalBuild.fs: when tcConfig.fileOrderAuto is set and not compiling FSharp.Core, eagerly pre-parse the source list, compute the reordered names, and feed them to the IncrementalBuilder. Reorder failures fall back silently to the original order. - fcs-smoke-test: standalone exe that drives FSharpChecker.ParseAndCheckProject against our locally-built FCS to confirm the hook fires end-to-end. Verified: smoke test PASS, 4/4 inference tests PASS, 6/6 error-corpus diagnostics identical between modes.
…s (Track 05 Phase 3) The deprecation warning for the `and` keyword was only emitted from the mutually-recursive type-checking path, so a normal `type X = ... and Y = ...` in an ordinary module was slipping through silently. Added the same check to the non-recursive SynModuleDecl.Types branch so any `and`-joined type declaration triggers FS3885 when --file-order-auto+ is set. Adds fcs-ide-smoke-test, an executable that drives the FCS APIs the IDE depends on against an auto-ordered project: - ParseAndCheckProject: project-level type check returns no errors for a wrong-order project (validates the IncrementalBuilder Phase 2 hook). - GetDeclarationListSymbols: completions on `Test.B.` from inside FileA surface symbols defined in FileB even though FileB is listed later in the source list. Latency reported as a sanity check (~30ms locally). - GetSymbolUseAtLocation: go-to-definition resolves to the right file across the auto-order boundary. - GetUsesOfSymbol: find-all-references hits both definition and use sites. - FS3885: the `and`-keyword deprecation warning surfaces in Diagnostics. What is *not* automated (still needs a human): - Ionide / VS Code end-to-end UX: completion popups, hover, squiggle rendering. - Visual Studio F# extension: Windows-only, GUI-bound. The FCS-level checks cover the contract those IDEs consume; the manual smoke tests are about UX feel rather than correctness of the underlying API.
…05 Phase 4) A .fsi/.fs pair declaring the same module was inflating the per-name contributor count in buildExportMap, so every module in a sig-paired project ended up flagged as a "shared prefix". Identifier-ref resolution skips shared prefixes (to avoid false cycles between unrelated files in the same namespace), which meant any consumer of a sig-paired module silently got zero dependency edges and the auto-order reduced to original input order. Fix: collapse sig+impl pairs to one entity when computing whether a name has multiple contributors. The pair-detection logic is inlined inside buildExportMap (the dedicated buildSigImplPairs helper lives later in the file; pulling it forward would have meant moving the helpers it depends on too). Adds two .fsi-handling fixtures + a runner driving the local fsc: - partial-fsi: a project mixing a sig-paired Lib (.fsi/.fs) with a sig-less Util, listed in wrong order. - fsi-ordering: the consumer/main use a type defined via a paired .fsi/.fs, with the consumers listed before the type files. Both fail without the flag and pass with --file-order-auto+, with the sig file landing immediately before its impl in every passing case. Regression sweep clean: inference (4/4), error-corpus (6/6 identical), FCS project smoke (PASS), FCS IDE smoke (7/7).
… 06) Wraps the bounded items in Track 06. Docs (`docs/file-order-auto-*.md`): - `file-order-auto-migration.md` — user-facing migration guide: enabling via MSBuild/fsc/FCS, what changes vs. doesn't, cycle handling, migrating off `and`, FS3885 suppression, known limitations, FAQ, and a brief architecture sketch. - `file-order-auto-release-notes.md` — fork release notes: highlights, defaults, test coverage matrix, what was NOT validated this iteration (full upstream test suite, OSS sweep, perf characterisation, IDE UX). `tests/file-order-auto-test/deprecation-test/` — direct fsc fixture proving FS3885 fires only in auto mode and is suppressable via `--nowarn:3885`. Asserts exact warning count (2) for a file with two `and`-joined type chains. `tests/file-order-auto-test/end-to-end/` — scaffolds a fresh `dotnet new console -lang F#`, drops in three interdependent files in deliberately wrong order (Program → Geometry → MathHelpers), builds via `-p:DotnetFscCompilerPath` + `-p:OtherFlags=--file-order-auto+` against the locally-built fsc, and verifies the produced exe prints the expected output. The smoke test goes via OtherFlags rather than `<FSharpAutoFileOrder>` because `dotnet build` uses the SDK-installed FSharp.Build task (not this fork's), so the `<FSharpAutoFileOrder>` MSBuild property won't be translated until the fork's FSharp.Build ships in an SDK. The behaviour is identical at the fsc level; the fsproj documents the user-facing knob. Track 06 plan updated to reflect what's done, deferred (full upstream test suite, large-OSS sweep, FSI wiring, migration analyzer CLI), and why each deferral is bounded.
Two upstream tests verify fsc's --help output against checked-in reference text and fail on this fork because we added --file-order-auto[+|-] to the options list. Two real changes plus the auto-regenerated xlf strings: - src/Compiler/FSComp.txt: optsFileOrderAuto was missing the standard `(%s by default)` wrapper used by every other on/off switch's help string, so the rendered line trailed with a bare "off". Fixed to match --realsig, --crossoptimize, etc. - tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/misc/compiler_help_output.bsl: inserted the new `--file-order-auto[+|-]` line between --realsig and --pathmap, matching where the option block places it. - tests/FSharp.Compiler.Service.Tests/expected-help-output.bsl: same insertion, narrower column wrapping to match this baseline's format. - src/Compiler/xlf/*.xlf: auto-regenerated to pick up the corrected optsFileOrderAuto source string. Confirmed by running the full local test suite (./build.sh -c Release --test): 0 failures across all 15,404 tests in FSharp.Compiler.ComponentTests, FSharp.Compiler.Service.Tests, FSharp.Compiler.Private.Scripting.UnitTests, FSharp.Build.UnitTests, and FSharp.Core.UnitTests. Manual-mode behaviour is bit-for-bit upstream; the auto-order code path is properly gated on tcConfig.fileOrderAuto.
While auto-ordering Argu, FsCheck, and FSharpPlus to validate the feature against real code, the dependency analyser was conjuring false cycles in multi-namespace projects. The faulty case: `Internals.TypeClass.fs` (namespace `FsCheck.Internals`) contains `Result.isOk` referring to `FSharp.Core.Result`. Our analyser prefix-resolved the bare `Result` against every enclosing namespace, including the parent `FsCheck`. `FsCheck.Result` exists as a type declared in `Testable.fs`, so the analyser conjured a false edge TypeClass → Testable → ... → TypeClass → cycle group → FS3200 fires because cycle-group synthesis wraps everything in `namespace rec`. The fix lives in `getEnclosingPrefixes` (SymbolCollection.fs:686): a file's parent namespaces are visible only when the file is a NamedModule (`module X.Y`), not when it is a DeclaredNamespace (`namespace X.Y`). F# does not auto-import parent namespaces of a declared namespace, so trying them is unsound regardless of whether the analyser can confirm the match would be a false positive. This fix is structurally correct and not a workaround. NamedModule's implicit parent visibility is required by tests like the end-to-end fixture (Geometry.fs in `module E.Geometry` references `MathHelpers.pi` where `MathHelpers` is in `namespace E`); the existing test still passes. Adds `tests/file-order-auto-test/oss-sweep/RESULTS.md` documenting how to reproduce the sweep, what passes (Argu — real-world success on a ~30-file library) and what doesn't (FsCheck, FSharpPlus). The remaining failure mode is a known limitation of `FileContentMapping.PrefixedIdentifier` truncating the last segment of qualified refs, which makes `Random.CreateWithSeedAndGamma` (a real cross-file static call) and `Result.isOk` (an FSharp.Core call) collide for the analyser inside a shared project namespace. The right fix is a richer AST walker that keeps the full path. That's structural work out of scope for this iteration. Regression sweep clean: inference 4/4, fsi 2/2, error-corpus 6/6 identical, deprecation 3/3, end-to-end PASS, FCS smoke PASS, FCS IDE smoke 7/7. Argu builds cleanly under --file-order-auto+.
… of OSS unblock) FileContentMapping.PrefixedIdentifier truncates the last segment of every long ident via skipLast=true. That's correct for upstream's parallel checker (which only needs module qualifiers) but wrong for our analyser: `Random.CreateWithSeedAndGamma` (a real cross-file static call to a project type) and `Result.isOk` (FSharp.Core) both reduce to the same single-segment path, so we can't distinguish a real dep from an FSharp.Core call. Adds `collectFullPathRefs` — a hand-rolled walker that visits every SynExpr/SynType/SynPat/SynMemberDefn/SynModuleDecl variant in this fork's AST and emits each LongIdent with its full path preserved. Wired into `runEnterPhase` in place of the FCM-based PrefixedIdentifier extraction. FCM still feeds nested-module Opens (a small, separate concern). This commit is necessary infrastructure but does NOT yet change matching behaviour or fix any failing project. Type-member registration and matching-policy refinement land in follow-up commits. Walker compiles cleanly and all existing fixtures still pass: - inference 4/4 - fsi 2/2 - error-corpus 6/6 identical - deprecation 3/3 - end-to-end PASS - Argu PASS - FsCheck/FSharpPlus still fail (same errors as before — expected, the matching policy hasn't changed yet).
…block Phase 2-4) Builds on Phase 1 (full-path walker) with the analyser changes that eliminate the false cycles in real-world projects. - TypeDeclStub now carries MemberNames; collectTypeDeclStub + collectTypeDeclStubFromSig walk SynTypeDefnRepr.ObjectModel members, the SynTypeDefn outer members slot, and AbstractSlot/AutoProperty/ ValField cases to extract names. - buildExportMap registers four kinds: Module, Type, Value, Member. Module-level let bindings get `qualName.bindingName` as Value; static/instance members get `qualName.TypeName.MemberName` as Member. - New ExportKind tracking lets the matching policy distinguish a real module match (legitimate cross-file dep) from a bare type-prefix match (likely a member call where the member isn't registered, e.g. an FSharp.Core method whose qualifier collides with a project type). resolvePathDeps walks prefixes longest-first; bare-Type matches are rejected when no Member match is registered. - Opens now use skipShared=true. `open FsCheck` from a file already in `namespace FsCheck` was previously broadcasting deps to every contributor of that namespace, manufacturing a giant SCC. Opens declare scope, not specific cross-file deps — identifier refs handle the actual links. OSS sweep result with this change: - Argu still PASS. - FsCheck: cycle problem eliminated (was 36 FS3200 errors). Now 136 real type-resolution errors — legitimate deps via `open` aren't always being detected, but it's a different (and tractable) class of issue from the false cycles. Build no longer attempts cycle-group synthesis on a falsely-detected cycle. - FSharpPlus: cycle problem also eliminated (was 166 errors). Now 2 internal compiler errors `FS0193 ... Key: 'Control'` — the enter phase appears to add conflicting stubs for `module Control` declared in multiple places. Separate bug, not the cycle issue. Regression sweep clean: inference 4/4, fsi 2/2, error-corpus 6/6 identical, deprecation 3/3, end-to-end PASS, FCS smoke PASS, FCS IDE smoke 7/7. Argu builds cleanly under --file-order-auto+.
…ocumented The walker + member registration + kind-aware matching + opens-skip-shared work (commits 9901547 and 2708300) eliminated the false-cycle issue that blocked FsCheck and FSharpPlus. Updated state: - Argu: PASS (unchanged). - FsCheck: cycle problem GONE. Was 36 FS3200 cycle errors, now 136 real type-resolution errors — analyser correctly produces a DAG, but some deps via `open`d namespaces aren't detected. Concrete next-step documented. - FSharpPlus: cycle problem GONE. Was 166 FS3200 errors, now 2 internal compiler errors `FS0193 ... Key: 'Control'`. Separate bug in the enter-phase stub building when many files share a namespace. Likely fix sketched. Recommendation in the doc: ship as-is for v1 PR. Argu works. The fundamental design (full-path walker + kind-aware matching) is sound. The remaining issues are bounded and have clear next steps, but neither is required to demonstrate the feature works on conventional code.
…adow
Three more analyser refinements that took FsCheck from 136 errors to
130 and eliminated FSharpPlus's compiler crashes (FS0193 → 200 real
type errors).
1. Type-parameter stubs (`mkTypeEntityStub`): the enter-phase entity
shells declared every type as `Typars=[]`. References like
`MyType<'A>` from another file then hit FS0033
"non-generic type does not expect any type arguments" before the
real impl was visible. Now we synthesise rigid typars matching
`TypeDeclStub.TypeParamCount`.
2. Cross-namespace cycle synthesis guard
(CycleGroupProcessing.fs:286): when a cycle group spans multiple
namespaces, the synthesis would wrap a `namespace X.Y` file as a
nested `module Y` inside `namespace rec X`. Other (non-cycle)
files declaring `namespace X.Y` then conflict with the synthetic
`module Y` → FS0247 "namespace and module both occur". Now we
detect any file whose namespace extends beyond the common prefix
and fall back to original order rather than synthesise.
3. Opens-as-prefixes for ident resolution + local-name shadowing:
- For each `open Foo.Bar` whose target is a known Module, treat
`Foo.Bar` as an additional resolution prefix for ident refs in
this file. Lets `TypeClass.TypeClass<...>` from a file with
`open FsCheck.Internals` resolve to
`FsCheck.Internals.TypeClass.TypeClass`.
- But suppress opens-as-prefix when the ref's first segment is
LOCALLY defined (a nested module/type/value of this file).
Without this, `Prop.safeForce` from inside Testable.fs (which
has both `open FsCheck.FSharp` AND a local `module Prop = ...`)
would falsely match `FsCheck.FSharp.Prop` and create a cycle
with FSharp.Prop.fs.
OSS sweep state:
- Argu still PASS.
- FsCheck: 130 type errors. Cycle groups fully eliminated by the
combined guard + local-shadow + opens-as-prefix changes — every
file now resolves to a SingleFile compilation unit. Remaining
errors are real ordering issues yet to resolve.
- FSharpPlus: 200 errors. The internal compiler errors are gone;
these are now real type-resolution issues, same class as FsCheck.
Regression sweep clean across all existing fixtures.
The enter phase was creating ModuleOrNamespace stubs for every module in every file (top-level NamedModule files, nested modules inside namespaces). When the real `module X = ...` declaration ran, F# saw our stub as an already-existing entity and rejected the redeclaration with FS0245 "X is not a concrete module or type". Types tolerate forward declaration: F# expects a type stub to be filled in by the real definition. Modules don't have the same mechanism — a module entity in scope IS the module. Two changes: 1. Filter private/internal types and nested modules out of stub building (SymbolCollection.fs:548). Other files can't reference them anyway; stubbing them only risks conflict. 2. Drop module stubs entirely from `buildTopLevel`. Top-level NamedModule files don't get a stub — the file's own `module X = ...` is the only declaration. Nested modules inside namespaces also get no stub. We still register their NAMES in the export map (so dependency analysis works), but don't construct an entity that would conflict during type-check. Type stubs still synthesise typars matching `TypeParamCount` so `MyType<'A>` from another file resolves correctly. OSS sweep state: - Argu: PASS (unchanged). - FsCheck: **PASS** (was 130 errors → 0). Real-world success on a hard SRTP-heavy project that previously failed under file-order-auto+ in every form. - FSharpPlus: 12 errors (was 200). Down to FS3200 cycle-synth issues in Extensions/Seq.fs and Extensions/List.fs where the synthesis splices multiple `namespace FSharpPlus` files into a single `namespace rec FSharpPlus`, leaving second+ files' `open` statements not-first in the resulting module. Bounded next step: wrap spliced content in anonymous modules so opens stay in scope. Regression sweep clean: inference 4/4, end-to-end PASS, error-corpus 6/6 identical.
…Plus now builds!) Real-world F# interleaves `open` statements with let bindings inside nested modules — perfectly legal in non-recursive code. When our cycle synthesis wraps everything in `namespace rec X`, F# requires opens to be FIRST in each module/namespace block (FS3200), and the interleaved opens trip that rule. Adds `hoistOpens`: walks a SynModuleDecl list and reorders each block so `SynModuleDecl.Open` entries come first, recursing into nested modules so opens inside `module Seq = ...` (etc.) also get hoisted. Applied at the start of `rewriteAsNestedDecls`. This change is safe: opens don't have side effects (they only affect name resolution), so reordering them within a module body changes nothing semantically. The only thing it changes is satisfying F#'s "opens-first-in-rec" rule. OSS sweep state: - Argu: PASS - FsCheck: PASS - FSharpPlus: **PASS** All three target projects now build cleanly under `--file-order-auto+`. The cycle-detection pipeline + namespace-rec synthesis pipeline + the analyser changes from the prior commits combine to handle real-world F# code. Regression sweep clean: inference 4/4, fsi 2/2, error-corpus 6/6 identical, deprecation 3/3, end-to-end PASS, FCS smoke PASS, FCS IDE smoke 7/7.
Argu, FsCheck, and FSharpPlus all build cleanly under --file-order-auto+. Documents the chain of analyser refinements that took us from "FsCheck/FSharpPlus fail with cycle errors" to "they all work": - Full-path AST walker (replaces FCM truncation). - Type member + module-let registration in export map. - Kind-aware matching (reject bare-Type prefix matches). - Opens skip shared-prefix targets to avoid broadcast. - Opens-as-prefixes for ident resolution, with local-name shadowing. - NamedModule vs DeclaredNamespace prefix scoping. - Type stubs include type parameters. - No module stubs (only types — modules require concrete bodies). - Cross-namespace cycle synthesis guard. - Recursive open-hoisting in synthesised cycle groups. The full upstream test suite is now running to confirm zero regressions. Existing fixtures (inference, fsi, error-corpus, deprecation, end-to-end, FCS smoke, FCS IDE smoke, cycle-test-b4) all pass.
Three sites in SymbolCollection.fs computed `name = match ids with | [id] -> id | _ -> List.last ids`. The wildcard branch crashed with `List.last []`
on any malformed/parse-recovered SynComponentInfo or SynModuleOrNamespace
whose longId is empty. The exception bubbled up as
`FS0193 internal error: One or more errors occurred. (The input list was empty. (Parameter 'list'))`
through Array.Parallel.mapi.
Real-world example: Expecto's library has at least one such pattern that
produces an empty longId during parse. Auto-order failed with FS0193 even
though baseline compilation worked fine.
Each match now adds a `[] -> Ident("", range0)` arm. The empty Ident
flows through the rest of the analyser as a name with empty text — it
gets registered in the export map under a key that no real reference
resolves to, so it's harmless.
Sites guarded:
- collectTypeDeclStub (impl)
- collectTypeDeclStubFromSig
- collectImplDecls (NestedModule case)
- collectSigDecls (NestedModule case)
- runEnterPhase top-level module name (impl)
- runEnterPhase top-level module name (sig)
Tested against 13 OSS F# projects on macOS arm64 + .NET 10 SDK:
PASS (7): Argu, FsCheck, FSharpPlus, FsToolkit.ErrorHandling,
Expecto, FSharp.Data.Json.Core, Fable.Promise.
FAIL but baseline-broken too (5, all environmental):
- FsPickler: F# `error FS0561` — pre-existing language incompatibility.
- Aether: targets net45, NU1202.
- Fantomas.Core: NU1403 hash mismatch.
- Fable.AST: netstandard2.0 missing System.ReadOnlySpan.
- Paket.Core: paket restore fails.
FAIL real-flag (1): Suave. Baseline already broken on .NET 10 (FS0971
'Undefined value this' in `task {}` — F# semantic gap unrelated to our
flag). Auto adds 26 more errors via the AutoOpen-tracking limitation
documented below.
Documents the AutoOpen limitation as a known gap. Three attempts at
"register AutoOpen aliases under the parent namespace" all caused
regressions (Suave 30→200, Expecto 0→6) because the aliases introduced
new false-match cycles or shadowed local scopes. The structural fix
needs separating "alias for resolution" from "name registered in
exportMap". Out of scope for this iteration.
Commit history of the unblock work documented in the doc:
- 9901547 — full-path AST walker
- 2708300 — type member + module-let registration, kind-aware match,
opens skip shared
- 6117008 — typar stubs, cross-namespace cycle guard,
opens-as-prefixes + local-name shadowing
- ae9beb4 — no module stubs (FsCheck unblocks)
- 49a380e — recursive open-hoisting in synthesis (FSharpPlus unblocks)
- 319ac62 — empty-longId guards (Expecto unblocks)
…gle-ident capture (Suave unblocks)
The Suave 26-extra-errors-vs-baseline issue was the AutoOpen gap:
`[<AutoOpen>] module Suave.Runtime` is invisible to our analyser, so
Connection.fs (with `open Suave; ... SocketBinding`) had its dep on
Runtime.fs missed, the file got placed too early, and type-check failed.
Three combined changes:
1. Separate `aliasMap` for AutoOpen resolution (NOT mixed into exportMap).
When a nested or top-level module is `[<AutoOpen>]`, its content
(Types, Values, Members) is registered in `aliasMap` under the parent
namespace path. `aliasMap` is consulted only as a fallback in
`addDepFromExportMap` after exportMap miss — never feeds
sharedPrefixes/kinds/Module-tracking. Earlier attempts at AutoOpen
aliases inside the main exportMap caused regressions because the
aliases triggered new false-match cycles via shared-prefix logic;
keeping aliases separate avoids that.
2. Sig→impl redirect: when a file's identifier refs resolve to a `.fsi`
file, the dep is redirected to the paired `.fs` impl. This way Tarjan
places the impl at the correct topological position and the
pair-rewriting step (which emits `[sig; impl]` at the impl's
position) keeps consumers correctly ordered after the pair. The
previous code emitted the pair at the impl's position but consumers
that depended on the SIG ended up before the pair.
3. Surgical single-ident capture: `SynExpr.App(funcExpr=SynExpr.Ident _)`
captures the function ident as a 1-segment ref. Lets
`transferStream conn stream` from a file with `open Suave.Sockets`
resolve via the alias `Suave.Sockets.transferStream` to AsyncSocket.fs.
Crucially we DON'T capture every `SynExpr.Ident` (full single-ident
capture broke FsToolkit.ErrorHandling by triggering false cycles via
refs to local parameters/values).
Top-level NamedModule with `[<AutoOpen>]` (`[<AutoOpen>] module
Suave.Sockets.AsyncSocket`) also registers content under the parent
path now (separate from the existing nested-AutoOpen handling).
OSS sweep state:
- Argu: PASS
- FsCheck: PASS
- FSharpPlus: PASS
- FsToolkit.ErrorHandling: PASS
- Expecto: PASS
- FSharp.Data.Json.Core: PASS
- Fable.Promise: PASS
- Suave: baseline 30 == auto 30 (errors are identical pre-existing F#
`task {}` issues, not flag-related — `diff` is empty)
Five projects fail-baseline-too (env-broken: paket lock, .NET version,
NuGet cache): FsPickler, Aether, Fantomas.Core, Fable.AST, Paket.Core.
8/8 buildable targets — auto adds zero errors over baseline.
Regression sweep clean: inference 4/4, fsi 2/2, error-corpus 6/6,
deprecation 3/3, end-to-end PASS.
7 PASS + 1 (Suave) where auto-mode error set is byte-identical to baseline. The remaining 5 fail-baseline-too on this toolchain (paket lock, .NET version, NuGet hash, ReadOnlySpan in netstandard2.0 — all environmental, not flag-related). Documents the full sequence of analyser refinements: walker, member registration, kind-aware matching, opens skip shared, opens-as-prefixes with local-name shadowing, NamedModule vs DeclaredNamespace scoping, typar stubs, no module stubs (only types), cross-namespace cycle guard, recursive open-hoisting, empty-longId guards, separate aliasMap for AutoOpen, sig→impl redirect, surgical single-ident capture.
… results - New docs/file-order-auto-design.md: engineering companion covering the enter phase, symbol collection, exportMap/aliasMap split, cycle group synthesis, sig+impl pairing, and the chain of analyser refinements that took the implementation from skeleton to OSS-buildable. - docs/file-order-auto-release-notes.md: replace stale "not validated" caveats with current state — 15,404 upstream tests passing, 13-project OSS sweep, and 7 explicitly-PASS targets. Link the new design doc from the architecture section.
Upstream main took FS3885 (parsLetBangCannotBeLastInCE) and FS3886 (tcListLiteralWithSingleTupleElement) for unrelated warnings during the rebase window. Our chkAndKeywordDeprecatedWithFileOrderAuto is now numbered FS3887. Updates: - docs (migration, release-notes, design): all FS3885 / 3885 → FS3887. - docs/release-notes/.FSharp.Compiler.Service/11.0.100.md: move our entry from "Breaking Changes" (where the rebase merge-3-way landed it) back to "Added" — opt-in flag with default off is not a breaking change. Update FS3885 → FS3887 in the entry text. - tests/file-order-auto-test/deprecation-test/run-all.sh: assertions and grep patterns updated to FS3887. - tests/file-order-auto-test/fcs-ide-smoke-test/Program.fs: ErrorNumber filter and labels updated to 3887. - tests/file-order-auto-test/oss-sweep/RESULTS.md: reproduction instructions reference --nowarn:3887. Build clean, deprecation fixture 3/3, inference 4/4, fsi 2/2, error-corpus 6/6.
bryancostanich
force-pushed
the
fix_dogmatic_file_order_nonsense
branch
from
b2e1887 to
9da94fe
Compare
The fixtures are run via shell scripts in tests/file-order-auto-test/, not through the batched CI test runner. Adding the directory to excludedPrefixes mirrors the treatment of tests/benchmarks/, tests/AheadOfTime/, tests/EndToEndBuildTests/, etc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
I love F#. I've also bounced off using it more, for years, mainly because of one thing: the requirement to manually maintain a topological order of source files in
.fsproj, and the resulting reliance onand-joined recursive type chains to work around it. It feels like accidental complexity in a language that otherwise does so much right.This PR is an opt-in fix that lets the compiler compute the file order itself. The default behaviour is unchanged — manual ordering still works exactly as it does upstream, and this fork is byte-identical to upstream when the flag is off. With the flag on, the compiler reorders the inputs based on what each file declares and what each file references.
Not a proposal to change F#'s semantics. A parallel mode you can opt into per-project.
What it does
--file-order-auto+— new compiler flag, off by default.<FSharpAutoFileOrder>true</FSharpAutoFileOrder>— MSBuild property.FSharpProjectOptions.OtherOptions— Ionide and other FCS hosts can opt in.and-keyword deprecation (FS3885) — under auto-mode,and-joined type chains emit a warning. Suppressable. Silent in manual mode.How it actually works
The skeleton was easy. Making it match baseline on real F# code was a chain of analyser refinements, each driven by a specific OSS project's failure mode:
FileContentMapping's truncation).qualName.TypeName.MemberName).Module | Type | Value | Member; reject bare-Type matches).aliasMapfor[<AutoOpen>]— never mixed into the mainexportMap, consulted only as fallback (the unblock that landed Suave)..fsi/.fspairs.SynExpr.Appfunction heads (recovers AutoOpen function aliases without breaking local bindings likelet result = ResultBuilder()).FS0247).FS3200).FS0245).FS0193 internal error).Full design and the rest of the chain in
docs/file-order-auto-design.md.Validated against
13 real-world OSS F# projects under
--file-order-auto+. Auto-mode adds zero errors over baseline for every buildable target.diffempty)Plus targeted fixtures at
tests/file-order-auto-test/: inference (4/4), fsi (2/2), error corpus (6/6 byte-for-byte parity), deprecation (3/3), FCS + IDE smoke, cycle synthesis.Full upstream F# test suite passes on this branch: 15,404 tests, 0 failures.
Status — draft
Filing as a draft to share the design and validation, and to see whether the team would consider taking this in any form. Known limitations:
.fsifiles fall back to original order inside the group.dotnet fsinot wired (fsc + FCS only).Docs
docs/file-order-auto-release-notes.md— what's in the fork, validationdocs/file-order-auto-migration.md— user guide, cycles,andmigrationdocs/file-order-auto-design.md— engineering deep-dive