icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Improve Escape Sequences in String Literals #660

Open InsertCreativityHere opened 11 months ago

InsertCreativityHere commented 11 months ago

Continued from #659.

Currently, the Slice compiler handles escape sequences like NodeJS does. If it sees a backslash in a string literal, we remove the backslash and the character immediately after it gets kept as a literal character. For instance: [deprecated("This is bad, use \"Foo\" instead.")].

This works perfectly for the two cases Slice cares about: \" -> " and \\ -> \, and is consistent with how we support escaping keywords for identifiers: \struct -> struct.

But it leaves other cases that might be surprising to some users: \n -> n (instead of a newline character) \p -> p (it's useless to escape p here)


Should we emit an error/warning when users do this explaining the Slice behavior? Or should we alter the Slice behavior to handle these differently?

I think the current behavior is fine (NodeJS does it, and it's consistent with identifiers), but I open this issue to discuss alternatives.

InsertCreativityHere commented 7 months ago

I think in the future we should add a lint for this: UnnecessaryCharacterEscape (by default it'd be a warning).

This solves the problem of a user typing \n and being surprised not to get a newline. Now they'll see this warning telling them how escaping works.

Also, I just think the lint is correct. Typing Hell\o is literally pointless in Slice. The only characters that need escaping are " and \. Escaping anything should be a warning IMO. (with a little thing saying: note: To type a literal backslash character, you must escape it using '\\')


Or, maybe we could broaden this even further and have a lint: UnnecessaryEscapeCharacter. And issue warnings for what I talked about above, and for things like:

struct \MyStruct {}

Again, this isn't incorrect to do, but it's completely useless. So it might be good to issue warnings for these kinds of things too.