Skip to content

RFC FS-1337: Optional Automatic File Order#832

Draft
bryancostanich wants to merge 1 commit intofsharp:mainfrom
bryancostanich:add-fs-1337-optional-automatic-file-order
Draft

RFC FS-1337: Optional Automatic File Order#832
bryancostanich wants to merge 1 commit intofsharp:mainfrom
bryancostanich:add-fs-1337-optional-automatic-file-order

Conversation

@bryancostanich
Copy link
Copy Markdown

Drafting RFC FS-1337 covering dotnet/fsharp#19647 (opt-in --file-order-auto+).

This is filed as a draft because the auto-inference path doesn't yet have its own approved-in-principle target. Suggestion #309 covers an adjacent direction (explicit TypeScript-style imports). I've left a comment on the implementation PR (dotnet/fsharp#19647) asking whether #309's scope should be split into two approval tracks (auto-inference + explicit imports), or whether a brand-new fslang-suggestion should be filed for the auto-inference path.

The two are complementary, not competing. Posting the RFC now so reviewers have something concrete to react to when deciding which approval target this should anchor on.

Summary

--file-order-auto+ is an opt-in compiler flag (off by default) that lets the compiler topologically sort source files from their declarations and qualified-identifier references. No new syntax. Cycles get wrapped in a synthesised namespace rec on the build path. Validated against 7 real OSS F# projects (Argu, FsCheck, FSharpPlus, FsToolkit.ErrorHandling, Expecto, FSharp.Data.Json.Core, Fable.Promise) — auto-mode adds zero errors over baseline on every buildable target.

See the RFC document for full design, alternatives, drawbacks, and validation evidence.


## Summary

Add an opt-in compiler flag, `--file-order-auto+` (off by default), that lets the compiler figure out the file order itself by reading what each file declares and what it references. List your source files (impl `.fs` and signature `.fsi`) in any order in the `.fsproj`, and the compiler topologically sorts them before type checking. Files that mutually reference each other get wrapped in a synthesised `namespace rec` on the build path, so cross-file mutual recursion just works without `and`-chains. Wire it through MSBuild with `<FSharpAutoFileOrder>true</FSharpAutoFileOrder>`. F# Compiler Service (FCS) hosts (Ionide and friends) opt in via `FSharpProjectOptions.OtherOptions`. F# Interactive (FSI) is not targeted by this RFC; multi-file `#load` semantics are untouched.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placing files in a *rec" group may mess with the initialization semantics for let (and static let) bindings in the files. This needs real care to make sure it isn't possible to ever see uninitialized let values in F#


Files in an SCC of size > 1 are wrapped into one `ParsedImplFileInput` whose top-level `SynModuleOrNamespace` entries get `isRecursive = true`, effectively a `namespace rec` covering the original modules. This is what makes cross-file `type Tree ↔ Forest` compile without `and`-chains. `open` declarations inside the synthesised block are hoisted to the front of each module/namespace (the FS3200 fix). Cross-namespace cycle groups fall back to original order to avoid FS0247 (a synthesised `module Y` inside `namespace rec X` would conflict with the original `namespace X.Y`).

FCS does not synthesise cycle groups in this RFC. IDE diagnostics for cycle-heavy projects show the cycle as a normal type error; the build path resolves it. This is called out in the migration docs. Adding FCS-level cycle synthesis is a follow-on RFC because it requires designing incremental graph invalidation.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realistically FCS must be part of this RFC - we can't have language features that don't have good tooling.

@bryancostanich
Copy link
Copy Markdown
Author

@dsyme fantastic points, i'll chew and get back to it!

@auduchinok
Copy link
Copy Markdown
Contributor

I'd like to second that FCS scenarios should be considered here. Moreover, thinking about how users use the IDE may change the implementation drastically.

When building a project using the compiler, the file order is not expected to change, so it sounds like calculating it based on the graph is sufficient. However, when working on a project, it's actually expected that the ordering will be changing, so I'd think about editing a project as something more important than the actual building.

Consider a case when files have to be reordered due to a refactoring made by a user. How would FCS handle that? Should a user trigger reordering in their IDE explicitly somehow? It means the tools will need to implement it too. Alternatively, FCS could do it automatically, but it sounds too unpredictable and may also worsen the performance of the IDE (due to how caches would be thrown away on the file order change). This has to be thought-through.

What if there's no correct order during a massive refactoring? How can we handle that? What if there's circular dependency? What if cutting a type during a refactoring changes the order in a way that other things resolve differently?

Comment on lines +54 to +56
### `and` keyword deprecation (FS3887)

When `--file-order-auto+` is set, `and`-joined type chains emit warning FS3887 ("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."). Suppressable via `--nowarn:3887` or `<NoWarn>3887</NoWarn>`. Silent in manual mode. The `and` keyword itself is not deprecated globally; only its use as a workaround for cross-file ordering becomes redundant.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should not be part of this design suggestion. and is a often keyword in real code due to how the language works, and changes to how fsproj files are handled should no be related to it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and is really a safety feature to explicitly control where recursion can or a cannot happen, just like rec is for the individual case.
Even though there is merit in making fsproj management easier, I believe it should not lead to a reduction of safety.


### Cycle group synthesis (build path only)

Files in an SCC of size > 1 are wrapped into one `ParsedImplFileInput` whose top-level `SynModuleOrNamespace` entries get `isRecursive = true`, effectively a `namespace rec` covering the original modules. This is what makes cross-file `type Tree ↔ Forest` compile without `and`-chains. `open` declarations inside the synthesised block are hoisted to the front of each module/namespace (the FS3200 fix). Cross-namespace cycle groups fall back to original order to avoid FS0247 (a synthesised `module Y` inside `namespace rec X` would conflict with the original `namespace X.Y`).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding automatic isRecursive=true drops a safety feature of the language (preventing ACCIDENTAL recursions, leading to either inifinite loops or stackoverflows).
I think this idea is already too opinionated on its own, I would not recommend adding another dimension (i.e. dropping something seen as a feature by users) to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants