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

Relax the yamllint rules on spaces inside braces for flow mappings #2077

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ per-file-ignores =
src/ansiblelint/rules/*.py: D100 D101 D102
src/ansiblelint/rules/__init__.py: D100 D101 D102

# FIXME: C901 Function is too complex
# FIXME: refactor _defaults_from_yamllint_config using match case
# once python 3.10 is mandatory
# Ref: https://github.com/ansible/ansible-lint/pull/2077
src/ansiblelint/yaml_utils.py: C901

# FIXME: drop these once they're fixed
# Ref: https://github.com/ansible-community/ansible-lint/issues/725
test/*: D100 D101 D102
Expand Down
36 changes: 34 additions & 2 deletions src/ansiblelint/yaml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@
# you can easily change it or disable in your .yamllint file.
line-length:
max: 160
# We are adding an extra space inside braces as that's how prettier does it
# and we are trying not to fight other linters.
braces:
min-spaces-inside: 0 # yamllint defaults to 0
max-spaces-inside: 1 # yamllint defaults to 0
"""


Expand Down Expand Up @@ -532,6 +537,9 @@ class FormattedEmitter(Emitter):

preferred_quote = '"' # either " or '

min_spaces_inside = 0
max_spaces_inside = 1

_sequence_indent = 2
_sequence_dash_offset = 0 # Should be _sequence_indent - 2
_root_is_sequence = False
Expand Down Expand Up @@ -592,13 +600,20 @@ def write_indicator(
indention: bool = False, # (sic) ruamel.yaml has this typo in their API
) -> None:
"""Make sure that flow maps get whitespace by the curly braces."""
# We try to go with one whitespace by the curly braces and adjust accordingly
# to what min_spaces_inside and max_spaces_inside are set to.
# This assumes min_spaces_inside <= max_spaces_inside
spaces_inside = min(
max(1, self.min_spaces_inside),
self.max_spaces_inside if self.max_spaces_inside != -1 else 1,
)
# 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 = " }"
indicator = (" " * spaces_inside) + "}"
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.
Expand All @@ -607,7 +622,7 @@ def write_indicator(
self._in_empty_flow_map = True
else:
self.column += 1
self.stream.write(" ")
self.stream.write(" " * spaces_inside)
self._in_empty_flow_map = False

# "/n/n" results in one blank line (end the previous line, then newline).
Expand Down Expand Up @@ -786,6 +801,9 @@ def __init__(
indent_sequences: bool = cast(bool, config["indent_sequences"])
preferred_quote: str = cast(str, config["preferred_quote"]) # either ' or "

min_spaces_inside: int = cast(int, config["min_spaces_inside"])
max_spaces_inside: int = cast(int, config["max_spaces_inside"])

self.default_flow_style = False
self.compact_seq_seq = True # type: ignore[assignment] # dash after dash
self.compact_seq_map = True # type: ignore[assignment] # key after dash
Expand All @@ -804,6 +822,10 @@ def __init__(
# NB: default_style affects preferred_quote as well.
# self.default_style ∈ None (default), '', '"', "'", '|', '>'

# spaces inside braces for flow mappings
FormattedEmitter.min_spaces_inside = min_spaces_inside
FormattedEmitter.max_spaces_inside = max_spaces_inside

# We need a custom constructor to preserve Octal formatting in YAML 1.1
self.Constructor = CustomConstructor
self.Representer.add_representer(OctalIntYAML11, OctalIntYAML11.represent_octal)
Expand Down Expand Up @@ -833,18 +855,28 @@ def _defaults_from_yamllint_config() -> Dict[str, Union[bool, int, str]]:
"width": 160,
"indent_sequences": True,
"preferred_quote": '"',
"min_spaces_inside": 0,
"max_spaces_inside": 1,
}
for rule, rule_config in load_yamllint_config().rules.items():
if not rule_config:
# rule disabled
continue

# refactor this if ... elif ... elif ... else monstrosity using match/case (PEP 634) once python 3.10 is mandatory
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 == "braces":
min_spaces_inside = rule_config["min-spaces-inside"]
if min_spaces_inside:
config["min_spaces_inside"] = int(min_spaces_inside)
max_spaces_inside = rule_config["max-spaces-inside"]
if max_spaces_inside:
config["max_spaces_inside"] = int(max_spaces_inside)
elif rule == "indentation":
indent_sequences = rule_config["indent-sequences"]
# one of: bool, "whatever", "consistent"
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/formatting-after/fmt-1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ quoting:

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

# YAML 1.1 Boolean-hell: https://yaml.org/type/bool.html
booleans-true:
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/formatting-before/fmt-1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ quoting:

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

# YAML 1.1 Boolean-hell: https://yaml.org/type/bool.html
booleans-true:
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/formatting-prettier/fmt-1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ quoting:

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

# YAML 1.1 Boolean-hell: https://yaml.org/type/bool.html
booleans-true:
Expand Down
8 changes: 7 additions & 1 deletion test/test_yaml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from ruamel.yaml.comments import CommentedMap, CommentedSeq
from ruamel.yaml.emitter import Emitter
from ruamel.yaml.main import YAML
from yamllint.linter import run as run_yamllint

import ansiblelint.yaml_utils
from ansiblelint.file_utils import Lintable
Expand Down Expand Up @@ -195,7 +196,7 @@ def test_custom_ruamel_yaml_emitter(
def fixture_yaml_formatting_fixtures(fixture_filename: str) -> Tuple[str, str, str]:
"""Get the contents for the formatting fixture files.

To regenerate these fixtures, please run ``tools/generate-formatting-fixtures.py``.
To regenerate these fixtures, please run ``test/fixtures/test_regenerate_formatting_fixtures.py``.

Ideally, prettier should not have to change any ``formatting-after`` fixtures.
"""
Expand Down Expand Up @@ -248,6 +249,11 @@ def test_formatted_yaml_loader_dumper(
# Instead, `pytest --regenerate-formatting-fixtures` will fail if prettier would
# change any files in test/fixtures/formatting-after

# Running our files through yamllint, after we reformatted them,
# should not yield any problems.
config = ansiblelint.yaml_utils.load_yamllint_config()
assert not list(run_yamllint(after_content, config))


@pytest.fixture(name="lintable")
def fixture_lintable(file_path: str) -> Lintable:
Expand Down