You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Numeric format strings silently ignoring unsupported values or producing wrong results #4046
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.
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
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?
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?
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
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 ]->letformat= emitExpr r String [ format ]"'{0:' + $0 + '}'"
Helper.LibCall(
com,"String","format",
t,[ format; thisArg.Value ],[ format.Type; thisArg.Value.Type ],
?loc = r
)|> Some
@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.
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.
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.
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.
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.
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.
Furthermore trying to detect the format provided by the user is not 100% reliable for example:
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.
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.
Activity
MangelMaxime commentedon Feb 25, 2025
I didn't know Fable was doing that in some cases, but I found this example:
Will make Fable compilation fails with this error:
@kerams Is it what you are referring too?
kerams commentedon Feb 25, 2025
I think so. Guess it's not a warning but an error.
MangelMaxime commentedon Feb 25, 2025
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.
MangelMaxime commentedon Feb 25, 2025
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")orTimeSpan.ToString("g"), we expect to generate an error asking the user to provideCultureInfo.InvariantCulture.But if we call
(5592405).ToString()orTimeSpan.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 doTimeSpan.ToString(CultureInfo.InvariantCulture)everywhere and we state on the docs that Fable is behaving as ifCultureInfo.InvariantCulturewas 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?
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?Do we prefer a lighter API, and consider that
CultureInfo.InvariantCultureis enforced globally ?Then we don't want to generate errors when not passing
CultureInfo.InvariantCulture@alfonsogarciacaro @ncave
MangelMaxime commentedon Feb 25, 2025
For reference, while we wait for a decision to be taken.
To generate an error if no
CultureInfois passed for number formatting here is the code that can be user:To be applied here:
Fable/src/Fable.Transforms/Replacements.fs
Lines 2499 to 2511 in 8273959
ncave commentedon Feb 26, 2025
@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.kerams commentedon Feb 26, 2025
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.MangelMaxime commentedon Feb 26, 2025
No it doesn't but this is perhaps possible to support it.
We can probably retrieve the value of
TreatWarningsAsErrorswhen cracking the projects and then when callingaddWarnwe check the value ofTreatWarningsAsErrorsand 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.I fully agree with that.
Which leads me to think that things like:
should not be generated by Fable.
We should allow user to write
myTimeSpan.ToString("g"), just like he can writemyDateTime.ToString("yyyyy")which generates an output equivalent tomyDateTime.ToString("yyyyy", CultureInfo.InvariantCulture)in .NET or ifCultureInfo.InvariantCultureis set at the assembly level.MangelMaxime commentedon Feb 26, 2025
Just want to add that if a format is not supported, then generating an error independently of
TreatWarningsAsErrorsshould 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
kerams commentedon Feb 26, 2025
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 specifyInvariantCultureeverywhere would be annoying.MangelMaxime commentedon Feb 27, 2025
Right for
CurrentCultureand Fable always considered it to be set toInvariantCulturebecause we don't support globalization.I will make sure to check that the documentation mention this.
MangelMaxime commentedon Feb 27, 2025
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.
Furthermore trying to detect the format provided by the user is not 100% reliable for example:
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.ncave commentedon Mar 1, 2025
@MangelMaxime
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.
fix(js): handle .NET format specifiers in F# interpolated strings
github-actions commentedon Apr 22, 2026
🤖 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
.NETformat specifier in an interpolated string as%P(N0)in thePrintFormatformat string. The regex inmakeStringTemplateFromWithwas%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:
%P\(([^)]*)\)to capture the.NETspecifiermakeStringTemplateFromAllowingFormatto callString.format("{0:N0}", value)— the same path used by(x).ToString("N0")Tests for
N0,N2,F2, and#,#are included. CI will validate.