Parsing fails on valid JS: string literal with \0

[bzz]

Parsing string.js file with content

var escSlash = '\0SLASH'+Math.random()+'\0';
var escOpen = '\0OPEN'+Math.random()+'\0';
var escClose = '\0CLOSE'+Math.random()+'\0';
var escComma = '\0COMMA'+Math.random()+'\0';
var escPeriod = '\0PERIOD'+Math.random()+'\0';

results in

 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0SLASH')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0OPEN')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0CLOSE')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0COMMA')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0')
 check: key "value": invalid syntax
 check: key "value": invalid syntax ('\0PERIOD')

Found though #54

Steps to reproduce

$ bblfsh-cli string.js

[bzz]

The issue comes from strconv.go#unquoteSingle here is a simple test case to reproduce

func TestUnquoteSingle(T *testing.T) {
	s, err := unquoteSingle(`'\0'`)
	require.NoError(T, err)
}

which results in invalid syntax and is showing up in the logs above though something like this

From original PR discussion #50 (comment)

As for strconv.go, I don't think it requires extensive test coverage since it's copied from the Go stdlib, which already ensures that the code is correct. But we can add a few small test cases to detect regressions.

[bzz]

The cause is an error from

strconv.UnquoteChar(`'\0'`, '\'')`

in this clause

[bzz]

Seems to be related to the difference how \0 is treated in 2 languages:

• in ES5 \0 is treated the same as a single byte escape sequence (even if it's not) equivalent to hex \x00 or octal \000 (octal is deprecated in strict mode) and is meant to represent U+0000
• in Golang it does not get any special treatment

A proposed solution includes converting \0 to the \x00 on Golang side in the Semantic mode.

Current implementation of Op singleQuote is supposed to behave like strconv.Unquote, but

• ✅strconv.Unquote("'\\0'") -> syntax error
• ✅strconv.Unquote("'\x00'") -> U+0000
• ✅unquoteSingle("'\\0'") -> syntax error
• 🚫unquoteSingle("'\x00'") -> syntax error

[creachadair]

A proposed solution includes converting \0 to the \x00 on Golang side in the Semantic mode.

That seems reasonable. The details of how we handle unquoting are private to the implementation, so if we have to do some patch-work it shouldn't break anything fundamental.

Unfortunately there are a lot of subtle variations in (un)escaping rules for string literals, and the desire to lean on Go's implementation is understandable but doesn't always line up correctly with other languages. 🙍🏻‍♀️