From 8f948f8adc5e065803925a47967c0ae77588225d Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 31 Aug 2026 13:24:11 +0200 Subject: [PATCH 1/3] String#encode: fallback #to_str/validation, xml NCR, IBM437, encode! frozen Advance core/string/encode_spec.rb from 106 to 134 passing examples: - :fallback now converts a non-String result via #to_str (never #to_s), rejects a substitute unrepresentable in the target with ArgumentError "too big fallback string", and treats an object without #[] as no fallback (the char falls through to the normal UndefinedConversionError). - xml: :text / :attr render a character absent from the destination as an upper-case numeric character reference (&#xHH;) instead of raising. - IBM437 gains a codec (charmap.CodePage437). - String#encode! raises FrozenError on a frozen receiver, even for a no-op. Co-Authored-By: Claude Opus 4.8 --- internal/vm/encoding_encode_test.go | 2 +- internal/vm/string_encode.go | 67 ++++++++++++++++++++++++----- internal/vm/string_encode_xtext.go | 1 + 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/internal/vm/encoding_encode_test.go b/internal/vm/encoding_encode_test.go index 3d3611de..80a6ea43 100644 --- a/internal/vm/encoding_encode_test.go +++ b/internal/vm/encoding_encode_test.go @@ -142,7 +142,7 @@ func TestStringEncodeFallback(t *testing.T) { {`"aéb".encode("US-ASCII", fallback: ->(c){ 42 })`, "no implicit conversion of Integer into String"}, // A target rbgo has no codec for is a named residual: the fallback cannot // mask the missing converter (rbgo raises where MRI would transcode natively). - {`"aéb".encode("IBM437", fallback: {"é"=>"e"})`, "ConverterNotFoundError"}, + {`"aéb".encode("Emacs-Mule", fallback: {"é"=>"e"})`, "ConverterNotFoundError"}, } for _, c := range errCases { if err := runErr(t, c.src); err == nil || !strings.Contains(err.Error(), c.substr) { diff --git a/internal/vm/string_encode.go b/internal/vm/string_encode.go index e8e22a7a..a1451f98 100644 --- a/internal/vm/string_encode.go +++ b/internal/vm/string_encode.go @@ -2,6 +2,7 @@ package vm import ( binpkg "encoding/binary" + "fmt" "strings" "unicode/utf16" "unicode/utf8" @@ -100,6 +101,22 @@ func applyXML(s string, o transcodeOpts) string { return s } +// xmlEscapeUndef replaces every character of the xml-escaped intermediate that +// the target `to` cannot represent with an upper-case hexadecimal numeric +// character reference (&#xHH;) — MRI's xml: :text / :attr rendering of an +// otherwise-undefined character. +func xmlEscapeUndef(s, to string) string { + var b strings.Builder + for _, r := range s { + if charEncodableIn(r, to) { + b.WriteRune(r) + continue + } + fmt.Fprintf(&b, "&#x%X;", r) + } + return b.String() +} + func truthyValue(v object.Value) bool { return v != object.NilV && v != object.Bool(false) } // charEncodableIn reports whether rune r can be represented in encoding `to`. The @@ -127,10 +144,18 @@ func charEncodableIn(r rune, to string) bool { // cannot be represented in the target `to` with the result of the :fallback // option — a Hash, Proc, Method or any object queried with #[], passed the // character as a String. A nil result (a Hash miss included) leaves the character -// in place for the normal undefined-conversion error; a non-String, non-nil -// result raises TypeError, matching MRI. It is called only when undef: :replace is -// off (that option pre-empts the fallback entirely). +// in place for the normal undefined-conversion error; a non-String result is put +// through #to_str (never #to_s), and anything that yields no String raises +// TypeError, matching MRI. A substitute that is itself unrepresentable in the +// target raises ArgumentError "too big fallback string". It is called only when +// undef: :replace is off (that option pre-empts the fallback entirely). func (vm *VM) applyFallback(u, to string, fb object.Value) string { + // An object that cannot be queried with #[] is no fallback at all: the + // unrepresentable character falls through to the normal undefined-conversion + // error (MRI looks for #[], and finding none never substitutes). + if !vm.respondsToDynamic(fb, "[]") { + return u + } var b strings.Builder for _, r := range u { if charEncodableIn(r, to) { @@ -138,16 +163,30 @@ func (vm *VM) applyFallback(u, to string, fb object.Value) string { continue } res := vm.send(fb, "[]", []object.Value{object.NewString(string(r))}, nil) - switch v := res.(type) { - case *object.String: - b.WriteString(v.Str()) - default: - if res == object.NilV { - b.WriteRune(r) - continue + if res == object.NilV { + b.WriteRune(r) + continue + } + s, ok := res.(*object.String) + if !ok { + // MRI converts a non-String result with #to_str, but never #to_s. + if vm.respondsToDynamic(res, "to_str") { + if conv, isStr := vm.send(res, "to_str", nil, nil).(*object.String); isStr { + s, ok = conv, true + } } + } + if !ok { raise("TypeError", "no implicit conversion of %s into String", vm.classOf(res).name) } + // The substitute must itself be representable in the target; MRI rejects one + // that is not with ArgumentError "too big fallback string". + for _, rr := range s.Str() { + if !charEncodableIn(rr, to) { + raise("ArgumentError", "too big fallback string") + } + } + b.WriteString(s.Str()) } return b.String() } @@ -390,6 +429,11 @@ func (vm *VM) stringEncode(s *object.String, args []object.Value) *object.String inter := vm.decodeToUTF8(s.Bytes(), from, opts, repl) inter = applyNewline(inter, opts) inter = applyXML(inter, opts) + // Under xml:, a character absent from the destination is rendered as a numeric + // character reference rather than raising an undefined-conversion error. + if opts.xml != "" { + inter = xmlEscapeUndef(inter, to) + } // The :fallback option supplies substitutes for unrepresentable characters, // but only where undef: :replace does not already handle them — that option // pre-empts the fallback (MRI never consults it once undef substitution is on). @@ -408,6 +452,9 @@ func (vm *VM) registerStringEncodeMethods() { }) vm.cString.define("encode!", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { s := self.(*object.String) + // #encode! mutates in place, so a frozen receiver is a FrozenError — even for + // a no-op transcoding (utf-8 to utf-8), which MRI still refuses. + vm.checkFrozen(s) enc := vm.stringEncode(s, args) s.SetBytes(append([]byte(nil), enc.Bytes()...)) s.Enc = enc.Enc diff --git a/internal/vm/string_encode_xtext.go b/internal/vm/string_encode_xtext.go index b2097ec5..88cd50c4 100644 --- a/internal/vm/string_encode_xtext.go +++ b/internal/vm/string_encode_xtext.go @@ -40,6 +40,7 @@ var xtextEncodings = map[string]encoding.Encoding{ "Windows-1257": charmap.Windows1257, "Windows-1258": charmap.Windows1258, "Windows-874": charmap.Windows874, + "IBM437": charmap.CodePage437, "Windows-31J": japanese.ShiftJIS, "Shift_JIS": japanese.ShiftJIS, "EUC-JP": japanese.EUCJP, From 68f9fe39d2696d121ccf12ad1d328a845cc7b55c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 31 Aug 2026 13:32:57 +0200 Subject: [PATCH 2/3] String#encode: default_internal target, converter-not-found + ASCII shortcut Continue advancing core/string/encode_spec.rb (134 -> 146 passing): - #encode with no destination transcodes to Encoding.default_internal when set. - An unusable destination (an unregistered name, or a dummy encoding like Emacs-Mule) is reported as Encoding::ConverterNotFoundError rather than an unknown-encoding ArgumentError or a premature UndefinedConversionError; a 7-bit ASCII String still transcodes to any ASCII-compatible destination as a byte-identical copy, matching MRI. - Encoding-name arguments resolve through the registry but pass an unknown name through verbatim so the missing-converter path reports it. - Adds internal/vm/encode_residuals_test.go and updates two stale expectations in encoding_encode_test.go to MRI's actual behaviour. Co-Authored-By: Claude Opus 4.8 --- internal/vm/encode_residuals_test.go | 66 ++++++++++++++++++++++++++++ internal/vm/encoding_encode_test.go | 2 +- internal/vm/string_encode.go | 55 ++++++++++++++++++++++- 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 internal/vm/encode_residuals_test.go diff --git a/internal/vm/encode_residuals_test.go b/internal/vm/encode_residuals_test.go new file mode 100644 index 00000000..bf3fb462 --- /dev/null +++ b/internal/vm/encode_residuals_test.go @@ -0,0 +1,66 @@ +package vm_test + +import ( + "strings" + "testing" +) + +// TestEncodeResiduals covers the String#encode conformance gaps closed against +// MRI Ruby 4.0.6: the :fallback #to_str / validation rules, xml: numeric +// character references for undefined characters, the IBM437 codec, transcoding +// to Encoding.default_internal, the ASCII-compatible no-converter shortcut, and +// String#encode!'s FrozenError. Every expectation was cross-checked against MRI. +func TestEncodeResiduals(t *testing.T) { + cases := []struct{ src, want string }{ + // :fallback converts a non-String result through #to_str (never #to_s). + { + `obj = Object.new; def obj.to_str; "bar"; end + print "B�".encode("US-ASCII", fallback: {"�" => obj})`, + "Bbar", + }, + // A #[]-responding object is passed the unrepresentable character. + {`print "B�".encode("US-ASCII", fallback: proc { |c| c.bytes.inspect })`, "B[239, 191, 189]"}, + // xml: :text / :attr render an undefined character as an upper-case NCR. + {`print "ürst".encode("US-ASCII", xml: :text)`, "ürst"}, + {`print "ürst".encode("US-ASCII", xml: :attr)`, `"ürst"`}, + // IBM437 now has a codec, in both directions. + {`print "あ".encode("euc-jp", "ibm437").bytes.inspect`, "[166, 208, 143, 171, 228, 143, 171, 177]"}, + {`print "aéb".encode("IBM437").bytes.inspect`, "[97, 130, 98]"}, + // A 7-bit ASCII String transcodes to a converter-less ASCII-compatible + // destination as a byte-identical copy. + {`s = "\x79".b.encode("Emacs-Mule"); print s; print s.encoding.name`, "yEmacs-Mule"}, + // With no destination, #encode transcodes to Encoding.default_internal. + {`Encoding.default_internal = Encoding::UTF_8 + print [0xA4, 0xA2].pack("CC").force_encoding("EUC-JP").encode + Encoding.default_internal = nil`, "あ"}, + // An encoding argument answering #to_str is honoured. + {`class EncName; def to_str; "utf-8"; end; end + print [0xA4, 0xA2].pack("CC").force_encoding("EUC-JP").encode(EncName.new)`, "あ"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + + errCases := []struct{ src, substr string }{ + // A fallback substitute unrepresentable in the target is rejected. + {`"B�".encode("US-ASCII", fallback: {"�" => "￮"})`, "too big fallback string"}, + {`"B�".encode("US-ASCII", fallback: -> c { "￮" })`, "too big fallback string"}, + // A fallback object with no #[] is no fallback: the normal undefined error. + {`"B�".encode("US-ASCII", fallback: Object.new)`, "U+FFFD from UTF-8 to US-ASCII"}, + // An unregistered destination is a missing converter, not an unknown name. + {`"abc".encode("xyz")`, "code converter not found (UTF-8 to xyz)"}, + // A registered but converter-less destination with non-ASCII data has no + // converter either. + {`[0x80].pack("C").force_encoding("BINARY").encode("Emacs-Mule")`, "code converter not found (ASCII-8BIT to Emacs-Mule)"}, + // #encode! refuses a frozen receiver, even for a no-op transcoding. + {`"foo".freeze.encode!("utf-8")`, "FrozenError"}, + {`"foo".freeze.encode!("euc-jp")`, "FrozenError"}, + } + for _, c := range errCases { + if err := runErr(t, c.src); err == nil || !strings.Contains(err.Error(), c.substr) { + t.Errorf("src=%q err=%v, want containing %q", c.src, err, c.substr) + } + } +} diff --git a/internal/vm/encoding_encode_test.go b/internal/vm/encoding_encode_test.go index 80a6ea43..acd45914 100644 --- a/internal/vm/encoding_encode_test.go +++ b/internal/vm/encoding_encode_test.go @@ -92,7 +92,7 @@ func TestStringEncode(t *testing.T) { {`"\x61\x00\x00".dup.force_encoding("utf-32le").encode("utf-8")`, "InvalidByteSequenceError"}, {`"\x80\x00\x00\x00".dup.force_encoding("utf-32be").encode("utf-8")`, "InvalidByteSequenceError"}, {`"\x00\xd8".dup.force_encoding("utf-16le").encode("utf-8")`, "InvalidByteSequenceError"}, - {`"abc".encode("utf-8", "no-such-enc")`, "unknown encoding name"}, + {`"abc".encode("utf-8", "no-such-enc")`, "code converter not found (no-such-enc to UTF-8)"}, {`"abc".encode("utf-8", xml: :bogus)`, "unexpected value for xml option"}, } for _, c := range errCases { diff --git a/internal/vm/string_encode.go b/internal/vm/string_encode.go index a1451f98..d3cd24db 100644 --- a/internal/vm/string_encode.go +++ b/internal/vm/string_encode.go @@ -401,6 +401,42 @@ func encodeUTF32(u string, be bool) []byte { return b } +// encodeEncName resolves a String#encode positional encoding argument to its +// name. An Encoding is taken as-is; a String (or a #to_str result) is canonicalised +// through the registry, but an unregistered name is returned verbatim rather than +// raising — #encode reports an unusable destination as a missing converter, not as +// the unknown-encoding ArgumentError that Encoding.find would raise. +func (vm *VM) encodeEncName(v object.Value) string { + resolve := func(s string) string { + if e, ok := vm.findEncoding(s); ok { + return e.name + } + return s + } + switch e := v.(type) { + case *encodingObj: + return e.name + case *object.String: + return resolve(e.Str()) + } + if vm.respondsToDynamic(v, "to_str") { + if s, ok := vm.send(v, "to_str", nil, nil).(*object.String); ok { + return resolve(s.Str()) + } + } + raise("TypeError", "no implicit conversion of %s into String", classNameOf(v)) + return "" +} + +// encAsciiCompat reports whether the registered encoding `name` is +// ASCII-compatible; an unregistered name is treated as not compatible. +func (vm *VM) encAsciiCompat(name string) bool { + if e, ok := vm.findEncoding(name); ok { + return e.asciiCompat + } + return false +} + // stringEncode implements String#encode(dst = default, src = self.encoding, **opts). func (vm *VM) stringEncode(s *object.String, args []object.Value) *object.String { var positional []object.Value @@ -415,10 +451,14 @@ func (vm *VM) stringEncode(s *object.String, args []object.Value) *object.String to := s.EncName() from := s.EncName() if len(positional) >= 1 { - to = vm.encodingArg(positional[0]).name + to = vm.encodeEncName(positional[0]) + } else if vm.defInternalEnc != nil { + // With no destination given, #encode transcodes to Encoding.default_internal + // when one is set (otherwise it is a copy in the source encoding). + to = vm.defInternalEnc.name } if len(positional) >= 2 { - from = vm.encodingArg(positional[1]).name + from = vm.encodeEncName(positional[1]) } repl := opts.replace @@ -426,6 +466,17 @@ func (vm *VM) stringEncode(s *object.String, args []object.Value) *object.String repl = defaultReplacement(to) } + // A destination rbgo cannot transcode into (a dummy encoding like Emacs-Mule, + // or an unregistered name) has no converter. MRI still succeeds when the whole + // string is 7-bit ASCII and both encodings are ASCII-compatible — the bytes are + // identical — and otherwise raises Encoding::ConverterNotFoundError. + if !encodingSupported(to) { + if asciiOnly(s.Bytes()) && vm.encAsciiCompat(to) && vm.encAsciiCompat(from) { + return object.NewStringBytesEnc(append([]byte(nil), s.Bytes()...), to) + } + raise("Encoding::ConverterNotFoundError", "code converter not found (%s to %s)", from, to) + } + inter := vm.decodeToUTF8(s.Bytes(), from, opts, repl) inter = applyNewline(inter, opts) inter = applyXML(inter, opts) From 101ed8ffb3366a5c78b34cbf5effdfc6c9e83323 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 31 Aug 2026 13:46:21 +0200 Subject: [PATCH 3/3] String#encode: drop now-unreachable converter-not-found branches; cover residuals The early converter check in stringEncode guarantees a supported destination before decode/encode/fallback run, and caseMapBytes decodes first, so the downstream unsupported-target branches in charEncodableIn, encodeFromUTF8 and xtextEncode became unreachable dead code. Remove them (xtextEncode no longer returns a found bool) and extend encode_residuals_test.go with the TypeError path for a non-convertible encoding argument. Merged coverage back to 100.0%. Co-Authored-By: Claude Opus 4.8 --- internal/vm/encode_residuals_test.go | 4 ++++ internal/vm/string_encode.go | 24 ++++++++++-------------- internal/vm/string_encode_xtext.go | 19 +++++++++---------- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/internal/vm/encode_residuals_test.go b/internal/vm/encode_residuals_test.go index bf3fb462..ae541528 100644 --- a/internal/vm/encode_residuals_test.go +++ b/internal/vm/encode_residuals_test.go @@ -54,6 +54,10 @@ func TestEncodeResiduals(t *testing.T) { // A registered but converter-less destination with non-ASCII data has no // converter either. {`[0x80].pack("C").force_encoding("BINARY").encode("Emacs-Mule")`, "code converter not found (ASCII-8BIT to Emacs-Mule)"}, + // An encoding argument that is neither an Encoding, a String, nor #to_str + // convertible is a TypeError. + {`"abc".encode(42)`, "no implicit conversion of Integer into String"}, + {`"abc".encode(:utf8)`, "no implicit conversion of Symbol into String"}, // #encode! refuses a frozen receiver, even for a no-op transcoding. {`"foo".freeze.encode!("utf-8")`, "FrozenError"}, {`"foo".freeze.encode!("euc-jp")`, "FrozenError"}, diff --git a/internal/vm/string_encode.go b/internal/vm/string_encode.go index d3cd24db..14961ac7 100644 --- a/internal/vm/string_encode.go +++ b/internal/vm/string_encode.go @@ -131,13 +131,12 @@ func charEncodableIn(r rune, to string) bool { case "ISO-8859-1": return r < 0x100 } - if enc, ok := xtextEncodings[to]; ok { - _, err := enc.NewEncoder().Bytes([]byte(string(r))) - return err == nil - } - // An unknown target has no codec; leave the character in place so the encoder - // raises Encoding::ConverterNotFoundError rather than the fallback masking it. - return true + // A supported non-core target is always in the x/text table: stringEncode has + // already rejected any target lacking a converter, so charEncodableIn — reached + // only from the post-decode passes — never sees an unknown encoding. + enc := xtextEncodings[to] + _, err := enc.NewEncoder().Bytes([]byte(string(r))) + return err == nil } // applyFallback rewrites the UTF-8 intermediate u, replacing each character that @@ -335,8 +334,9 @@ func (vm *VM) decodeUTF32(src []byte, be bool, o transcodeOpts, repl string) str // encodeFromUTF8 encodes the UTF-8 intermediate u into encoding `to`. Characters // absent from the target raise Encoding::UndefinedConversionError, unless -// undef:replace substitutes them; an unsupported target raises -// Encoding::ConverterNotFoundError. +// undef:replace substitutes them. Its callers (stringEncode's early converter +// check, and caseMapBytes which first decodes) guarantee `to` has a converter, so +// a non-core target is one of the x/text codecs. func (vm *VM) encodeFromUTF8(u, to string, o transcodeOpts, repl string) []byte { switch to { case "UTF-8": @@ -350,11 +350,7 @@ func (vm *VM) encodeFromUTF8(u, to string, o transcodeOpts, repl string) []byte case "UTF-32LE", "UTF-32BE": return encodeUTF32(u, to == "UTF-32BE") } - if b, ok := xtextEncode(u, to, o.undefReplace, repl); ok { - return b - } - raise("Encoding::ConverterNotFoundError", "code converter not found (UTF-8 to %s)", to) - return nil + return xtextEncode(u, to, o.undefReplace, repl) } // encodeByteRange encodes each rune of u to a single byte when it is below limit diff --git a/internal/vm/string_encode_xtext.go b/internal/vm/string_encode_xtext.go index 88cd50c4..36ebe27b 100644 --- a/internal/vm/string_encode_xtext.go +++ b/internal/vm/string_encode_xtext.go @@ -66,25 +66,24 @@ func xtextDecode(src []byte, from string) (string, bool) { return string(out), true } -// xtextEncode encodes the UTF-8 string u into encoding `to` using x/text. ok is -// false when `to` has no x/text codec. A character absent from the target raises -// Encoding::UndefinedConversionError unless undefReplace substitutes it. -func xtextEncode(u, to string, undefReplace bool, repl string) ([]byte, bool) { - enc, ok := xtextEncodings[to] - if !ok { - return nil, false - } +// xtextEncode encodes the UTF-8 string u into encoding `to` using x/text. `to` is +// guaranteed to be one of the x/text codecs — encodeFromUTF8 reaches here only for +// a target that already passed the converter check. A character absent from the +// target raises Encoding::UndefinedConversionError unless undefReplace substitutes +// it. +func xtextEncode(u, to string, undefReplace bool, repl string) []byte { + enc := xtextEncodings[to] if undefReplace { // ReplaceUnsupported substitutes unrepresentable runes with SUB (0x1A) and // never errors; swap SUB for the requested replacement string afterwards. out, _ := encoding.ReplaceUnsupported(enc.NewEncoder()).Bytes([]byte(u)) - return replaceBytes(out, 0x1a, repl), true + return replaceBytes(out, 0x1a, repl) } out, err := enc.NewEncoder().Bytes([]byte(u)) if err != nil { raise("Encoding::UndefinedConversionError", "from UTF-8 to %s", to) } - return out, true + return out } // replaceBytes replaces every occurrence of the single byte b in s with repl.