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

Add support for reformatting YAML files #1943

Merged
merged 36 commits into from
Mar 5, 2022

Conversation

cognifloyd
Copy link
Contributor

@cognifloyd cognifloyd commented Feb 26, 2022

I want to introduce a feature to ansible-lint: It should be able to fix issues identified by rules. The feature in this PR is a pre-requisite to introducing that.

In order to fix any issues identified by rules, we will have to read each YAML file with ruamel.yaml and then write it out. Reading/writing YAML files with ruamel.yaml standardizes spacing and quotes while preserving comments. In other words, it does very basic YAML reformatting which might fix some of the issues identified by yamllint. Fixing most of the issues identified by ansible-lint (including at least some of the yamllint-identified issues) requires transforming the files in some way. This PR only implements the read+write part, leaving the actual rule transforms for future PRs.

To use this (opt-in) mode, run ansible-lint --write (UX discussion summarized in #1910). That makes ansible-lint pass all YAML files it checks through our customized ruamel.yaml.YAML instance.

ansible-lint --write reports a count of the number of modified files, which looks approximately like this:

Modified 42 files.
Finished with 0 failure(s), 0 warning(s), on 100 files.

Notes:

Other merged PRs prepared the basis for this one (which replaces #1828):

@cognifloyd cognifloyd requested a review from a team as a code owner February 26, 2022 18:16
@cognifloyd cognifloyd added this to the 6.0.0 milestone Feb 26, 2022
@cognifloyd cognifloyd self-assigned this Feb 26, 2022
@cognifloyd cognifloyd requested review from cidrblock, ssbarnea and tadeboro and removed request for a team February 26, 2022 18:16
@cognifloyd cognifloyd changed the title New Feature: Reformat YAML files with ruamel.yaml Add support for reformatting YAML files Feb 26, 2022
@ssbarnea
Copy link
Member

Please rebase :P

@cognifloyd
Copy link
Contributor Author

  1. If yaml is in the skip_list, then should --write throw an error? No other transforms will be able to have any affect without reformatting the yaml file. In any case, I'm looking at how to account for the skip list.
  2. Oh. I thought I had a safety on there for that issue. Looks like I missed handling that in this iteration. If the file is not a map or a list, then it should not try to write it because ruamel.yaml cannot preserve comments when the file loads as "None" (which an empty vars file will do). I'll push the fix in a bit.
  3. The empty flow issue { } vs {} is addressed in Manage YAML comment indentation #1946.

@cognifloyd
Copy link
Contributor Author

  1. If --write and yaml is in skip_list, then the Transformer no longer runs. Instead a message gets printed to explain why nothing was modified.
  2. I added the empty file safety check.

src/ansiblelint/app.py Outdated Show resolved Hide resolved
@cognifloyd cognifloyd requested a review from ssbarnea March 1, 2022 16:22
@cognifloyd cognifloyd force-pushed the transformer-write-yaml branch 4 times, most recently from 39ed67e to 6e02c03 Compare March 1, 2022 18:30
@cognifloyd cognifloyd force-pushed the transformer-write-yaml branch from 6e02c03 to 2a57c35 Compare March 2, 2022 01:01
test/test_transformer.py Outdated Show resolved Hide resolved
This was referenced Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants