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

ansible-lint should be able to generate comments that match it's own rules #3890

Closed
tremble opened this issue Nov 10, 2023 · 4 comments · Fixed by #4139
Closed

ansible-lint should be able to generate comments that match it's own rules #3890

tremble opened this issue Nov 10, 2023 · 4 comments · Fixed by #4139
Assignees
Labels

Comments

@tremble
Copy link

tremble commented Nov 10, 2023

Summary

This is potentially better reported as 2 issues, and I'll happily split it, but the two are tightly coupled...

As mentioned in #3839 the default behaviour of ansible-lint --fix results in comments preceded with just a single space, while the linter wants 2. This in turn means that while fixing one issue it causes a new issue. What adds to the confusion is that the run which re-writes the yaml returns success and 0 errors, while a second run returns the violation.

While I understand the desire to support compatibility with prettier, this results in broken default behaviour. Which is very confusing for the user.

Either:

  • There should be a mechanism so that --fix adds 1 or 2 spaces based on the user's preference, preferably with a default that matches the default linting behaviour
  • Or the default behaviour of ansible-lint should be updated to allow for single spaces,

However, at the very least ansible-lint should be honestly report that the file it generated does not meet its own rules.

Issue Type
  • Bug Report
OS / ENVIRONMENT
ansible-lint --version
ansible-lint 6.22.0 using ansible-core:2.15.5rc1.post0 ansible-compat:4.1.10 ruamel-yaml:0.18.5 ruamel-yaml-clib:0.2.7

ansible installed with git
ansible-lint installed using pip

STEPS TO REPRODUCE
$ cat example.yml 
---
- name: Example task with comment
  assert:
    that:
      - true  # Hello world


$ ansible-lint --fix example.yml
$ ansible-lint --fix example.yml
Desired Behavior

Output of second ansible-lint call should result in 0 errors and 0 changes

Actual Behavior

Output of first ansible-lint --fix call reports 0 errors, but 1 change

Modified 1 files.
Passed: 0 failure(s), 0 warning(s) on 1 files. Last profile that met the validation criteria was 'production'.

Re-running ansible-lint --fix reports 1 violation and 0 changes:

WARNING  Listing 1 violation(s) that are fatal
yaml[comments]: Too few spaces before comment
example.yml:5

Read documentation for instructions on how to ignore specific rule violations.

              Rule Violation Summary               
 count tag            profile rule associated tags 
     1 yaml[comments] basic   formatting, yaml     
@tremble tremble added bug new Triage required labels Nov 10, 2023
@priyamsahoo priyamsahoo removed the new Triage required label Nov 22, 2023
@ssbarnea
Copy link
Member

This is more of a missing feature as in ability to use the value configured inside .yamllint config file. This bug can be reproduced only with a custom config file like this:

---
rules:
  comments:
    min-spaces-from-content: 2

A PR to improve the linter formatting ability would be welcomed here, but I do not think that the team will have the time to implement this change.

@tremble
Copy link
Author

tremble commented Nov 22, 2023

This is more of a missing feature as in ability to use the value configured inside .yamllint config file. This bug can be reproduced only with a custom config file like this:

My .yamllint file doesn't mention min-spaces-from-content:

---
rules:
  indentation:
    ignore: &default_ignores |
      # automatically generated, we can't control it
      changelogs/changelog.yaml
      # Will be gone when we release and automatically reformatted
      changelogs/fragments/*
  document-start:
    ignore: *default_ignores
  line-length:
    ignore: *default_ignores
    max: 160 

ignore-from-file: .gitignore

It's possible I've got another config file around somewhere, but I'm not sure where

yamllint                1.29.0

@raphendyr
Copy link

I noticed that the ansible-lint includes it's own yamllint config. The min-space-from-content: 1 comes from it: https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/yaml_utils.py#L49-L70

@ssbarnea
Copy link
Member

ssbarnea commented May 6, 2024

@Qalthos Please implement a change that disables the fix option when min-spaces-from-content is overridden in yamllint config, as we do not support this. We will later add the other 2-3 formatting options that we do not support for various reasons like ruamel and prettier limitations.

Related #4118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants