Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String escapes #38

Open
laurentlb opened this issue Mar 21, 2019 · 4 comments
Open

String escapes #38

laurentlb opened this issue Mar 21, 2019 · 4 comments

Comments

@laurentlb
Copy link
Contributor

The specification doesn't describe string literals.
I tried this code. Each implementation of Starlark has a different result.

print("\a")
print("\b")
print("\c")
...
print("\z")

TODO:

  • Agree on the escapes we handle.
  • What shall we do with unknown escapes (e.g. \z). All implementations and Python return two characters \ and z. This behavior will prevent us to support more escapes in the future (as it would break users).

The fact that the \ character is sometimes used as an escape, and sometimes as a literal character is confusing.

Example of bug found in user code:

s = s.replace("'", "\'").replace('"', '\"')
@laurentlb
Copy link
Contributor Author

From the Python manual:

Changed in version 3.6: Unrecognized escape sequences produce a DeprecationWarning. In some future version of Python they will be a SyntaxError.

I'd propose to make this change in Starlark too.

@laurentlb
Copy link
Contributor Author

cc @Quarz0 @alandonovan
We'd like to make this change in Bazel soon.

@alandonovan
Copy link
Contributor

I agree that rejecting unrecognized escape sequences (like C/C++/Go/Java) is the right approach.
Migration should not be hard because the problem can be reliably identified during lexing.
I don't think we should add deprecation warnings: 99.9% of people forced to read them are powerless to do anything to address the problem.

@laurentlb
Copy link
Contributor Author

Agree. My comment was unclear, I wanted to suggest only the syntax error (Bazel has a way to roll out incompatible changes without using warnings).

bazel-io pushed a commit to bazelbuild/bazel that referenced this issue Jun 4, 2019
Related: #8380 , [#38](bazelbuild/starlark#38)

introduce flag --incompatible_restrict_escape_sequences=false
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
#8380

Closes #8526.

PiperOrigin-RevId: 251434634
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Related: bazelbuild#8380 , [bazelbuild#38](bazelbuild/starlark#38)

introduce flag --incompatible_restrict_escape_sequences=false
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380

Closes bazelbuild#8526.

PiperOrigin-RevId: 251434634
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
Related: bazelbuild#8380 , [bazelbuild#38](bazelbuild/starlark#38)

introduce flag --incompatible_restrict_escape_sequences=false
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380

Closes bazelbuild#8526.

PiperOrigin-RevId: 251434634
alandonovan pushed a commit to google/starlark-go that referenced this issue Mar 26, 2020
This change causes Starlark, like Go, to reject backslashes that
are not part of an escape sequence. Previously they were treated
literally, so \ ( would encode a two-character string, and much
code relied on this, especially for regular expressions.

This may break some programs, but the fix is simple:
double each errant backslash.

Python does not yet enforce this behavior, but since 3.6
has emitted a deprecation warning for it.

Also, document string escapes.

Related issues:
- Google issue b/34519173: "bazel: Forbid undefined escape sequences in strings"
- bazelbuild/starlark#38: Starlark spec: String escapes
- bazelbuild/buildtools#688: Bazel: Fix string escapes
- bazelbuild/bazel#8380: Bazel incompatible_restrict_string_escapes: Restrict string escapes

Change-Id: I5c9609a4e28d58593e9d6918757bca2cfd838d51
alandonovan pushed a commit to google/starlark-go that referenced this issue Mar 26, 2020
This change causes Starlark, like Go, to reject backslashes that
are not part of an escape sequence. Previously they were treated
literally, so \ ( would encode a two-character string.

Many programs rely on this, especially for regular expressions and
shell commands, and will be broken by this change, but the fix is simple:
double each errant backslash.

Python does not yet enforce this behavior, but since 3.6
has emitted a deprecation warning for it.

Also, document string escapes.

Related issues:
- Google issue b/34519173: "bazel: Forbid undefined escape sequences in strings"
- bazelbuild/starlark#38: Starlark spec: String escapes
- bazelbuild/buildtools#688: Bazel: Fix string escapes
- bazelbuild/bazel#8380: Bazel incompatible_restrict_string_escapes: Restrict string escapes

Change-Id: I5c9609a4e28d58593e9d6918757bca2cfd838d51
alandonovan pushed a commit to google/starlark-go that referenced this issue Mar 26, 2020
This change causes Starlark, like Go, to reject backslashes that
are not part of an escape sequence. Previously they were treated
literally, so \ ( would encode a two-character string.

Many programs rely on this, especially for regular expressions and
shell commands, and will be broken by this change, but the fix is simple:
double each errant backslash.

Python does not yet enforce this behavior, but since 3.6
has emitted a deprecation warning for it.

Also, document string escapes.

Related issues:
- Google issue b/34519173: "bazel: Forbid undefined escape sequences in strings"
- bazelbuild/starlark#38: Starlark spec: String escapes
- bazelbuild/buildtools#688: Bazel: Fix string escapes
- bazelbuild/bazel#8380: Bazel incompatible_restrict_string_escapes: Restrict string escapes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants