Relax the yamllint rules on spaces inside braces for flow mappings

[ziegenberg]

This allows zero or one space inside braces for flow mappings.

Fixes: #2076

[ziegenberg]

I'd need a hint on where to put tests for testing this change. I see that test_formatted_yaml_loader_dumper tests for changes done by prettier vs ruamel.yaml. But after formatting, those changes are not fed into yamllint again.

[cognifloyd]

Let's see.

This is where we add the extra space:

ansible-lint/src/ansiblelint/yaml_utils.py

Lines 587 to 611 in aa305f7

	def write_indicator(
	self,
	indicator: str, # ruamel.yaml typehint is wrong. This is a string.
	need_whitespace: bool,
	whitespace: bool = False,
	indention: bool = False, # (sic) ruamel.yaml has this typo in their API
	) -> None:
	"""Make sure that flow maps get whitespace by the curly braces."""
	# If this is the end of the flow mapping that isn't on a new line:
	if (
	indicator == "}"
	and (self.column or 0) > (self.indent or 0)
	and not self._in_empty_flow_map
	):
	indicator = " }"
	super().write_indicator(indicator, need_whitespace, whitespace, indention)
	# if it is the start of a flow mapping, and it's not time
	# to wrap the lines, insert a space.
	if indicator == "{" and self.column < self.best_width:
	if self.check_empty_mapping():
	self._in_empty_flow_map = True
	else:
	self.column += 1
	self.stream.write(" ")
	self._in_empty_flow_map = False

We would need to add a case for braces here:

ansible-lint/src/ansiblelint/yaml_utils.py

Lines 828 to 861 in aa305f7

	def _defaults_from_yamllint_config() -> Dict[str, Union[bool, int, str]]:
	"""Extract FormattedYAML-relevant settings from yamllint config if possible."""
	config = {
	"explicit_start": True,
	"explicit_end": False,
	"width": 160,
	"indent_sequences": True,
	"preferred_quote": '"',
	}
	for rule, rule_config in load_yamllint_config().rules.items():
	if not rule_config:
	# rule disabled
	continue
	if rule == "document-start":
	config["explicit_start"] = rule_config["present"]
	elif rule == "document-end":
	config["explicit_end"] = rule_config["present"]
	elif rule == "line-length":
	config["width"] = rule_config["max"]
	elif rule == "indentation":
	indent_sequences = rule_config["indent-sequences"]
	# one of: bool, "whatever", "consistent"
	# so, we use True for "whatever" and "consistent"
	config["indent_sequences"] = bool(indent_sequences)
	elif rule == "quoted-strings":
	quote_type = rule_config["quote-type"]
	# one of: single, double, any
	if quote_type == "single":
	config["preferred_quote"] = "'"
	elif quote_type == "double":
	config["preferred_quote"] = '"'
	return cast(Dict[str, Union[bool, int, str]], config)

And then save the braces setting somewhere here:

ansible-lint/src/ansiblelint/yaml_utils.py

Lines 780 to 787 in aa305f7

	config = self._defaults_from_yamllint_config()
	# these settings are derived from yamllint config
	self.explicit_start: bool = config["explicit_start"] # type: ignore[assignment]
	self.explicit_end: bool = config["explicit_end"] # type: ignore[assignment]
	self.width: int = config["width"] # type: ignore[assignment]
	indent_sequences: bool = cast(bool, config["indent_sequences"])
	preferred_quote: str = cast(str, config["preferred_quote"]) # either ' or "

And pass that setting into the FormattedEmitter here ish:

ansible-lint/src/ansiblelint/yaml_utils.py

Lines 801 to 805 in aa305f7

	# ignore invalid preferred_quote setting
	if preferred_quote in ['"', "'"]:
	FormattedEmitter.preferred_quote = preferred_quote
	# NB: default_style affects preferred_quote as well.
	# self.default_style ∈ None (default), '', '"', "'", '|', '>'

So we can access it in FormattedEmitter.write_indicator (where we're adding the space) to determine whether or not to add the space.

Our default config needs to be compatible with prettier's default (add the space in flow-style maps), but I'm happy to respect the (potentially custom) yamllint config in more cases.

[cognifloyd]

This is the test for the FormattedEmitter:

ansible-lint/test/test_yaml_utils.py

Line 171 in aa305f7

def test_custom_ruamel_yaml_emitter(

So we might need additional test cases or an additional test function here to make sure we're respecting the yamllint config.

[cognifloyd]

This is the test that actually calls prettier to regenerate the fixtures:
https://github.com/ansible/ansible-lint/blob/main/test/fixtures/test_regenerate_formatting_fixtures.py

We might want to add a call in there to also run yamllint on the files and make sure our yamllint config is also compatible with prettier.

That might mean adding something to one or more of the test fixture files (or adding a new fixture file) that has flow maps with too many + too few spaces.

[cognifloyd]

Thank you for working on this!

I've been a little swamped and unable to get back to finishing the --write stuff (jinja reformatting is my next on my plate #2066, and then I can finally add rule transforms to automatically address fixing some of the issues identified by our rules).

I will review whatever you do as quickly as I can however, because I hate holding up people's PRs.

[ziegenberg]

That might mean adding something to one or more of the test fixture files (or adding a new fixture file) that has flow maps with too many + too few spaces.

There's already a case here for too few spaces and I will add one more with too many spaces.

ansible-lint/test/fixtures/formatting-before/fmt-1.yml

Lines 25 to 26 in 6516fc9

	inline-dictionary:
	- {foo: bar} # should add some spacing between curly braces and content

[ziegenberg]

@cognifloyd I got a first version that basically works. Also the tests were extended to test with yamllint after ruaml.yaml did it's thing.

What is missing? Extending FormattedEmitter.write_indicatorto honor the new settings. I'll probably get that done tomorrow.

Do you see anything completely horrible for now?

If you remove the new settings around this lines, the extended test fails.
https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/yaml_utils.py#L61-64

[ziegenberg]

@cognifloyd I think, this is now ready for review.

[ziegenberg]

I'm looking at the src/ansiblelint/yaml_utils.py:850:5: C901 'FormattedYAML._defaults_from_yamllint_config' is too complex (11) warning right now. I'll add a comment about rewriting this using match - case once py310 is mandatory. And silence the flake8 grumbling about complexity.