adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.83k stars 269 forks source link

Wrong removal of empty lines before comments following empty or flow elements #603

Closed juanvalino closed 10 months ago

juanvalino commented 10 months ago

Seems that fix mode incorrectly removes empty lines before comments following empty or flow elements. This leads to inconsistent results and to the modification of yaml files correctly formatted.

Suppose the following input yaml file:

---
# Comment 1

variable1: "value1"

# Comment 2

variable2: 2

# Comment 3

variable3:

# Comment 4

variable4:
  - value4

# Comment 5

variable5: {}

# Comment 6

variable6:
  "6:": 6

# Comment 7

variable7: []

# Comment 8

variable8:
  - "8": 8

# Comment 9

variable9:

# Comment 10

variable10:
  "10":
    -10

# Comment 11

variable11: {"11": 11}

# Comment 12

variable12: >-
  12

# Comment 13

variable13: ["13"]

# Comment 14

variable14: |
  14

After applying ansible-lint --fix we get the following result:

---
# Comment 1

variable1: value1

# Comment 2

variable2: 2

# Comment 3

variable3:
# Comment 4                 <--- Previous empty line incorrectly removed

variable4:
  - value4

# Comment 5

variable5: {}
# Comment 6                 <--- Previous empty line incorrectly removed

variable6:
  "6:": 6

# Comment 7

variable7: []
# Comment 8                 <--- Previous empty line incorrectly removed

variable8:
  - "8": 8

# Comment 9

variable9:
# Comment 10                 <--- Previous empty line incorrectly removed

variable10:
  "10": -10

# Comment 11

variable11: { "11": 11 }
# Comment 12                 <--- Previous empty line incorrectly removed

variable12: >-
  12

# Comment 13

variable13: ["13"]
# Comment 14                 <--- Previous empty line incorrectly removed

variable14: |
  14

Look how some empty lines before comments following empty or flow elements got removed.

juanvalino commented 10 months ago

Seems that the problem is in the write_comment() method of yaml_utils.py file, but I don't have enough skills of ruamel.yaml to bring you a fix.

juanvalino commented 10 months ago

Doing some debug seems that the code enters in this if after and empty or flow element, and seems it shouldn't:

        if (
            pre
            and not value.strip()
            and not isinstance(
                self.event,
                (
                    ruamel.yaml.events.CollectionEndEvent,
                    ruamel.yaml.events.DocumentEndEvent,
                    ruamel.yaml.events.StreamEndEvent,
                    ruamel.yaml.events.MappingStartEvent,
                ),
            )
        ):
            # drop pure whitespace pre comments
            # does not apply to End events since they consume one of the newlines.
            value = ""
andrewimeson commented 10 months ago

I think this issue belongs to the ansible-lint repository, not to yamllint.

juanvalino commented 10 months ago

I filled the issue in ansible-lint repository: https://github.com/ansible/ansible-lint/issues/3871