Repository: robertkrimen/otto
Pull request #524 (2024-06-10 13:55:53)

parser/lexer: correct ID_Start & ID_Continue checks
`unicode.IsLetter` and `unicode.IsDigit` will not return the complete set of `ID_Start` and `ID_Continue` characters defined here:  https://www.unicode.org/reports/tr31/.

filips: > Looks reasonable but lets get some tests which exercise this.

I added a few tests for the areas where the allowed character sets differ. Please let me know if you want me to add further tests.
filips: @stevenh Linter warnings should be fixed now
filips: > Thanks for this, just a minor nit and then good to go.

Sure thing, I pushed with the methods unexported

Patch:
diff --git a/parser/lexer.go b/parser/lexer.go
index 3ff62d0..0d9132f 100644
--- a/parser/lexer.go
+++ b/parser/lexer.go
@@ -38,6 +38,53 @@ func digitValue(chr rune) int {
 	return 16 // Larger than any legal digit value
 }
 
+// See https://www.unicode.org/reports/tr31/ for reference on ID_Start and ID_Continue.
+var includeIDStart = []*unicode.RangeTable{
+	unicode.Lu,
+	unicode.Ll,
+	unicode.Lt,
+	unicode.Lm,
+	unicode.Lo,
+	unicode.Nl,
+	unicode.Other_ID_Start,
+}
+
+var includeIDContinue = []*unicode.RangeTable{
+	unicode.Lu,
+	unicode.Ll,
+	unicode.Lt,
+	unicode.Lm,
+	unicode.Lo,
+	unicode.Nl,
+	unicode.Other_ID_Start,
+	unicode.Mn,
+	unicode.Mc,
+	unicode.Nd,
+	unicode.Pc,
+	unicode.Other_ID_Continue,
+}
+
+var exclude = []*unicode.RangeTable{
+	unicode.Pattern_Syntax,
+	unicode.Pattern_White_Space,
+}
+
+func unicodeIDStart(r rune) bool {
+	if unicode.In(r, exclude...) {
+		return false
+	}
+
+	return unicode.In(r, includeIDStart...)
+}
+
+func unicodeIDContinue(r rune) bool {
+	if unicode.In(r, exclude...) {
+		return false
+	}
+
+	return unicode.In(r, includeIDContinue...)
+}
+
 func isDigit(chr rune, base int) bool {
 	return digitValue(chr) < base
 }
@@ -45,14 +92,14 @@ func isDigit(chr rune, base int) bool {
 func isIdentifierStart(chr rune) bool {
 	return chr == '$' || chr == '_' || chr == '\\' ||
 		'a' <= chr && chr <= 'z' || 'A' <= chr && chr <= 'Z' ||
-		chr >= utf8.RuneSelf && unicode.IsLetter(chr)
+		chr >= utf8.RuneSelf && unicodeIDStart(chr)
 }
 
 func isIdentifierPart(chr rune) bool {
 	return chr == '$' || chr == '_' || chr == '\\' ||
 		'a' <= chr && chr <= 'z' || 'A' <= chr && chr <= 'Z' ||
 		'0' <= chr && chr <= '9' ||
-		chr >= utf8.RuneSelf && (unicode.IsLetter(chr) || unicode.IsDigit(chr))
+		chr >= utf8.RuneSelf && unicodeIDContinue(chr)
 }
 
 func (p *parser) scanIdentifier() (string, error) {


Test patch:
diff --git a/parser/lexer_test.go b/parser/lexer_test.go
index faa98a4..7e1e5fa 100644
--- a/parser/lexer_test.go
+++ b/parser/lexer_test.go
@@ -351,6 +351,21 @@ Second line \
 			token.RIGHT_BRACKET, "", 6,
 		)
 
+		// Identifier from Unicode Nl
+		test("\u16ee",
+			token.IDENTIFIER, "ᛮ", 1,
+		)
+
+		// Identifier from Unicode Other_ID_Start
+		test("\u212e",
+			token.IDENTIFIER, "℮", 1,
+		)
+
+		// Using char from ID_Continue after valid start char
+		test("a\u0300",
+			token.IDENTIFIER, "à", 1,
+		)
+
 		// ILLEGAL
 
 		test(`3ea`,
@@ -383,5 +398,15 @@ Second line \
 			token.STRING, "\"\\x0G\"", 1,
 			token.EOF, "", 7,
 		)
+
+		// Starting identifier with ID_Continue char from Nm
+		test("\u0300",
+			token.ILLEGAL,
+		)
+
+		// Starting identifier with Pattern_Syntax
+		test("'",
+			token.ILLEGAL,
+		)
 	})
 }