What option/entry-point should ansible-lint use for the format/rewrite/transforms feature?

[cognifloyd]

Overview

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.

NB: This is and must be opt-in. ansible-lint will not modify any files unless you ask it to.

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 many of the yamllint-identified issues) requires transforming the files in some way. #1828 only implements the read+write part, leaving the actual rule transforms for future PRs.

Before #1828 can be finalized (which will allow me to work on other PRs), we have to decide what interface to use for these features.

UX Proposals

NB: I would have merged some cells in these charts, but markdown doesn't support that, so I duplicated the contents. 🤷‍♂️

Each of the comments/charts is a UX propsoal.

Please

• vote for the option you like best, or
• comment on them to discuss pros/cons of the different proposals, or
• comment to add another option (I can help create a chart to map out how it might work).

In particular: Is it better to use ansible-lint --<something> or a separate entry point like ansible-<something>? And then, what should that <something> be?

Vote Tally

Current winner marked with 🏆.

• --fmt-yaml and --transform options
  • 👍 1
  • 👎 1
• --fmt and --transform options
  • 👍 0
  • 👎 1
• 🏆 --write option
  • 👍 2
  • 👎 0
• ansible-fmt entry point
  • 👍 0
  • 👎 3
• some other entry point (no names proposed yet, so this is excluded)
  • 👍 2
  • 👎 0

edit: I had all the propsals in the top-level description. I moved them to comments so each proposal can have it's own thread of conversation.

edit2: I tried to quote all the UX-relevant discussion from #1828. Sorry if I missed something.

[cognifloyd]

--fmt-yaml and --transform options

		today	#1828	possible future PR	future PR(s)
Feature \ CLI		ansible-lint	ansile-lint --fmt-yaml	ansible-lint --fmt-yaml --skip-lint	ansile-lint --transform [--fmt-yaml]
Lint ansible content	YAML	✔️	✔️		✔️ *
Lint ansible content	Jinja2	✔️	✔️		✔️ *
Reformat files	YAML		✔️	✔️	✅ ^ +
Reformat files	Jinja2				✅ ^ -
Transform files (fix or simplify fixing lint issues)	YAML				✔️
Transform files (fix or simplify fixing lint issues)	Jinja2				✔️

• * Transforms require lint results to identify what to fix

• ^ Transforms must reformat any transformed files

• + Reformatting YAML files without lint errors is optional

• - Reformatting JInja Templates can be lossy with whitespace because AST does not preserve chomped whitespace (like {%- or -%} or {{- or -}}). So, only Jinja templates with identified issues get reformatted.

[cognifloyd]

@felixfontein said he prefers --fmt-yaml over --fmt in #1828 (comment)

--fmt-yaml sounds good to me. (No idea what the others think though :) )

[cognifloyd]

@ssbarnea said he did not like this option in #1828 (comment):

I do not want to use file format in command name because in the future we may want reformatting for other file formats, and we already have a cluttered list of cli options.

[cognifloyd]

--fmt and --transform options

This is the same as --fmt-yaml, but replace --fmt-yaml with --fmt.

[cognifloyd]

ansible-lint --fmt suggests that it could reformat all files, when it only really reformats yaml files. That's where the --fmt-yaml option came from: to clarify that this only reformats yaml files.

[cognifloyd]

--write option

		today	#1828	future PR(s)
Feature \ CLI		ansible-lint	ansile-lint --write	ansile-lint --write
Lint ansible content	YAML	✔️	✔️	✔️ *
Lint ansible content	Jinja2	✔️	✔️	✔️ *
Reformat files	YAML		✔️	✅ ^ ⚠️
Reformat files	Jinja2			✅ ^ -
Transform files (fix or simplify fixing lint issues)	YAML			✔️
Transform files (fix or simplify fixing lint issues)	Jinja2			✔️

• * Transforms require lint results to identify what to fix

• ^ Transforms must reformat any transformed files

• ⚠️ Reformatting YAML files without lint errors is NOT optional. --write will always rewrite all YAML files even if no lint errors are detected.

• - Only Jinja templates with identified lint errors get reformatted.

[cognifloyd]

--write was suggested by @ssbarnea in #1828 (comment)

I investigated the list of already existing command line arguments and I think we should use --write. Prettier has the same option, alongside the short version but we cannot use that.

[cognifloyd]

@lilatomic said he liked --write in #1828 (comment):

--write seems like a good option, but I'm not so sure of how it combines with those considerations: --write-list doesn't make sense to me, but --write-format seems ok. I think --transform-list is a good substitute for --write-list.

I can't really come up with anything better. --fix seems too glib, and my personal vote for --remediate is definitely bad.

[cognifloyd]

I asked

Are you saying --write would replace just --fmt (--fmt-yaml in the chart)? Or both --fmt and --transform?

@ssbarnea replid in #1828 (comment):

IMHO, --write is all we need [...]
Reformatting and fixing is not a black and white area, many things could be between them.

[cognifloyd]

ansible-fmt entry point

		today	#1828	future PR(s)
Feature \ CLI		ansible-lint	ansile-fmt	ansile-fmt
Lint ansible content	YAML	✔️		✔️ *
Lint ansible content	Jinja2	✔️		✔️ *
Reformat files	YAML		✔️	✅ ^ ⚠️
Reformat files	Jinja2			✅ ^ -
Transform files (fix or simplify fixing lint issues)	YAML			✔️
Transform files (fix or simplify fixing lint issues)	Jinja2			✔️

• * Transforms require lint results to identify what to fix. ansible-fmt could skip reporting the issues however. To find out what's left, run ansible-lint separately.

• ^ Transforms must reformat any transformed files

• ⚠️ Reformatting YAML files without lint errors is NOT optional. --write will always rewrite all YAML files even if no lint errors are detected.

• - Only Jinja templates with identified lint errors get reformatted.

[cognifloyd]

@felixfontein said he didn't like ansible-fmt in #1828 (comment)

I would expect ansible-fmt to be a "beautifier" which uses best practices for both the YAML and the included Jinja2 expressions. That's why I would avoid using this name for now.

[cognifloyd]

@emptywee said:

Did you consider extracting this new formatting/transforming functionality into a new entity? so it's not a part of the linter?
ansible-fmt doesn't really reflect the purpose of the transformer/autofixer you are working on
maybe something like ansible-transform will do
same code base, but its own entry point and flags

[cognifloyd]

@webknjaz prefers a separate entry-point. He said in #1828 (comment):

I'd prefer a separate entry point because disguising a formatted under a callable named lint is rather misleading from the UX perspective. Also, what if there's a "--fix-but-not-all" which requires specifying even more args related strictly to formatting. The more it grows, the more complicated it becomes to use and understand.

[cognifloyd]

So, several people don't like ansible-fmt, but they like the idea of something separate from ansible-lint. Any suggestions on what that other entry point might be named? I'm coming up blank.

[cognifloyd]

@bcoca said in #1828 (comment):

for ux, --conform-to <profile> might be my preferred, assumes linting profiles and if no profile, use default linting profile/rules.

I think we could extend the --write option (in future PRs) with this so that it would be --write --conform-to <profile>. This could allow a selection of different rule transforms which would facilitate marking a set of rule transforms as relevant for upgrading to a particular version of ansible. ie make it easy to "run" the upgrade guides on ansible content.