adrienverge / yamllint

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

truthy: Adapt forbidden values based on YAML version #650

Closed adrienverge closed 5 months ago

adrienverge commented 6 months ago

Specification of YAML ≤ 1.1 has 22 boolean values:

y     | Y            | n     | N
yes   | Yes   | YES  | no    | No    | NO
true  | True  | TRUE | false | False | FALSE
on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6 ^1:

true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of the document, let's adapt the list of forbidden values.

In the future, we should:

Solves: https://github.com/adrienverge/yamllint/issues/587

Related to: #559 #540 #430 #344 #247 #232 #158

coveralls commented 6 months ago

Coverage Status

coverage: 99.823% (+0.001%) from 99.822% when pulling 3b6a3dfa3cf5994c3bd244494589fa1c3b7246d5 on feat/truthy-depending-on-yaml-spec-version into 9931ad699ce2be71286bca0b991c8b3427ef6d40 on master.

adrienverge commented 5 months ago

@perlpunk today I attended a great talk in Brussels which said there were 22 values (not 18), so I updated the commit message:

-Specification of YAML ≤ 1.1 has 18 boolean values:
+Specification of YAML ≤ 1.1 has 22 boolean values:

Notes: - I haven't adapted the yamllint rule code, because PyYAML treats the 4 extra values y, Y, n and N as strings (so it's neither 1.1- nor 1.2-complete). - I had a hard time finding the exact list on yaml.org (it's not directly on the YAML 1.1 specification page).

fosdem2024

By the way, I read that between 1.1 and 1.2 "the merge << and value = special mapping keys have been removed" but I heard that some 1.2-compatible YAML parsing libs implement the merge marker << as an option. Do you think yamllint should complain when it finds both a tag %YAML 1.2 and a merge marker <<?

perlpunk commented 5 months ago

@adrienverge thanks very much! Also, nice to meet you :)

Yeah, I think the non-standard behaviour of PyYAML makes it a bit more difficult. And I think more libraries will implement things like << optionally. I think it's a good idea because << is well known and very useful. Of course it makes our job harder...

Regarding %YAML 1.2, I'm not sure how often it will be used in the future. It's meant for that kind of thing, but I think people might avoid it because they don't want to type it in every YAML file.

For my YAML::PP library I made an option schema, which is an array, so you can say schema => ['Core', 'Merge']) for example, so I was thinking to do something similar for yamltidy. But while Core stands for a set of tags, Merge is just one tag. In YAML::PP, both are a class, e.g. YAML::PP::Schema::Merge, that's why it works. For getting the PyYAML behaviour I would probably just add another class YAML::PP::Schema::PyYAML1_1.

So I don't have a clear idea here yet how a linter/formatter config option could look like...

The so called tag repository is kind of hidden, yeah. You can find it in the 1.1 spec under YAML tag repository. YAML 1.1 was always a draft actually, and then for 1.2 it wasn't used anymore. So then I put together this page. The data in that repository can be used as the official test suite for resolving tags. I hope we can move that into yaml-test-suite at some point.

perlpunk commented 5 months ago

...and I just realized that I forgot No in the test data (I just have NO and no), that's why https://perlpunk.github.io/yaml-test-schema/data.html only shows 21 booleans!

adrienverge commented 5 months ago

@perlpunk thank you! I'll bookmark https://perlpunk.github.io/yaml-test-schema/schemas.html as it's very handy (and I'm glad you could detect a missing entry in the test list thanks to my message :))

I agree that prepending explicit tags like %YAML 1.2 is not practical and few users would do it. I'm attracted by the idea of making yamllint 1.2-compatible by default, but that would partially defeat the purpose of some rules (e.g. octal-values or truthy which help detecting values valid in 1.1 but not in 1.2).

Anyway, I believe the change of this pull request is a good move so I'll merge it. And let's not lint << + YAML 1.2 cases since they are used in practice and quite useful.

perlpunk commented 5 months ago

@perlpunk thank you! I'll bookmark https://perlpunk.github.io/yaml-test-schema/schemas.html

Also, like I said, you can use the data https://github.com/perlpunk/yaml-test-schema/tree/master/data for your test suite. I will add the No thing ASAP :)