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 value is redundantly quoted with single quotes" when string contains line break and backslash #663

Open
thomasgilgenast opened this issue Feb 26, 2024 · 8 comments

Comments

@thomasgilgenast
Copy link

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
Copy link
Author

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
Copy link
Owner

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
Copy link

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.

paddyroddy added a commit to astro-informatics/sleplet that referenced this issue Mar 27, 2024
paddyroddy added a commit to astro-informatics/sleplet that referenced this issue Mar 27, 2024
paddyroddy added a commit to UCL-MIRSG/.github that referenced this issue Mar 27, 2024
paddyroddy added a commit to UCL-MIRSG/.github that referenced this issue Mar 27, 2024
@dorak88783
Copy link

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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

Thanks, created #665.

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

5 participants