adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.79k stars 265 forks source link

"string value is redundantly quoted with single quotes" when string contains line break and backslash #663

Open thomasgilgenast opened 5 months ago

thomasgilgenast commented 5 months ago

With a .yamllint.yml like

---

extends: default

rules:
  quoted-strings:
    quote-type: double
    required: only-when-needed

and a test.yaml like

---

key: "foo/bar/\
   baz/qux"

I get the desired behavior of the backslash within double quotes

$ yq .key test.yaml
"foo/bar/baz/qux"

but yamllint thinks the quotes are redundant:

$ yamllint test.yaml
test.yaml
  3:6       error    string value is redundantly quoted with double quotes  (quoted-strings)

The quotes are not redundant - removing them changes the value of the string. After removing the quotes:

$ yq .key test.yaml
"foo/bar/\\ baz/qux"

or removing both the quotes and the backslash

$ yq .key test.yaml
"foo/bar/ baz/qux"

Is there a way to configure yamllint to allow quotes when they are not redundant, but not allow them when they are redundant?

thomasgilgenast commented 5 months ago

I realized after opening this that it should theoretically be possible to use extra-allowed to allow strings that match the regex ^.*\\\n.*$ (or something similar to this) to be quoted. I could not figure out how to escape this properly though.

extra-allowed: ["\\"] or extra-allowed: ['\'] throw

re.error: bad escape (end of pattern) at position 0

extra-allowed: ["\\\\"] or extra-allowed: ['\\'] still throw

  3:6       error    string value is redundantly quoted with double quotes  (quoted-strings)

Trying to match the newline with single or double quoted \n with any number of backslashes also never succeeds. So my suspicion is that the regex matching happens after the string has been parsed from the YAML file, at which point the string's value is "foo/bar/baz/qux" and there is no remaining hint of a newline or backslash.

adrienverge commented 5 months ago

Hello Thomas and thanks for the report.

This indeed looks like a bug with option only-when-needed (but I don't think using extra-allowed is a good hack to solve this).

A fix in yamllint/rules/quoted_strings.py and a non-regression test tests/rules/test_quoted_strings.py would be welcome!

@ruipinge you contributed required: only-when-needed in #235, would you like to have a look into this?

paddyroddy commented 4 months ago

I think similar to this I have:

steps:
  - name: Coveralls Finished
    # yamllint disable-line rule:line-length
    uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 # v2
    with:
      parallel-finished: true
      carryforward: "\
        run-macos-latest-3.10,\
        run-ubuntu-latest-3.10,\
        run-macos-latest-3.11,\
        run-ubuntu-latest-3.11"

I'm getting string value is redundantly quoted with double quotes. But if one removes the quotes than the carryforward bit is like

carryforward: "\ run-macos-latest-3.10,\ run-ubuntu-latest-3.10,\ run-macos-latest-3.11,\ run-ubuntu-latest-3.11"

which won't work as intended.

dorak88783 commented 4 months ago

Slightly similar, I have a dependabot.yml file with

    schedule:
      interval: daily
      time: "06:00"

where the quotes around the time value are really needed. Yet yamllint suggests that they are redundant.

andrewimeson commented 4 months ago
time: "06:00"

and

time: 06:00

are parsed identically in both pyyaml and yq (Go). Are you Sure that Dependabot needs it quoted?

dorak88783 commented 4 months ago

Are you Sure that Dependabot needs it quoted?

Thanks for your quick reply!

Well, now I'm not sure if it's YAML in general or Dependabot-specific. Also I'm not familiar with all different types of YAML specification, so I couldn't find a hard rule on this. Certainly, when I use the version without quotes in my dependabot config, I get the error

The property '#/updates/0/schedule/time' of type integer did not match the following type: string

The docs are not completely specific on the expected type (https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#scheduletime). The example there also quote other strings that can be unquoted.

andrewimeson commented 4 months ago

@dorak88783 actually, it looks like with the YAML 1.1 time: 06:00 can be interpreted as as a base 60 integer. I think that should be a new issue though, would you create one?

dorak88783 commented 3 months ago

Thanks, created https://github.com/adrienverge/yamllint/issues/665.