diff --git a/src/fable-library-py/fable_library/List.fs b/src/fable-library-py/fable_library/List.fs index feebfcfa5..509cf2d12 100644 --- a/src/fable-library-py/fable_library/List.fs +++ b/src/fable-library-py/fable_library/List.fs @@ -69,8 +69,7 @@ type LinkedList<'T> = loop 0 xs - override xs.ToString() = - "[" + System.String.Join("; ", xs) + "]" + override xs.ToString() = structuredCollectionToString "[" xs "]" override xs.Equals(other: obj) = if obj.ReferenceEquals(xs, other) then diff --git a/src/fable-library-py/fable_library/Native.fs b/src/fable-library-py/fable_library/Native.fs index 939e65492..7b67f2e35 100644 --- a/src/fable-library-py/fable_library/Native.fs +++ b/src/fable-library-py/fable_library/Native.fs @@ -12,6 +12,33 @@ type Cons<'T>(arrayType: string) = [] member _.Allocate(len: int) = nativeOnly +/// Renders a collection the way F#'s structured formatting (%A) does, quoting +/// string elements: e.g. `structuredCollectionToString "[" xs "]"` -> `["a"; "b"]` +/// +/// Notes: +/// `inline` and the manual `for` loop (rather than `Seq.map`) are deliberate: +/// they keep the generated code free of extra module imports, which otherwise +/// caused a circular import (`list` <-> `seq_native`). +let inline structuredCollectionToString (opening: string) (xs: 'T seq) (closing: string) : string = + let mutable result = opening + let mutable first = true + + let toStringQuoted (x: 'T) : string = + match box x with + | :? string as s -> "\"" + s + "\"" + | _ -> string x + + for x in xs do + result <- + (if first then + result + else + result + "; ") + + toStringQuoted x + + first <- false + + result + closing module Helpers = let arrayFrom (xs: 'T seq) : 'T[] = Array.ofSeq xs diff --git a/src/fable-library-py/fable_library/Set.fs b/src/fable-library-py/fable_library/Set.fs index 82b3147f5..c25626a5f 100644 --- a/src/fable-library-py/fable_library/Set.fs +++ b/src/fable-library-py/fable_library/Set.fs @@ -934,7 +934,7 @@ type Set<[] 'T when 'T: comparison>(comparer: IComparer<' // Set(comparer, SetTree.ofArray comparer arr) override this.ToString() = - "set [" + System.String.Join("; ", this) + "]" + structuredCollectionToString "set [" this "]" // [] // [] diff --git a/src/fable-library-py/fable_library/record.py b/src/fable-library-py/fable_library/record.py index d43d0f0b5..49d33e7ec 100644 --- a/src/fable-library-py/fable_library/record.py +++ b/src/fable-library-py/fable_library/record.py @@ -71,11 +71,22 @@ def record_equals[T](self: T, other: T) -> bool: return self == other +def _field_to_string(value: object) -> str: + # F#'s structured formatting quotes strings, e.g. `{ Name = "John" }`. + if isinstance(value, str): + return f'"{value}"' + return str(value) + + def record_to_string(self: Record) -> str: if hasattr(self, "__slots__"): - return "{ " + "\n ".join(map(lambda slot: slot + " = " + str(getattr(self, slot)), self.__slots__)) + " }" + return ( + "{ " + + "\n ".join(map(lambda slot: slot + " = " + _field_to_string(getattr(self, slot)), self.__slots__)) + + " }" + ) else: - return "{ " + "\n ".join(map(lambda kv: kv[0] + " = " + str(kv[1]), self.__dict__.items())) + " }" + return "{ " + "\n ".join(map(lambda kv: kv[0] + " = " + _field_to_string(kv[1]), self.__dict__.items())) + " }" def record_get_hashcode(self: Record) -> int: diff --git a/src/fable-library-py/src/strings.rs b/src/fable-library-py/src/strings.rs index ae9ebffbb..e1b17dd71 100644 --- a/src/fable-library-py/src/strings.rs +++ b/src/fable-library-py/src/strings.rs @@ -27,7 +27,7 @@ use crate::array::FSharpArray; use pyo3::prelude::*; -use pyo3::types::PyAny; +use pyo3::types::{PyAny, PyString}; use regex::Regex; use std::sync::LazyLock; @@ -113,6 +113,10 @@ mod printf { input: String, /// Accumulated arguments with type annotations args: Vec, + /// Parallel to `args`: whether each argument was originally a Python `str`. + /// Used so that `%A` (structured formatting) can quote bare strings while + /// `%s`/`%O` leave them unquoted. + arg_is_str: Vec, /// Optional continuation function to apply to the final formatted string continuation: Option>, /// Cached count of format placeholders (computed once at creation) @@ -124,6 +128,7 @@ mod printf { Python::attach(|py| Self { input: self.input.clone(), args: self.args.clone(), + arg_is_str: self.arg_is_str.clone(), continuation: self.continuation.as_ref().map(|c| c.clone_ref(py)), placeholder_count: self.placeholder_count, }) @@ -138,6 +143,7 @@ mod printf { Self { input, args: Vec::new(), + arg_is_str: Vec::new(), continuation: None, placeholder_count, } @@ -172,6 +178,9 @@ mod printf { } }; + // Remember whether the original argument was a string so that `%A` + // can quote it (structured formatting) while `%s`/`%O` do not. + new_format.arg_is_str.push(arg.is_instance_of::()); new_format.args.push(arg_str); // Use cached placeholder count instead of recomputing @@ -274,9 +283,10 @@ mod printf { // Parse type information if present (only for known type annotations) let (value_str, type_info) = parse_type_annotation(arg_value); + let is_str = self.arg_is_str.get(arg_index).copied().unwrap_or(false); let formatted_value = - format_value(value_str, type_info, format_type, flags, precision)?; + format_value(value_str, type_info, format_type, flags, precision, is_str)?; // Apply padding if width is specified let final_value = apply_padding(&formatted_value, width, flags); @@ -286,7 +296,8 @@ mod printf { } // Handle remaining simple patterns that the regex might have missed - handle_remaining_patterns(&mut result, &self.args[arg_index..]); + let remaining_is_str: &[bool] = self.arg_is_str.get(arg_index..).unwrap_or(&[]); + handle_remaining_patterns(&mut result, &self.args[arg_index..], remaining_is_str); // Handle %% escape sequences at the end result = result.replace("%%", "%"); @@ -356,6 +367,7 @@ mod printf { format_type: &str, flags: &str, precision: Option, + is_str: bool, ) -> PyResult { match format_type { "d" | "i" => format_integer(value_str, flags), @@ -363,6 +375,10 @@ mod printf { "g" | "G" => format_general(value_str, flags), "x" => format_hex_lower(value_str, type_info), "X" => format_hex_upper(value_str, type_info), + // `%A` uses structured formatting, which quotes a bare string + // (e.g. `"test"`). Containers already quote their nested strings in + // their own `__str__`, so this only affects the top-level value. + "A" if is_str => Ok(format!("\"{value_str}\"")), "s" | "A" => Ok(value_str.to_string()), "O" => Ok(value_str.to_string()), // Object display "b" => format_boolean(value_str), @@ -552,8 +568,8 @@ mod printf { } /// Handle remaining simple patterns - fn handle_remaining_patterns(result: &mut String, remaining_args: &[String]) { - for arg in remaining_args { + fn handle_remaining_patterns(result: &mut String, remaining_args: &[String], remaining_is_str: &[bool]) { + for (i, arg) in remaining_args.iter().enumerate() { if let Some(pos) = result.find("%s") { result.replace_range(pos..pos + 2, arg); } else if let Some(pos) = result.find("%d") { @@ -565,7 +581,12 @@ mod printf { } else if let Some(pos) = result.find("%i") { result.replace_range(pos..pos + 2, arg); } else if let Some(pos) = result.find("%A") { - result.replace_range(pos..pos + 2, arg); + let is_str = remaining_is_str.get(i).copied().unwrap_or(false); + if is_str { + result.replace_range(pos..pos + 2, &format!("\"{arg}\"")); + } else { + result.replace_range(pos..pos + 2, arg); + } } else { break; } diff --git a/src/fable-library-ts/List.fs b/src/fable-library-ts/List.fs index 38440743d..e41e9724c 100644 --- a/src/fable-library-ts/List.fs +++ b/src/fable-library-ts/List.fs @@ -69,8 +69,7 @@ type LinkedList<'T> = loop 0 xs - override xs.ToString() = - "[" + System.String.Join("; ", xs) + "]" + override xs.ToString() = structuredCollectionToString "[" xs "]" override xs.Equals(other: obj) = if obj.ReferenceEquals(xs, other) then diff --git a/src/fable-library-ts/Map.fs b/src/fable-library-ts/Map.fs index 2a040fe3c..d6741543f 100644 --- a/src/fable-library-ts/Map.fs +++ b/src/fable-library-ts/Map.fs @@ -956,6 +956,8 @@ type Map<[] 'Key, [ Seq.iter (fun p -> f p.Value p.Key m) override this.ToString() = + // Note: .NET's FSharpMap.ToString renders keys/values with `%O` (not `%A`), + // so strings are intentionally *not* quoted here (e.g. `map [(Age, 12)]`). let inline toStr (kv: KeyValuePair<'Key, 'Value>) = System.String.Format("({0}, {1})", kv.Key, kv.Value) diff --git a/src/fable-library-ts/Native.fs b/src/fable-library-ts/Native.fs index 384919e80..87acd9b54 100644 --- a/src/fable-library-ts/Native.fs +++ b/src/fable-library-ts/Native.fs @@ -12,6 +12,34 @@ type Cons<'T> = [] abstract Allocate: len: int -> 'T[] +/// Renders a collection the way F#'s structured formatting (%A) does, quoting +/// string elements: e.g. `structuredCollectionToString "[" xs "]"` -> `["a"; "b"]` +/// +/// Notes: +/// `inline` and the manual `for` loop (rather than `Seq.map`) are deliberate: +/// they keep the generated code free of extra module imports, which otherwise +/// caused a circular import (`list` <-> `seq_native`). +let inline structuredCollectionToString (opening: string) (xs: 'T seq) (closing: string) : string = + let mutable result = opening + let mutable first = true + + let toStringQuoted (x: 'T) : string = + match box x with + | :? string as s -> "\"" + s + "\"" + | _ -> string x + + for x in xs do + result <- + (if first then + result + else + result + "; ") + + toStringQuoted x + + first <- false + + result + closing + module Helpers = [] let inline internal asArray (a: ResizeArray<'T>) : 'T[] = nativeOnly diff --git a/src/fable-library-ts/Set.fs b/src/fable-library-ts/Set.fs index 4376dbce9..a96e6c36e 100644 --- a/src/fable-library-ts/Set.fs +++ b/src/fable-library-ts/Set.fs @@ -951,7 +951,7 @@ type Set<[] 'T when 'T: comparison>(comparer: IComparer<' // Set(comparer, SetTree.ofArray comparer arr) override this.ToString() = - "set [" + System.String.Join("; ", this) + "]" + structuredCollectionToString "set [" this "]" // [] // [] diff --git a/src/fable-library-ts/String.ts b/src/fable-library-ts/String.ts index 997324875..5b2fe5483 100644 --- a/src/fable-library-ts/String.ts +++ b/src/fable-library-ts/String.ts @@ -253,6 +253,8 @@ function formatReplacement(rep: any, flags: any, padLength: any, precision: any, } } else if (rep instanceof Date) { rep = dateToString(rep); + } else if (format === "A" && typeof rep === "string") { + rep = "\"" + rep + "\""; } else { rep = toString(rep); } diff --git a/src/fable-library-ts/Types.ts b/src/fable-library-ts/Types.ts index 2f54e71f6..2707bf65b 100644 --- a/src/fable-library-ts/Types.ts +++ b/src/fable-library-ts/Types.ts @@ -9,18 +9,25 @@ export function seqToString(self: Iterable): string { let str = "["; for (const x of self) { if (count === 0) { - str += toString(x); + str += toStringQuoted(x); } else if (count === 100) { str += "; ..."; break; } else { - str += "; " + toString(x); + str += "; " + toStringQuoted(x); } count++; } return str + "]"; } +// Structured (%A-style) rendering of a value used as an element/field of a +// container. F#'s structured formatting (which records, unions, lists, etc. +// use in their ToString) quotes strings, e.g. `["a"; "b"]` and `{ Name = "John" }`. +function toStringQuoted(x: any, callStack = 0): string { + return typeof x === "string" ? "\"" + x + "\"" : toString(x, callStack); +} + export function toString(x: any, callStack = 0): string { if (x != null && typeof x === "object") { if (typeof x.toString === "function" && x.toString !== Object.prototype.toString) { @@ -31,7 +38,7 @@ export function toString(x: any, callStack = 0): string { const cons = Object.getPrototypeOf(x)?.constructor; return cons === Object && callStack < 10 // Same format as recordToString - ? "{ " + Object.entries(x).map(([k, v]) => k + " = " + toString(v, callStack + 1)).join("\n ") + " }" + ? "{ " + Object.entries(x).map(([k, v]) => k + " = " + toStringQuoted(v, callStack + 1)).join("\n ") + " }" : cons?.name ?? ""; } } @@ -39,23 +46,16 @@ export function toString(x: any, callStack = 0): string { } export function unionToString(name: string, fields: any[]) { - function unionFieldToString(x: any): string { - if (typeof x === "string") { - return '"' + x + '"'; - } - return toString(x); - } - if (fields.length === 0) { return name; } else { let fieldStr; let withParens = true; if (fields.length === 1) { - fieldStr = unionFieldToString(fields[0]); + fieldStr = toStringQuoted(fields[0]); withParens = fieldStr.indexOf(" ") >= 0; } else { - fieldStr = fields.map((x: any) => unionFieldToString(x)).join(", "); + fieldStr = fields.map((x: any) => toStringQuoted(x)).join(", "); } return name + (withParens ? " (" : " ") + fieldStr + (withParens ? ")" : ""); } @@ -119,7 +119,7 @@ function recordToJSON(self: T) { } function recordToString(self: T) { - return "{ " + Object.entries(self as any).map(([k, v]) => k + " = " + toString(v)).join("\n ") + " }"; + return "{ " + Object.entries(self as any).map(([k, v]) => k + " = " + toStringQuoted(v)).join("\n ") + " }"; } function recordGetHashCode(self: T) { diff --git a/tests/Js/Main/StringTests.fs b/tests/Js/Main/StringTests.fs index 591443b79..5227bbb77 100644 --- a/tests/Js/Main/StringTests.fs +++ b/tests/Js/Main/StringTests.fs @@ -281,6 +281,25 @@ let tests = testList "Strings" [ testCase "Fix #2398: Exception when two successive string format placeholders and value of first one ends in `%`" <| fun () -> sprintf "%c%s" '%' "text" |> equal "%text" + // See https://github.com/fable-compiler/Fable/issues/4732 + testCase "sprintf \"%A\" quotes strings" <| fun () -> + sprintf "%A" "test" |> equal "\"test\"" + sprintf "%A" "" |> equal "\"\"" + // %O and %s don't quote a bare string + sprintf "%O" "test" |> equal "test" + sprintf "%s" "test" |> equal "test" + + // See https://github.com/fable-compiler/Fable/issues/4732 + testCase "sprintf \"%A\" wraps strings without escaping" <| fun () -> + // F#'s %A only wraps a string in quotes; it does NOT escape embedded + // double-quotes, backslashes, or control characters (verified on .NET). + sprintf "%A" "a\"b" |> equal "\"a\"b\"" + sprintf "%A" "a\\b" |> equal "\"a\\b\"" + sprintf "%A" "a\nb" |> equal "\"a\nb\"" + sprintf "%A" "a\tb" |> equal "\"a\tb\"" + // Same behavior when the string is nested inside a container + sprintf "%A" [ "a\"b" ] |> equal "[\"a\"b\"]" + testCase "Unions with sprintf %A" <| fun () -> Bar(1,5) |> sprintf "%A" |> equal "Bar (1, 5)" Foo1 4.5 |> sprintf "%A" |> equal "Foo1 4.5" @@ -350,8 +369,8 @@ let tests = testList "Strings" [ testCase "Printf %A works with anonymous records" <| fun () -> // See #4029 let person = {| FirstName = "John"; LastName = "Doe" |} let s = sprintf "%A" person - System.Text.RegularExpressions.Regex.Replace(s.Replace("\"", ""), @"\s+", " ") - |> equal """{ FirstName = John LastName = Doe }""" + System.Text.RegularExpressions.Regex.Replace(s, @"\s+", " ") + |> equal """{ FirstName = "John" LastName = "Doe" }""" testCase "Interpolated strings keep empty lines" <| fun () -> let s1 = $"""1 @@ -403,11 +422,11 @@ let tests = testList "Strings" [ testCase "sprintf \"%A\" with lists works" <| fun () -> let xs = ["Hi"; "Hello"; "Hola"] - (sprintf "%A" xs).Replace("\"", "") |> equal "[Hi; Hello; Hola]" + sprintf "%A" xs |> equal "[\"Hi\"; \"Hello\"; \"Hola\"]" testCase "sprintf \"%A\" with nested lists works" <| fun () -> let xs = [["Hi"]; ["Hello"]; ["Hola"]] - (sprintf "%A" xs).Replace("\"", "") |> equal "[[Hi]; [Hello]; [Hola]]" + sprintf "%A" xs |> equal "[[\"Hi\"]; [\"Hello\"]; [\"Hola\"]]" testCase "sprintf \"%A\" with sequences works" <| fun () -> let xs = seq { "Hi"; "Hello"; "Hola" } diff --git a/tests/Python/TestString.fs b/tests/Python/TestString.fs index a8e09b0ab..41876f37d 100644 --- a/tests/Python/TestString.fs +++ b/tests/Python/TestString.fs @@ -158,15 +158,36 @@ let ``test sprintf \"%A\" formats booleans as lowercase`` () = let optResult = sprintf "Some(%A)" true equal "Some(true)" optResult +// See https://github.com/fable-compiler/Fable/issues/4732 +[] +let ``test sprintf \"%A\" quotes strings`` () = + sprintf "%A" "test" |> equal "\"test\"" + sprintf "%A" "" |> equal "\"\"" + // %O and %s don't quote a bare string + sprintf "%O" "test" |> equal "test" + sprintf "%s" "test" |> equal "test" + +// See https://github.com/fable-compiler/Fable/issues/4732 +[] +let ``test sprintf \"%A\" wraps strings without escaping`` () = + // F#'s %A only wraps a string in quotes; it does NOT escape embedded + // double-quotes, backslashes, or control characters (verified on .NET). + sprintf "%A" "a\"b" |> equal "\"a\"b\"" + sprintf "%A" "a\\b" |> equal "\"a\\b\"" + sprintf "%A" "a\nb" |> equal "\"a\nb\"" + sprintf "%A" "a\tb" |> equal "\"a\tb\"" + // Same behavior when the string is nested inside a container + sprintf "%A" [ "a\"b" ] |> equal "[\"a\"b\"]" + [] let ``test sprintf \"%A\" with lists works`` () = let xs = ["Hi"; "Hello"; "Hola"] - (sprintf "%A" xs).Replace("\"", "") |> equal "[Hi; Hello; Hola]" + sprintf "%A" xs |> equal "[\"Hi\"; \"Hello\"; \"Hola\"]" [] let ``test sprintf \"%A\" with nested lists works`` () = let xs = [["Hi"]; ["Hello"]; ["Hola"]] - (sprintf "%A" xs).Replace("\"", "") |> equal "[[Hi]; [Hello]; [Hola]]" + sprintf "%A" xs |> equal "[[\"Hi\"]; [\"Hello\"]; [\"Hola\"]]" [] let ``test sprintf \"%A\" with sequences works`` () =