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

incompatible_restrict_string_escapes: Restrict string escapes #8380

Closed
laurentlb opened this issue May 17, 2019 · 5 comments
Closed

incompatible_restrict_string_escapes: Restrict string escapes #8380

laurentlb opened this issue May 17, 2019 · 5 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)

Comments

@laurentlb
Copy link
Contributor

laurentlb commented May 17, 2019

When the flag incompatible_restrict_escape_sequences is set, Bazel will reject any string with an unknown escape sequence.

For example, the sequence \w doesn't exist. The old behavior was to interpret "\w" as if it was \\w (in other words, the string will contain a backslash and a w). This behavior is problematic because adding any new escape sequence to the language becomes a breaking change. It may also confuse readers of the code: some backslashes need to be escaped, some don't.

The fix is simple. If you need a literal backslash, escape it:

- x = "\w"
+ x = "\\w"

Buildifier will be updated to automatically fix this.

We intend to flip this flag with Bazel 1.0.

Implement: bazelbuild/starlark#38

@laurentlb laurentlb added P1 I'll work on this now. (Assignee required) team-Starlark incompatible-change Incompatible/breaking change labels May 17, 2019
Quarz0 added a commit to Quarz0/bazel that referenced this issue May 31, 2019
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380
Quarz0 added a commit to Quarz0/bazel that referenced this issue May 31, 2019
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380
Quarz0 added a commit to Quarz0/bazel that referenced this issue May 31, 2019
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380
Quarz0 added a commit to Quarz0/bazel that referenced this issue May 31, 2019
When the flag is enabled, invalid escape sequences like "\z" are
rejected.

RELNOTES: Flag `--incompatible_restrict_escape_sequences` is added. See
bazelbuild#8380
bazel-io pushed a commit 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
@laurentlb laurentlb changed the title Restrict string escapes incompatible_restrict_escape_sequences: Restrict string escapes Jun 12, 2019
@laurentlb laurentlb assigned laurentlb and unassigned vladmos Jun 12, 2019
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
@laurentlb laurentlb changed the title incompatible_restrict_escape_sequences: Restrict string escapes incompatible_restrict_string_escapes: Restrict string escapes Jul 4, 2019
bazel-io pushed a commit that referenced this issue Jul 10, 2019
Baseline: 2e374a9

Cherry picks:

   + 6d0b14b:
     rule_test: apply "tags" to all rules in the macro

Incompatible changes:

  - Add --incompatible_enable_profile_by_default to enable the JSON
    profile by default.
  - The --incompatible_windows_style_arg_escaping flag is flipped to
    "true", and the "false" case unsupported. Bazel no longer accepts
    this flag.

Important changes:

  - Bazel now supports hiding compiler warnings for targets that
    you're not explicitly building (see
    https://docs.bazel.build/versions/master/user-manual.html#flag--au
    to_output_filter).
  - Flag `--incompatible_restrict_escape_sequences` is added. See
    #8380
  - The "info" command now supports the "starlark-semantics"
    argument, which outputs a representation of the effective Starlark
    semantics option values.
  - The `outputs` parameter of the `rule()` function is deprecated
    and attached to flag `--incompatible_no_rule_outputs_param`.
    Migrate rules to use `OutputGroupInfo` or `attr.output` instead.
    See #7977 for more info.
  - When `--incompatible_strict_action_env` is enabled, the default
    `PATH` now includes `/usr/local/bin`.
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - `@bazel_tools//tools/jdk:toolchain_java10` and
    `@bazel_tools//tools/jdk:toolchain_java11` are now available to
    enable java 10, respectively java 11 language level support.
  - The `command` parameter of the `actions.run_shell()` function
    will be restricted to only accept strings (and not string
    sequences). This check is attached to flag
    `--incompatible_run_shell_command_string`. One may migrate by
    using the `arguments` parameter of `actions.run()` instead. See
    #5903 for more info.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 for
    details.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 f...
  - Bazel's C++ autoconfiguration now understands `BAZEL_LINKLIBS`
    environment variable to specify system libraries that should be
    appended to the link command line.
  - paths under the execution root starting with "." or "_" will be
    re-linked across builds
  - execution_log_json_file now allows actions without outputs.
  - Labels aapt as deprecated for aapt_version, and heavily endorses
    aapt2.
  - Update doc links still pointing to cc_binary.features to point to
    common features
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 for
    details.
    RELNOTES:
  - --incompatible_disable_nocopts flag has been added. See
    #8706 for details.
  - Fixed treatment of <dist:module /> tags in AndroidManifest.xml
  - Fixed asset precedence for android_binary rules with aapt2.
  - Bazel now officially supports running on CentOS 7.
  - The runtime dynamic libraries are no longer in default output
    group of cc_binary.
  - set the FDOBuildType as CSFDO for binaries built with
    --cs_fdo_absolute_path.
  - Bazel can now be bootstrapped and built on arm64 platforms
    without requiring any flags or patches.
  - Fixed treatment of AndroidManifest.xml attributes which contained
    XML escaping
  - Retire experimental blaze flag
    experimental_link_compile_output_separately. The same behavior is
    available through the feature dynamic_link_test_srcs.
  - --incompatible_load_java_rules_from_bzl was added to forbid
    loading the native java rules directly. See more on tracking
    issue #8746
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - Attribute names are going to be restricted and must be
    syntactically valid identifiers.
    #6437
  - rule_test: fix Bazel 0.27 regression ("tags" attribute was
    ingored, #8723

This release contains contributions from many people at Google, as well as Ben Diuguid, Benjamin Peterson, Dave Lee, Loo Rong Jie, Mark Butcher, Marwan Tammam, Pedro Alvarez.
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
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
Baseline: 2e374a9

Cherry picks:

   + 6d0b14b:
     rule_test: apply "tags" to all rules in the macro

Incompatible changes:

  - Add --incompatible_enable_profile_by_default to enable the JSON
    profile by default.
  - The --incompatible_windows_style_arg_escaping flag is flipped to
    "true", and the "false" case unsupported. Bazel no longer accepts
    this flag.

Important changes:

  - Bazel now supports hiding compiler warnings for targets that
    you're not explicitly building (see
    https://docs.bazel.build/versions/master/user-manual.html#flag--au
    to_output_filter).
  - Flag `--incompatible_restrict_escape_sequences` is added. See
    bazelbuild#8380
  - The "info" command now supports the "starlark-semantics"
    argument, which outputs a representation of the effective Starlark
    semantics option values.
  - The `outputs` parameter of the `rule()` function is deprecated
    and attached to flag `--incompatible_no_rule_outputs_param`.
    Migrate rules to use `OutputGroupInfo` or `attr.output` instead.
    See bazelbuild#7977 for more info.
  - When `--incompatible_strict_action_env` is enabled, the default
    `PATH` now includes `/usr/local/bin`.
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - `@bazel_tools//tools/jdk:toolchain_java10` and
    `@bazel_tools//tools/jdk:toolchain_java11` are now available to
    enable java 10, respectively java 11 language level support.
  - The `command` parameter of the `actions.run_shell()` function
    will be restricted to only accept strings (and not string
    sequences). This check is attached to flag
    `--incompatible_run_shell_command_string`. One may migrate by
    using the `arguments` parameter of `actions.run()` instead. See
    bazelbuild#5903 for more info.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See bazelbuild#8622 for
    details.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See bazelbuild#8622 f...
  - Bazel's C++ autoconfiguration now understands `BAZEL_LINKLIBS`
    environment variable to specify system libraries that should be
    appended to the link command line.
  - paths under the execution root starting with "." or "_" will be
    re-linked across builds
  - execution_log_json_file now allows actions without outputs.
  - Labels aapt as deprecated for aapt_version, and heavily endorses
    aapt2.
  - Update doc links still pointing to cc_binary.features to point to
    common features
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See bazelbuild#8622 for
    details.
    RELNOTES:
  - --incompatible_disable_nocopts flag has been added. See
    bazelbuild#8706 for details.
  - Fixed treatment of <dist:module /> tags in AndroidManifest.xml
  - Fixed asset precedence for android_binary rules with aapt2.
  - Bazel now officially supports running on CentOS 7.
  - The runtime dynamic libraries are no longer in default output
    group of cc_binary.
  - set the FDOBuildType as CSFDO for binaries built with
    --cs_fdo_absolute_path.
  - Bazel can now be bootstrapped and built on arm64 platforms
    without requiring any flags or patches.
  - Fixed treatment of AndroidManifest.xml attributes which contained
    XML escaping
  - Retire experimental blaze flag
    experimental_link_compile_output_separately. The same behavior is
    available through the feature dynamic_link_test_srcs.
  - --incompatible_load_java_rules_from_bzl was added to forbid
    loading the native java rules directly. See more on tracking
    issue bazelbuild#8746
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - Attribute names are going to be restricted and must be
    syntactically valid identifiers.
    bazelbuild#6437
  - rule_test: fix Bazel 0.27 regression ("tags" attribute was
    ingored, bazelbuild#8723

This release contains contributions from many people at Google, as well as Ben Diuguid, Benjamin Peterson, Dave Lee, Loo Rong Jie, Mark Butcher, Marwan Tammam, Pedro Alvarez.
laurentlb pushed a commit that referenced this issue Jul 16, 2019
Baseline: 2e374a9

Cherry picks:

   + 6d0b14b:
     rule_test: apply "tags" to all rules in the macro

Incompatible changes:

  - Add --incompatible_enable_profile_by_default to enable the JSON
    profile by default.
  - The --incompatible_windows_style_arg_escaping flag is flipped to
    "true", and the "false" case unsupported. Bazel no longer accepts
    this flag.

Important changes:

  - Bazel now supports hiding compiler warnings for targets that
    you're not explicitly building (see
    https://docs.bazel.build/versions/master/user-manual.html#flag--au
    to_output_filter).
  - Flag `--incompatible_restrict_escape_sequences` is added. See
    #8380
  - The "info" command now supports the "starlark-semantics"
    argument, which outputs a representation of the effective Starlark
    semantics option values.
  - The `outputs` parameter of the `rule()` function is deprecated
    and attached to flag `--incompatible_no_rule_outputs_param`.
    Migrate rules to use `OutputGroupInfo` or `attr.output` instead.
    See #7977 for more info.
  - When `--incompatible_strict_action_env` is enabled, the default
    `PATH` now includes `/usr/local/bin`.
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - `@bazel_tools//tools/jdk:toolchain_java10` and
    `@bazel_tools//tools/jdk:toolchain_java11` are now available to
    enable java 10, respectively java 11 language level support.
  - The `command` parameter of the `actions.run_shell()` function
    will be restricted to only accept strings (and not string
    sequences). This check is attached to flag
    `--incompatible_run_shell_command_string`. One may migrate by
    using the `arguments` parameter of `actions.run()` instead. See
    #5903 for more info.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 for
    details.
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 f...
  - Bazel's C++ autoconfiguration now understands `BAZEL_LINKLIBS`
    environment variable to specify system libraries that should be
    appended to the link command line.
  - paths under the execution root starting with "." or "_" will be
    re-linked across builds
  - execution_log_json_file now allows actions without outputs.
  - Labels aapt as deprecated for aapt_version, and heavily endorses
    aapt2.
  - Update doc links still pointing to cc_binary.features to point to
    common features
  - Incompatible change
    `--incompatible_use_platforms_repo_for_constraints` has been
    added. See #8622 for
    details.
    RELNOTES:
  - --incompatible_disable_nocopts flag has been added. See
    #8706 for details.
  - Fixed treatment of <dist:module /> tags in AndroidManifest.xml
  - Fixed asset precedence for android_binary rules with aapt2.
  - Bazel now officially supports running on CentOS 7.
  - The runtime dynamic libraries are no longer in default output
    group of cc_binary.
  - set the FDOBuildType as CSFDO for binaries built with
    --cs_fdo_absolute_path.
  - Bazel can now be bootstrapped and built on arm64 platforms
    without requiring any flags or patches.
  - Fixed treatment of AndroidManifest.xml attributes which contained
    XML escaping
  - Retire experimental blaze flag
    experimental_link_compile_output_separately. The same behavior is
    available through the feature dynamic_link_test_srcs.
  - --incompatible_load_java_rules_from_bzl was added to forbid
    loading the native java rules directly. See more on tracking
    issue #8746
  - Turn on --experimental_build_setting_api by default for starlark
    build settings (see
    https://docs.bazel.build/versions/master/skylark/config.html#user-
    defined-build-settings for more info)
  - Attribute names are going to be restricted and must be
    syntactically valid identifiers.
    #6437
  - rule_test: fix Bazel 0.27 regression ("tags" attribute was
    ingored, #8723

This release contains contributions from many people at Google, as well as Ben Diuguid, Benjamin Peterson, Dave Lee, Loo Rong Jie, Mark Butcher, Marwan Tammam, Pedro Alvarez.
vladmos pushed a commit to bazelbuild/buildtools that referenced this issue Jul 30, 2019
This adds support for `--incompatible_restrict_string_escapes` bazelbuild/bazel#8380
@meteorcloudy
Copy link
Member

Is buildifier updated for this incompatible change?

@vladmos
Copy link
Member

vladmos commented Aug 8, 2019

Buildifier support is on the way: bazelbuild/buildtools#694

@vladmos
Copy link
Member

vladmos commented Nov 22, 2019

Buildifier supports the transformation: buildifier --lint=fix --warnings=string-escape path/to/BUILD

@laurentlb
Copy link
Contributor Author

laurentlb commented Nov 22, 2019

When is it going to be enabled by default?
(for buildifier path/to/BUILD)

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
@philwo
Copy link
Member

philwo commented Nov 18, 2020

Closing, because this flag has been flipped in 73402fa.

@philwo philwo closed this as completed Nov 18, 2020
girving added a commit to girving/pentago that referenced this issue Apr 3, 2021
bazel-io pushed a commit that referenced this issue Oct 21, 2021
This allows us to simplify the code and remove the options from the lexer and parser.

#8380

PiperOrigin-RevId: 404799093
siberex added a commit to siberex/bazel-sonarqube that referenced this issue Dec 30, 2021
ddl-jbrown pushed a commit to cerebrotech/bazel-sonarqube that referenced this issue Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

8 participants