diff --git a/docs/ir/naming.md b/docs/ir/naming.md index 3a6f4a2eb..d4e567b03 100644 --- a/docs/ir/naming.md +++ b/docs/ir/naming.md @@ -87,9 +87,14 @@ lossless-by-default rules out. `Source` is the internal precedent: it carries `U no content rule touches it, because it records what the spec said rather than deciding a spelling. What is left is decidable without any grammar, and `irverify` holds an alias to exactly that much: -- **Every entry names something** (`ir/naming-alias-blank`). An entry whose every rune is one - Unicode classifies as invisible — a space, a control, a format character, or a default-ignorable - one — matches nothing under any grammar. `""`, `" "`, `"\u200b"` and `"\u3164"` are alike here. +- **Every entry names something** (`ir/naming-alias-blank`). An entry whose every rune is white + space, a control, or a `Default_Ignorable_Code_Point` (most format characters, the variation + selectors, and fillers such as U+3164) matches nothing under any grammar, U+1680 OGHAM SPACE MARK + counting with white space though it renders a line. `""`, `" "`, `"\u200b"`, `"\u3164"` and + `"\ufe0f"` are alike here. The format characters Unicode takes out of that property because they + render — the prepended concatenation marks, the interlinear annotation characters and the + Egyptian hieroglyph format controls — are visible, so an entry made of them is not blank, as + U+2800 is not. An invisible rune sitting *beside* a visible one is a different question and is not asked: whether `com.example.User` is a legal name is decidable only under the grammar of the format it will be matched against, which the IR does not know. diff --git a/ir/irverify/naming.go b/ir/irverify/naming.go index dfc9f4739..2d5eed2fa 100644 --- a/ir/irverify/naming.go +++ b/ir/irverify/naming.go @@ -300,8 +300,8 @@ func isWordSequence(s string) bool { } // isBlankName reports whether s holds no rune a name could be made of. It takes -// as invisible only what Unicode itself classifies so, needing no format's -// grammar. +// as invisible only what Unicode's own properties list (isInvisible), needing no +// format's grammar. // // strings.TrimSpace is not that test: IsSpace misses the zero-width joiners, // the soft hyphen and the BOM (all Cf), and space, control and Cf all miss @@ -319,10 +319,36 @@ func isBlankName(s string) bool { return true } -// isInvisible reports whether Unicode classifies r as carrying no visible mark. +// isInvisible reports whether r is white space, a control, or +// Default_Ignorable_Code_Point, the set the blank rules take as carrying no +// visible mark. U+1680 OGHAM SPACE MARK renders a line and stays in, as white +// space (GitHub #832). The package unicode has no table for that property, so it +// is built from its terms as +// DerivedCoreProperties.txt defines it, Other_Default_Ignorable_Code_Point, +// Cf and Variation_Selector less Prepended_Concatenation_Mark and the two +// ranges unicode lacks a table for, the interlinear annotation characters and +// the Egyptian hieroglyph format controls, which are Cf and render. func isInvisible(r rune) bool { - return unicode.IsSpace(r) || unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || - unicode.Is(unicode.Other_Default_Ignorable_Code_Point, r) + // The Cf ranges Unicode 17.0.0 takes out of Default_Ignorable_Code_Point that + // package unicode has no table for: U+FFF9..U+FFFB and U+13430..U+1343F. + // DerivedCoreProperties.txt's header writes the second as 13430..13440, but + // U+13440 is Mn and in no added term, so subtracting it changes nothing. + // TestIsInvisible_RangesWereCheckedAgainstThisUnicode fails when a Go release + // moves unicode.Version, so that move rechecks both. + const ( + interlinearFirst, interlinearLast = '\uFFF9', '\uFFFB' + egyptianFormatFirst, egyptianFormatLast = '\U00013430', '\U0001343F' + ) + interlinear := r >= interlinearFirst && r <= interlinearLast + egyptianFormat := r >= egyptianFormatFirst && r <= egyptianFormatLast + switch { + case unicode.IsSpace(r), unicode.IsControl(r): + return true + case interlinear, egyptianFormat, unicode.Is(unicode.Prepended_Concatenation_Mark, r): + return false + } + return unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Other_Default_Ignorable_Code_Point, r) || + unicode.Is(unicode.Variation_Selector, r) } // isCased reports whether s still carries casing an emitter should own. The test diff --git a/ir/irverify/naming_alias_test.go b/ir/irverify/naming_alias_test.go index 280b43412..d2fd9424c 100644 --- a/ir/irverify/naming_alias_test.go +++ b/ir/irverify/naming_alias_test.go @@ -7,6 +7,7 @@ import ( "fmt" "reflect" "testing" + "unicode" "unicode/utf8" "github.com/stretchr/testify/assert" @@ -48,10 +49,10 @@ func TestVerify_NoAliasesIsClean(t *testing.T) { } // TestVerify_BlankAliasIsAViolation covers the whole of what "names nothing" -// means. An entry made only of runes Unicode calls invisible is as blank as "", -// and testing emptiness alone — or trimming only what unicode.IsSpace reports — -// would let most of these through the one rule that exists to catch an entry -// matching nothing. +// means. An entry made only of runes the rule takes as invisible is as blank as +// "", and testing emptiness alone — or trimming only what unicode.IsSpace +// reports — would let most of these through the one rule that exists to catch +// an entry matching nothing. func TestVerify_BlankAliasIsAViolation(t *testing.T) { t.Parallel() blanks := map[string]string{ @@ -70,7 +71,24 @@ func TestVerify_BlankAliasIsAViolation(t *testing.T) { "nul": "\x00", "hangul filler": "\u3164", "jamo filler": "\u115f", - "invisible mix": "\u200b\t\ufeff\u3164", + // Variation_Selector: Mn, so neither Cf nor Other_Default_Ignorable. + "variation selector": "\ufe0f", + "variation selector supplement": "\U000e0100", + "mongolian selector": "\u180b", + "mongolian selector four": "\u180f", + // Other_Default_Ignorable, directly below the excluded U+FFF9. + "reserved before annotation": "\ufff8", + "invisible mix": "\u200b\t\ufeff\u3164", + // White_Space, though it renders a line. + "ogham space mark": "\u1680", + // White_Space in neither Zs nor Cc. + "line separator": "\u2028", + "paragraph separator": "\u2029", + // Controls past C0, which only their own term holds. + "delete": "\x7f", + "c1 control": "\u0080", + // Other_Default_Ignorable in Mn, which no other term holds. + "combining grapheme joiner": "\u034f", } for name, alias := range blanks { t.Run(name, func(t *testing.T) { @@ -92,13 +110,49 @@ func TestVerify_InvisibleRuneBesideAVisibleOneIsNotBlank(t *testing.T) { assert.Empty(t, aliasViolations(t, "com.example.\u200bUser", " padded ")) } -// TestVerify_BlankLookingGraphicAliasIsNotBlank holds the other side. U+2800 -// BRAILLE PATTERN BLANK renders as whitespace but is a graphic character -// Unicode does not classify as invisible, so the rule leaves it alone rather -// than deciding on its own account what looks empty. -func TestVerify_BlankLookingGraphicAliasIsNotBlank(t *testing.T) { +// TestVerify_VisibleAliasIsNotBlank holds the other side. U+2800 +// BRAILLE PATTERN BLANK renders as whitespace but Unicode does not classify it +// as invisible, so the rule leaves it alone rather than deciding on its own +// account what looks empty. It leaves alone, too, the format characters Unicode +// takes out of Default_Ignorable_Code_Point because they render: a prepended +// concatenation mark, an interlinear annotation character and an Egyptian +// hieroglyph format control. The rows beside those ranges and the private use one +// hold a term from widening over them: U+13440 from Mn, U+E000 from C, U+FFFC +// from So, U+1342F from Lo. +func TestVerify_VisibleAliasIsNotBlank(t *testing.T) { t.Parallel() - assert.Empty(t, aliasViolations(t, "\u2800")) + visible := map[string]string{ + "braille blank": "\u2800", + "arabic number sign": "\u0600", + "arabic end of ayah": "\u06dd", + "syriac abbreviation": "\u070f", + "arabic pound mark": "\u0890", + "arabic disputed end": "\u08e2", + "kaithi number sign": "\U000110bd", + "annotation anchor": "\ufff9", + "annotation end": "\ufffb", + "egyptian format": "\U00013430", + "egyptian last": "\U0001343f", + "object replacement": "\ufffc", + "egyptian before": "\U0001342f", + "egyptian mirror": "\U00013440", + "private use": "\ue000", + } + for name, alias := range visible { + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.Empty(t, aliasViolations(t, alias)) + }) + } +} + +// TestIsInvisible_RangesWereCheckedAgainstThisUnicode fails when a Go release moves +// unicode.Version, since isInvisible holds two ranges no table of package unicode +// carries. Recheck them against DerivedCoreProperties.txt, then move the version here. +func TestIsInvisible_RangesWereCheckedAgainstThisUnicode(t *testing.T) { + t.Parallel() + assert.Equal(t, "17.0.0", unicode.Version, + "recheck isInvisible's ranges against DerivedCoreProperties.txt, then move this version") } // TestVerify_IllFormedAliasIsAViolation covers the rule every channel shares, diff --git a/ir/irverify/stringlists_test.go b/ir/irverify/stringlists_test.go index 1bca23e6f..703c26433 100644 --- a/ir/irverify/stringlists_test.go +++ b/ir/irverify/stringlists_test.go @@ -101,8 +101,9 @@ func TestVerify_HeldListFixturesMatchTheRules(t *testing.T) { } // TestVerify_BlankListEntryIsAViolation holds each path and set list to its -// blank rule. An entry made only of runes Unicode calls invisible is as blank as -// "", and each is reported at the entry, so the visible one beside them is not. +// blank rule. An entry made only of runes the rule takes as invisible is as +// blank as "", and each is reported at the entry, so the visible ones beside +// them are not. func TestVerify_BlankListEntryIsAViolation(t *testing.T) { t.Parallel() require.Empty(t, verifyDeclared(heldListDoc()), "the fixture must draw nothing of its own") @@ -114,10 +115,10 @@ func TestVerify_BlankListEntryIsAViolation(t *testing.T) { code, message = "ir/set-entry-blank", "set entry is blank, so it names nothing" } doc := heldListDoc() - list.set(doc, []string{"", " ", "\u3164", "acme"}) + list.set(doc, []string{"", " ", "\u3164", "\ufe0f", "acme", "\u0600", "\ufff9", "\U00013430"}) got := verifyDeclared(doc) - require.Len(t, got, 3, "one violation per blank entry") + require.Len(t, got, 4, "one violation per blank entry, none for a visible format character") for i, v := range got { assert.Equal(t, code, v.Code) assert.Equal(t, fmt.Sprintf("%s[%d]", list.path, i), v.Path)