diff --git a/README.md b/README.md index 12de544..ada2699 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,11 @@ A broad, practical subset of Ruby 4.0, all differential-tested against MRI: `(a, b)` destructuring group params, stabby lambdas `->(){}`, numbered params (`_1`) and `it`, `yield`, `super`, **multiple-value `return a, b`**. - **Classes/modules/metaprogramming:** `class`/`module`, inheritance, `@ivars`, - **`@@class variables`**, constants, singleton method defs + **`@@class variables`**, constants — including **non-ASCII** ones, since Ruby + decides constant-ness from the first character's *case* and not from `A-Z`, so + `Ἅbb` and the titlecase `Džx` are constants while `μm`, `日x` and `אx` are + local variables (`token.IsConstStart` carries the rule and its citations) — + singleton method defs (`def self.foo`/`def obj.foo`/`def Const.foo`), **global-variable assignment** (`$g = …`), multiple assignment / destructuring, **adjacent string-literal concatenation** (`"a" "b"`). diff --git a/nonascii_constant_test.go b/nonascii_constant_test.go new file mode 100644 index 0000000..3a5a710 --- /dev/null +++ b/nonascii_constant_test.go @@ -0,0 +1,87 @@ +package parser + +import ( + "testing" + + "github.com/go-ruby-parser/parser/ast" +) + +// TestNonASCIIConstantIdentifier covers the parse-level consequence of Ruby +// deciding constant-ness from the first character's CASE rather than from its +// being in 'A'..'Z': a name beginning with a non-ASCII uppercase or titlecase +// letter is a CONSTANT, and one beginning with a lowercase or CASELESS letter is +// a local variable. +// +// ⛔ `LookupIdent` tested `s[0]`, a BYTE, so every multibyte name came back +// IDENT. `ἍBB = 1` parsed as a local-variable assignment, which is why +// `defined?(ἍBB)` answered nil in rbgo where MRI answers "constant", and why +// ruby/spec's `language/variables_spec.rb` "parses a non-ASCII upcased +// character as a constant identifier" could not pass +// (go-embedded-ruby/ruby#799). +// +// Each classification was measured against /usr/bin/ruby; the per-character +// table and its citations live in token.IsConstStart. +func TestNonASCIIConstantIdentifier(t *testing.T) { + for _, tc := range []struct { + src string + kind string // "const" or "local" + }{ + {"ἍBB = 1", "const"}, // GREEK CAPITAL ALPHA WITH PSILI AND OXIA (Lu) + {"Дeta = 1", "const"}, // CYRILLIC CAPITAL DE (Lu) + {"École = 1", "const"}, + {"Džx = 1", "const"}, // titlecase digraph (Lt) + {"μ = 1", "local"}, // GREEK SMALL MU (Ll) + {"日x = 1", "local"}, // CJK, caseless (Lo) + {"אx = 1", "local"}, // HEBREW ALEF, caseless (Lo) + {"Ascii = 1", "const"}, + {"ascii = 1", "local"}, + } { + t.Run(tc.src, func(t *testing.T) { + prog, err := Parse(tc.src) + if err != nil { + t.Fatalf("Parse(%q) error: %v", tc.src, err) + } + if len(prog.Body) != 1 { + t.Fatalf("Parse(%q) gave %d statements, want 1", tc.src, len(prog.Body)) + } + switch node := prog.Body[0].(type) { + case *ast.ConstAssign: + if tc.kind != "const" { + t.Errorf("Parse(%q) gave *ast.ConstAssign, want a local assignment", tc.src) + } + case *ast.Assign: + if tc.kind != "local" { + t.Errorf("Parse(%q) gave *ast.Assign, want *ast.ConstAssign", tc.src) + } + default: + t.Errorf("Parse(%q) gave %T, want *ast.ConstAssign or *ast.Assign", tc.src, node) + } + }) + } +} + +// A non-ASCII constant must work everywhere an ASCII one does, or the fix would +// only have moved the problem into the parser's other constant positions. +func TestNonASCIIConstantInEveryPosition(t *testing.T) { + for _, src := range []string{ + "ἍBB", // a read + "ἍBB = 1", // an assignment + "ἍBB ||= 1", // an op-assign + "ἍBB::Inner", // as a scope + "Outer::ἍBB", // as a scoped name + "ἍBB, x = 1, 2", // a masgn target + "class ἍBB; end", // a class name + "module ἍBB; end", // a module name + "class ἍBB < Base; end", // and with a superclass + "case x; in ἍBB; end", // a pattern + "def ἍBB.m; end", // a singleton definee + "ἍBB.new", // a receiver + } { + if _, err := Parse(src); err != nil { + t.Errorf("Parse(%q) error: %v", src, err) + } + } + if _, err := Parse("begin\n x\nrescue ἍBB => e\nend"); err != nil { + t.Errorf("non-ASCII constant as a rescue class: %v", err) + } +} diff --git a/token/const_start_test.go b/token/const_start_test.go new file mode 100644 index 0000000..ba8df57 --- /dev/null +++ b/token/const_start_test.go @@ -0,0 +1,94 @@ +package token + +import "testing" + +// Ruby decides constant-ness from the first character's CASE, not from its being +// in 'A'..'Z'. Every row here was measured against /usr/bin/ruby through +// `Object.const_defined?(name.to_sym)`, which raises NameError for a name that +// is not a constant name — that is rb_is_const_name (symbol.c:1287), which is +// the same predicate parse.y consults. +// +// ⛔ Before this, the test was `s[0] >= 'A' && s[0] <= 'Z'` on a BYTE, so every +// multibyte name read as a local: `ἍBB = 1` created a local variable and no +// constant existed afterwards. +func TestIsConstStartFollowsRubysCaseRule(t *testing.T) { + for _, tc := range []struct { + name string + first string // the leading character, by name and category + konst bool + }{ + // ASCII, unchanged. + {"A", "ASCII upper", true}, + {"Z", "ASCII upper", true}, + {"a", "ASCII lower", false}, + {"z", "ASCII lower", false}, + {"_", "ASCII underscore", false}, + {"_Foo", "ASCII underscore", false}, + + // Lu — uppercase letters outside ASCII. + {"ἍBB", "GREEK CAPITAL ALPHA WITH PSILI AND OXIA (Lu)", true}, + {"Дa", "CYRILLIC CAPITAL DE (Lu)", true}, + {"İstanbul", "LATIN CAPITAL I WITH DOT ABOVE (Lu)", true}, + {"École", "LATIN CAPITAL E WITH ACUTE (Lu)", true}, + + // ⛔ Lt — titlecase letters. rb_sym_constant_char_p tests the + // "titlecaseletter" Onigmo property explicitly, AFTER isupper and + // islower have both said no, so these are constants although + // unicode.IsUpper alone reports false for them. + {"Džx", "LATIN CAPITAL D WITH SMALL Z WITH CARON (Lt)", true}, + {"Ljx", "LATIN CAPITAL L WITH SMALL J (Lt)", true}, + + // Ll — lowercase letters outside ASCII. + {"μm", "GREEK SMALL MU (Ll)", false}, + {"école", "LATIN SMALL E WITH ACUTE (Ll)", false}, + + // ⛔ Lo — CASELESS letters start a LOCAL variable, not a constant. + // This is why neither clause may be written as "not lowercase". + {"日x", "CJK UNIFIED IDEOGRAPH 65E5 (Lo)", false}, + {"אx", "HEBREW ALEF (Lo)", false}, + {"بx", "ARABIC BEH (Lo)", false}, + + // Degenerate inputs: no character to read is not an uppercase one. + {"", "empty", false}, + {"\xff\xfe", "invalid UTF-8", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := IsConstStart(tc.name); got != tc.konst { + t.Errorf("IsConstStart(%q) = %v, want %v — %s", tc.name, got, tc.konst, tc.first) + } + // LookupIdent must agree, since that is what the lexer calls. A + // keyword is neither, and no keyword is non-ASCII, so every row here + // reaches the same branch. + want := IDENT + if tc.konst { + want = CONST + } + if tc.name == "" { + return // LookupIdent is never called with an empty identifier + } + if got := LookupIdent(tc.name); got != want { + t.Errorf("LookupIdent(%q) = %v, want %v — %s", tc.name, got, want, tc.first) + } + }) + } +} + +// The control: a keyword still wins over the case rule, and the rule has not +// swallowed the ASCII path. Without this, a version of IsConstStart that always +// answered false would still pass the `false` rows above. +func TestIsConstStartControls(t *testing.T) { + if LookupIdent("def") != DEF { + t.Error("a keyword must still be looked up as a keyword") + } + if !IsConstStart("Foo") { + t.Fatal("the ASCII path is broken: Foo must be a constant start") + } + if IsConstStart("foo") { + t.Fatal("the ASCII path is broken: foo must not be a constant start") + } + // And the planted positive for the branch this change added: a name that is + // a constant ONLY because of the non-ASCII clause. + if !IsConstStart("ἍBB") { + t.Fatal("the non-ASCII clause is not reached") + } +} diff --git a/token/token.go b/token/token.go index 2fa50f3..b66c73b 100644 --- a/token/token.go +++ b/token/token.go @@ -6,6 +6,11 @@ // the grammar grows (plan-rbgo.md §10). package token +import ( + "unicode" + "unicode/utf8" +) + type Type int const ( @@ -167,8 +172,52 @@ func LookupIdent(s string) Type { if kw, ok := Keywords[s]; ok { return kw } - if c := s[0]; c >= 'A' && c <= 'Z' { + if IsConstStart(s) { return CONST } return IDENT } + +// IsConstStart reports whether s begins with a character Ruby treats as the +// start of a CONSTANT, rather than of a local variable. +// +// ⛔ The test used to be `s[0] >= 'A' && s[0] <= 'Z'`, which reads a BYTE. For +// any multibyte character s[0] is a UTF-8 lead byte of 0xC2 or more, never in +// 'A'..'Z', so every non-ASCII name was classified as a local — `ἍBB = 1` +// created a local variable and no constant existed afterwards, where MRI reports +// `defined?(ἍBB)` as "constant". +// +// The rule is not the lexer's: parse.y assumes tCONSTANT provisionally and +// DOWNGRADES it with `if (result == tCONSTANT && is_local_id(ident)) result = +// tIDENTIFIER` (parse.y:10453 at v4.0.9), so the decision belongs to the ID +// system. rb_sym_constant_char_p (symbol.c:508-542) is where it is made: +// +// ASCII -> ISUPPER +// non-ASCII, rb_enc_isupper -> constant +// non-ASCII, rb_enc_islower -> local +// titlecase letter, Unicode -> constant +// anything else (caseless) -> local +// +// Go's unicode.IsUpper is that isupper and unicode.IsTitle that titlecase +// property, so the two clauses below are those four lines. Measured against +// /usr/bin/ruby through `Object.const_defined?`, which raises NameError for a +// name that is not a constant name (rb_is_const_name, symbol.c:1287): Greek Ἅ, +// Cyrillic Д and Turkish İ are constants; the titlecase digraphs Dž and Lj are +// constants too; Greek μ, CJK 日, Hebrew א and Arabic ب are not — a caseless +// letter starts a LOCAL variable in Ruby, which is why neither clause may be +// loosened to "not lower". +func IsConstStart(s string) bool { + if s == "" { + return false + } + if c := s[0]; c < utf8.RuneSelf { + return c >= 'A' && c <= 'Z' + } + r, size := utf8.DecodeRuneInString(s) + if r == utf8.RuneError && size <= 1 { + // Invalid UTF-8: rb_sym_constant_char_p returns FALSE when + // rb_enc_precise_mbclen finds no character, so this is a local. + return false + } + return unicode.IsUpper(r) || unicode.IsTitle(r) +}