False positive: interpreting GitHub actions "on:" as a truthy value

[jamesbraza]

Here is part of a GitHub action workflow foo.yml:

---
name: Foo

on:
  push:

Currently, yamllint==v1.26.3 invoked via pre-commit==v4.0.1 with --strict raises the following warning:

yamllint......................................................................Failed
- hook id: yamllint
- exit code: 2

.github/workflows/lint.yml
  4:1       warning  truthy value should be one of [false, true]  (truthy)

I believe yamllint's perception of on: being a truthy value is incorrect, and thus is a false positive warning.

For now, I am using this workaround:

---
name: Foo

on:  # yamllint disable-line rule:truthy
  push:

[adrienverge]

Before opening a new issue:

• Have you searched for existing issues before opening this one: Allow configuration of truthy keys #344, 'truthy' rule throws false positives on keys named on. #232, truthy rule: add check-keys option #247, false positive truthy failure on travis files #158...?
• Have you read the docs about truthy? It's very short and answers your questions: on is a truthy value, and check-keys is probably what you're looking for.

[jamesbraza]

Ha you're right, I guess it didn't even occur to me that an on key could be considered truthy, and thus this'd be a dup. Thanks for your patience :)

After reviewing all the issues and docs linked, I think the one-line disable is the most desirable.

on:  # yamllint disable-line rule:truthy

I wish that check-keys: false or allowed-values: ['true', 'false', 'on'] didn't apply to all files. By accommodating GitHub Action's Workflow choice in using the key on, I have to relax a rule for all yaml files in my repo, something I don't want to do. I think the allowed-keys proposed here #344 (comment) was a useful idea, but it still has the problem of applying to all yaml files.

Is it possible to extend a .yamllint config twice? The first extends would be making the default .yamllint config for the entire repo, and the second extends would be making an exception that applies specifically to the .github/workflows subdirectory.

[mvz]

I'm running into this problem as well. Has anyone tried to see if github actions accepts using

"on":
  push:

Update: I've tried and it seems to work fine.

[adrienverge]

In response to message (deleted in the meantime by its author named "Error 404"):

@mvz Using "on" works but I think using "foo" is not a good idea because we use a linter (in many cases) to help us to keep our code with a consistent/homogeneous style. If you will use "key": value (with quotes) in one line and uses key: value (without quotes) in other lines then your code will not have a homogeneous style.

I think we should not modify our code, not adding # yamllint disable-line rule:truth or adding "on" because the code is ok, the issue is in the linter. While the bug is fixed we could configure our workflows to let us decide if a linter warning/error is actually an error or a false positive.

As already stated, this issue is a multi-duplicate, and the described behavior is not a false positive. Again, the option check-keys is probably what you're looking for.

[mvz]

I think many people are not aware that on being truthy also applies to keys, in addition to values. I certainly wasn't.

[trallnag]

Interesting. I wonder how GitHub processes it internally. I've heard that they are using a custom YAML parser which ignores many parts of the YAML standard

[adrienverge]

1.2 has been out there for quite a while.

True.
We could also decide that from a future version (say, 1.35), yamllint would treat input files as YAML 1.2 by default (in particular, yes and on would stop triggering errors). This would be a slight breaking change, but yamllint always tried to be 1.2-compatible.
@perlpunk what do you think?

[perlpunk]

@adrienverge I wouldn't make the 1.2 behaviour the default. That could break too much I think. edit: but that's of course something you can decide better.

I am currently also thinking about how this could be configured in yamltidy, and I'm leaning towards a schema: key, that in my case (since yamltidy is new) would default to the 1.2 Core schema. But I would also like to allow additional things, for example it's possible that someone is using YAML 1.2 with the optional merge key, so yamltidy should know to not unquote a "<<" string. Not sure how the config for such extra things would look like yet.

[adrienverge]

yamltidy

That's cool!!

and I'm leaning towards a schema: key

This certainly is a good idea. In the yamllint case, it could be a configurable option in .yamllint conf, that would default to either 1.1 or 1.2 (to be decided). I propose that we use the same name (e.g. schema), for consistency between tools.

[adrienverge]

@perlpunk see #650 (for info I would find default-yaml-spec-version: 1.1 | 1.2 a good option name for yamllint configuration, I like the fact that it's explicit).