Skip to content

Numeric format strings silently ignoring unsupported values or producing wrong results #4046

Open

Description

@kerams
let x = 100000
x.ToString "#,#" |> printfn "%s"
x.ToString "N0" |> printfn "%s"
printfn $"{x:N0}"

In fsi, this prints 3 identical lines based on the current culture.

In Fable, the result is

100,000
100000
%P(N0)
  • x.ToString "#,#" produces the correct string, but I'd appreciate a Fable warning saying that invariant culture will be used (I think I've seen something like that in other places).
  • x.ToString "N0" silently produces an incorrect string. An unsupported format error is warranted in my opinion.
  • $"{x:N0}" creates the compiled representation of the interpolation string, which is complete nonsense. An unsupported format error is warranted in my opinion.
  • Also $"{1000:``#,#``} items" results in %P(#,#) items.

Activity

  1. MangelMaxime commented on Feb 25, 2025

    @MangelMaxime
    Member

    x.ToString "#,#" produces the correct string, but I'd appreciate a Fable warning saying that invariant culture will be used (I think I've seen something like that in other places).

    I didn't know Fable was doing that in some cases, but I found this example:

    TimeSpan.Zero.ToString("g") |> ignore

    Will make Fable compilation fails with this error:

    FABLE: TimeSpan.ToString with one argument is not supported, because it depends of local culture, please add CultureInfo.InvariantCulture
    

    @kerams Is it what you are referring too?

  2. kerams commented on Feb 25, 2025

    @kerams
    ContributorAuthor

    I think so. Guess it's not a warning but an error.

  3. MangelMaxime commented on Feb 25, 2025

    @MangelMaxime
    Member

    The warning one that I know about is something like if you pass a CultureInfo parameter Fable says, that it will just be ignored.

    I am just discovering the error one today thanks to you.

  4. MangelMaxime commented on Feb 25, 2025

    @MangelMaxime
    Member

    I am looking at fixing this issue and I am able to fix the first 2 issues.

    Looking more into these issues, I have the feeling that we are having a different behavior / expectation depending on what API we are calling.

    If we call (5592405).ToString("x") or TimeSpan.ToString("g"), we expect to generate an error asking the user to provide CultureInfo.InvariantCulture.

    But if we call (5592405).ToString() or TimeSpan.ToString(), we don't generate the error even if this API is culture dependent (I believe). Perhaps, we don't generate the error for keeping the user code simpler as he doesn't need to do TimeSpan.ToString(CultureInfo.InvariantCulture) everywhere and we state on the docs that Fable is behaving as if CultureInfo.InvariantCulture was passed globally.

    Finally, if we call DateTime.Now.ToString("O") then Fable don't generate the error, perhaps we should?

    I think it would be nice to coming up with a convention and try to apply it everywhere. To do so we need to decide what is the balance we want?

    1. Do we want to enforce the same behavior between .NET and Fable? And prevent potential differences?

      Then we probably want to force the user to pass CultureInfo.InvariantCulture. Do we want to force it only when using custom format or all the time?

    2. Do we prefer a lighter API, and consider that CultureInfo.InvariantCulture is enforced globally ?

      Then we don't want to generate errors when not passing CultureInfo.InvariantCulture

    @alfonsogarciacaro @ncave

  5. MangelMaxime commented on Feb 25, 2025

    @MangelMaxime
    Member

    For reference, while we wait for a decision to be taken.

    To generate an error if no CultureInfo is passed for number formatting here is the code that can be user:

        | "ToString", [ ExprTypeAs(String, _) ] ->
            $"%s{i.DeclaringEntityFullName}.ToString with one argument is not supported, because it depends on local culture, please add CultureInfo.InvariantCulture"
            |> addError com ctx.InlinePath r
    
            None
        | "ToString", [ ExprTypeAs(String, format); _culture ] ->
            let format = emitExpr r String [ format ] "'{0:' + $0 + '}'"
    
            Helper.LibCall(
                com,
                "String",
                "format",
                t,
                [ format; thisArg.Value ],
                [ format.Type; thisArg.Value.Type ],
                ?loc = r
            )
            |> Some

    To be applied here:

    | "ToString", [ ExprTypeAs(String, format) ] ->
    let format = emitExpr r String [ format ] "'{0:' + $0 + '}'"
    Helper.LibCall(
    com,
    "String",
    "format",
    t,
    [ format; thisArg.Value ],
    [ format.Type; thisArg.Value.Type ],
    ?loc = r
    )
    |> Some

  6. ncave commented on Feb 26, 2025

    @ncave
    Collaborator

    @MangelMaxime Considering our limited resources, IMO all we can realistically shoot for is supporting CultureInfo.InvariantCulture. I don't think we should be throwing errors, but warnings are fine. We should be able to produce warnings on unsupported formats when the format is a const string, and otherwise just silently do the best rendering we can do for that unsupported format. Again, this is just IMO.

  7. kerams commented on Feb 26, 2025

    @kerams
    ContributorAuthor

    Does Fable respect TreatWarningsAsErrors? I don't think it does, in which case a warning for an usupported format is insufficient for me. I really don't want it to go unnoticed.

  8. MangelMaxime commented on Feb 26, 2025

    @MangelMaxime
    Member

    Does Fable respect TreatWarningsAsErrors?

    No it doesn't but this is perhaps possible to support it.

    We can probably retrieve the value of TreatWarningsAsErrors when cracking the projects and then when calling addWarn we check the value of TreatWarningsAsErrors and instead generate an error or a warning and error I don't remember the behavior in .NET. The idea is to have a simple implementation IHMO.

    Considering our limited resources, IMO all we can realistically shoot for is supporting CultureInfo.InvariantCulture.

    I fully agree with that.

    Which leads me to think that things like:

    FABLE: TimeSpan.ToString with one argument is not supported, because it depends of local culture, please add CultureInfo.InvariantCulture
    

    should not be generated by Fable.

    We should allow user to write myTimeSpan.ToString("g"), just like he can write myDateTime.ToString("yyyyy") which generates an output equivalent to myDateTime.ToString("yyyyy", CultureInfo.InvariantCulture) in .NET or if CultureInfo.InvariantCulture is set at the assembly level.

  9. MangelMaxime commented on Feb 26, 2025

    @MangelMaxime
    Member

    Just want to add that if a format is not supported, then generating an error independently of TreatWarningsAsErrors should be the expected behavior.

    This is perhaps what you were was referring to @kerams.

    The discussion became messy because we have 2 topics happening at once in this issue I think. Sorry for that

  10. kerams commented on Feb 26, 2025

    @kerams
    ContributorAuthor

    myDateTime.ToString("yyyyy") which generates an output equivalent to myDateTime.ToString("yyyyy", CultureInfo.InvariantCulture) in .NET

    In .NET, CurrentCulture, which is not a thing in Fable, is used by default. The warning you referenced thus points out a difference in behavior between the platforms, but on the other hand, not all format strings are culture-dependent, and I get that having to specify InvariantCulture everywhere would be annoying.

  11. MangelMaxime commented on Feb 27, 2025

    @MangelMaxime
    Member

    In .NET, CurrentCulture, which is not a thing in Fable, is used by default. The warning you referenced thus points out a difference in behavior between the platforms, but on the other hand, not all format strings are culture-dependent, and I get that having to specify InvariantCulture everywhere would be annoying.

    Right for CurrentCulture and Fable always considered it to be set to InvariantCulture because we don't support globalization.

    I will make sure to check that the documentation mention this.

  12. MangelMaxime commented on Feb 27, 2025

    @MangelMaxime
    Member

    Looking back at this issue, I think we are trying to do too much.

    What I mean by that is the behavior in .NET when passing an invalid format is not to generate a compile time error but instead an exception at runtime.

    Image

    Furthermore trying to detect the format provided by the user is not 100% reliable for example:

    let myFormat (cond : bool) =
        if cond then
            "D"
        else
            "B"
    
    (10).ToString(myFormat true) |> printfn "%A"

    In this code, trying to find "D" from at the replacement call is not easy because this is not a direct string but instead a function.

  13. ncave commented on Mar 1, 2025

    @ncave
    Collaborator

    @MangelMaxime

    Furthermore trying to detect the format provided by the user is not 100% reliable

    Yes, we can only examine the .ToString(format) argument and issue an unsupported warning if the format is a const string argument supplied directly (which it is most of the time).

    Still, that covers the majority of the cases. My only point against a runtime error on unsupported formats was that a working F# code (on .NET) will be throwing a hard error on Fable, personally I would rather have some value back (and a compile-time warning) vs a hard runtime error from a code that works in .NET. But that's a personal preference, of course, so I understand other people might prefer the hard error.

    For the (minority of) cases where the format is not a string const and we cannot examine whether we can support it or not, we can just issue a general warning that we only support certain formats. This way we can have 100% "warning" coverage. As long as we have the warning, whether we throw an error on unsupported formats, or just the best value we can, is a matter of preference.

  14. added a commit that references this issue on Apr 22, 2026
    4f3a4ff
  15. github-actions commented on Apr 22, 2026

    @github-actions
    Contributor

    🤖 This is an automated response from Repo Assist.

    I've investigated items 3 and 4 from the original report ($"{x:N0}" → literal %P(N0), and $"{n:#,#} items" → literal %P(#,#) items) and traced the root cause:

    Root cause: The F# compiler encodes a .NET format specifier in an interpolated string as %P(N0) in the PrintFormat format string. The regex in makeStringTemplateFromWith was %P\(\) (empty parens only), so %P(N0) was silently treated as a hole-free literal — the actual value was never interpolated.

    I've opened a fix in PR #→ see Repo Assist PR for fix that:

    1. Updates the regex to %P\(([^)]*)\) to capture the .NET specifier
    2. Wires it into makeStringTemplateFromAllowingFormat to call String.format("{0:N0}", value) — the same path used by (x).ToString("N0")

    Tests for N0, N2, F2, and #,# are included. CI will validate.

    Generated by 🌈 Repo Assist, see workflow run. Learn more.

    To install this agentic workflow, run

    gh aw add githubnext/agentics/workflows/repo-assist.md@51c8f6ad4357d2ecc06e47120031b3d75e80227d
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions