From 013ed7760174c8aed57bd391df203a1ea87417f5 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 00:08:10 +0100 Subject: [PATCH 01/11] test(yaml): different and missing values --- docs/ideas/yaml/jmespath-on-section.toml | 2 + docs/ideas/yaml/jmespath-table.toml | 2 + docs/plugins.rst | 7 ++ poetry.lock | 6 +- pyproject.toml | 3 +- src/nitpick/constants.py | 2 +- src/nitpick/formats.py | 41 ++++++++--- src/nitpick/plugins/pre_commit.py | 1 + src/nitpick/plugins/text.py | 3 +- src/nitpick/plugins/yaml.py | 93 ++++++++++++++++++++++++ src/nitpick/style/core.py | 2 +- tests/helpers.py | 37 ++++++---- tests/test_toml.py | 2 +- tests/test_yaml.py | 73 +++++++++++++++++++ tests/test_yaml/actual.yaml | 7 ++ tests/test_yaml/expected.yaml | 16 ++++ tests/test_yaml/style.toml | 18 +++++ 17 files changed, 284 insertions(+), 31 deletions(-) create mode 100644 src/nitpick/plugins/yaml.py create mode 100644 tests/test_yaml.py create mode 100644 tests/test_yaml/actual.yaml create mode 100644 tests/test_yaml/expected.yaml create mode 100644 tests/test_yaml/style.toml diff --git a/docs/ideas/yaml/jmespath-on-section.toml b/docs/ideas/yaml/jmespath-on-section.toml index 0876ec60..e1f8be16 100644 --- a/docs/ideas/yaml/jmespath-on-section.toml +++ b/docs/ideas/yaml/jmespath-on-section.toml @@ -23,3 +23,5 @@ __jinja = false # 6. Another way to turn off Jinja for a specific key only, not the whole dict # (using the "__" syntax from Django filters, SQLAlchemy, factoryboy...) "runs-on__no_jinja" = "${{ matrix.os }}" + +# FIXME: create a style from this diff --git a/docs/ideas/yaml/jmespath-table.toml b/docs/ideas/yaml/jmespath-table.toml index cc74ce99..43a1ebf7 100644 --- a/docs/ideas/yaml/jmespath-table.toml +++ b/docs/ideas/yaml/jmespath-table.toml @@ -23,3 +23,5 @@ with = {"python-version" = "${{ matrix.python-version }}"} name__jinja = "Set up Python ${{ matrix.python-version }}" name__no_jinja = "Set up Python ${{ matrix.python-version }}" name__jinja_off = "Set up Python ${{ matrix.python-version }}" + +# FIXME: create a style from this diff --git a/docs/plugins.rst b/docs/plugins.rst index 132fdc6c..679ebcd7 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -78,3 +78,10 @@ See also `the [tool.poetry] section of the pyproject.toml file Style example: :ref:`Python 3.8 version constraint `. There are :ref:`many other examples here `. + +.. _yamlplugin: + +YAML files +---------- + +Enforce config on YAML files. diff --git a/poetry.lock b/poetry.lock index c251c594..1d477d4a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -758,7 +758,7 @@ python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" [[package]] name = "pygments" -version = "2.10.0" +version = "2.11.0" description = "Pygments is a syntax highlighting package written in Python." category = "main" optional = false @@ -1728,8 +1728,8 @@ pyflakes = [ {file = "pyflakes-2.4.0.tar.gz", hash = "sha256:05a85c2872edf37a4ed30b0cce2f6093e1d0581f8c19d7393122da7e25b2b24c"}, ] pygments = [ - {file = "Pygments-2.10.0-py3-none-any.whl", hash = "sha256:b8e67fe6af78f492b3c4b3e2970c0624cbf08beb1e493b2c99b9fa1b67a20380"}, - {file = "Pygments-2.10.0.tar.gz", hash = "sha256:f398865f7eb6874156579fdf36bc840a03cab64d1cde9e93d68f46a425ec52c6"}, + {file = "Pygments-2.11.0-py3-none-any.whl", hash = "sha256:ac8098bfc40b8e1091ad7c13490c7f4797e401d0972e8fcfadde90ffb3ed4ea9"}, + {file = "Pygments-2.11.0.tar.gz", hash = "sha256:51130f778a028f2d19c143fce00ced6f8b10f726e17599d7e91b290f6cbcda0c"}, ] pylint = [ {file = "pylint-2.12.0-py3-none-any.whl", hash = "sha256:ba00afcb1550bc217bbcb0eb76c10cb8335f7417a3323bdd980c29fb5b59f8d2"}, diff --git a/pyproject.toml b/pyproject.toml index cbb40d38..f79d440b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,9 +38,10 @@ NIP = "nitpick.flake8:NitpickFlake8Extension" [tool.poetry.plugins.nitpick] text = "nitpick.plugins.text" json = "nitpick.plugins.json" -pre_commit = "nitpick.plugins.pre_commit" +pre_commit = "nitpick.plugins.pre_commit" # FIXME: remove this when removing the plugin class ini = "nitpick.plugins.ini" toml = "nitpick.plugins.toml" +yaml = "nitpick.plugins.yaml" [tool.poetry.dependencies] python = "^3.6.1" diff --git a/src/nitpick/constants.py b/src/nitpick/constants.py index aa4b64da..d54fcbd7 100644 --- a/src/nitpick/constants.py +++ b/src/nitpick/constants.py @@ -56,7 +56,7 @@ DOUBLE_QUOTE = '"' #: Special unique separator for :py:meth:`flatten()` and :py:meth:`unflatten()`, -# to avoid collision with existing key values (e.g. the default dot separator "." can be part of a pyproject.toml key). +# to avoid collision with existing key values (e.g. the default dot separator "." can be part of a TOML key). SEPARATOR_FLATTEN = "$#@" #: Special unique separator for :py:meth:`nitpick.generic.quoted_split()`. diff --git a/src/nitpick/formats.py b/src/nitpick/formats.py index af223429..3f026842 100644 --- a/src/nitpick/formats.py +++ b/src/nitpick/formats.py @@ -1,6 +1,5 @@ """Configuration file formats.""" import abc -import io import json from collections import OrderedDict from pathlib import Path @@ -10,7 +9,7 @@ import toml from autorepr import autorepr from loguru import logger -from ruamel.yaml import YAML, RoundTripRepresenter +from ruamel.yaml import YAML, RoundTripRepresenter, StringIO from ruamel.yaml.comments import CommentedMap, CommentedSeq from sortedcontainers import SortedDict @@ -234,30 +233,51 @@ def load(self) -> bool: return True +class SensibleYAML(YAML): + """YAML with sensible defaults but an inefficient dump to string. + + `Output of dump() as a string + `_ + """ + + def __init__(self, *, typ=None, pure=False, output=None, plug_ins=None): + super().__init__(typ=typ, pure=pure, output=output, plug_ins=plug_ins) + self.map_indent = 2 + self.sequence_indent = 4 + self.sequence_dash_offset = 2 + + def loads(self, string: str): + """Load YAML from a string... that unusual use case in a world of files only.""" + return self.load(StringIO(string)) + + def dumps(self, data) -> str: + """Dump to a string... who would want such a thing? One can dump to a file or stdout.""" + output = StringIO() + self.dump(data, output, transform=None) + return output.getvalue() + + class YamlFormat(BaseFormat): """YAML configuration format.""" + document: SensibleYAML + def load(self) -> bool: """Load a YAML file by its path, a string or a dict.""" if self._loaded: return False - yaml = YAML() - yaml.map_indent = 2 - yaml.sequence_indent = 4 - yaml.sequence_dash_offset = 2 + self.document = SensibleYAML() if self.path is not None: self._string = Path(self.path).read_text(encoding="UTF-8") if self._string is not None: - self._object = yaml.load(io.StringIO(self._string)) + self._object = self.document.loads(self._string) if self._object is not None: if isinstance(self._object, BaseFormat): self._reformatted = self._object.reformatted else: - output = io.StringIO() - yaml.dump(self._object, output) - self._reformatted = output.getvalue() + self._reformatted = self.document.dumps(self._object) self._loaded = True return True @@ -279,6 +299,7 @@ def cleanup(cls, *args: List[Any]) -> List[Any]: RoundTripRepresenter.add_representer(SortedDict, RoundTripRepresenter.represent_dict) +RoundTripRepresenter.add_representer(OrderedDict, RoundTripRepresenter.represent_dict) class JsonFormat(BaseFormat): diff --git a/src/nitpick/plugins/pre_commit.py b/src/nitpick/plugins/pre_commit.py index 89d342cd..a582fe9c 100644 --- a/src/nitpick/plugins/pre_commit.py +++ b/src/nitpick/plugins/pre_commit.py @@ -208,6 +208,7 @@ def plugin_class() -> Type["NitpickPlugin"]: @hookimpl def can_handle(info: FileInfo) -> Optional[Type["NitpickPlugin"]]: """Handle pre-commit config file.""" + # FIXME: the YAML plugin should handle this file, now or later if info.path_from_root == PRE_COMMIT_CONFIG_YAML: return PreCommitPlugin return None diff --git a/src/nitpick/plugins/text.py b/src/nitpick/plugins/text.py index fd996135..98eeb598 100644 --- a/src/nitpick/plugins/text.py +++ b/src/nitpick/plugins/text.py @@ -12,6 +12,7 @@ from nitpick.violations import Fuss, ViolationEnum TEXT_FILE_RTFD_PAGE = "plugins.html#text-files" +KEY_CONTAINS = "contains" class TextItemSchema(Schema): @@ -58,7 +59,7 @@ class TextPlugin(NitpickPlugin): violation_base_code = 350 def _expected_lines(self): - return [obj.get("line") for obj in self.expected_config.get("contains", {})] + return [obj.get("line") for obj in self.expected_config.get(KEY_CONTAINS, {})] @property def initial_contents(self) -> str: diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py new file mode 100644 index 00000000..6305a84b --- /dev/null +++ b/src/nitpick/plugins/yaml.py @@ -0,0 +1,93 @@ +"""YAML files.""" +from itertools import chain +from typing import Iterator, Optional, Type + +from ruamel.yaml import YAML + +from nitpick.constants import PRE_COMMIT_CONFIG_YAML +from nitpick.formats import BaseFormat, YamlFormat +from nitpick.plugins import hookimpl +from nitpick.plugins.base import NitpickPlugin +from nitpick.plugins.info import FileInfo +from nitpick.plugins.text import KEY_CONTAINS +from nitpick.typedefs import YamlObject +from nitpick.violations import Fuss, SharedViolations, ViolationEnum + + +def change_yaml(document: YAML, dictionary: YamlObject): + """Traverse a YAML document recursively and change values, keeping its formatting and comments.""" + del document + del dictionary + # FIXME: + # for key, value in dictionary.items(): + # if isinstance(value, (dict, OrderedDict)): + # if key in document: + # change_toml(document[key], value) + # else: + # document.add(key, value) + # else: + # document[key] = value + + +class YamlPlugin(NitpickPlugin): + """Enforce config on YAML files.""" + + identify_tags = {"yaml"} + violation_base_code = 360 + fixable = False # FIXME: + + def enforce_rules(self) -> Iterator[Fuss]: + """Enforce rules for missing data in the YAML file.""" + if KEY_CONTAINS in self.expected_config: + # If the expected configuration has this key, it means that this config is being handled by TextPlugin. + # TODO: A YAML file that has a "contains" key on its root cannot be handled as YAML... how to fix this? + return + + yaml_format = YamlFormat(path=self.file_path) + comparison = yaml_format.compare_with_flatten(self.expected_config) # FIXME: + if not comparison.has_changes: + return + + document = yaml_format.document if self.autofix else None + yield from chain( + self.report(SharedViolations.DIFFERENT_VALUES, document, comparison.diff), + self.report(SharedViolations.MISSING_VALUES, document, comparison.missing), + ) + if self.autofix and self.dirty and document: + document.dump(document, self.file_path) + + def report(self, violation: ViolationEnum, document: Optional[YAML], change: Optional[BaseFormat]): + """Report a violation while optionally modifying the YAML document.""" + if not change: + return + if document: + change_yaml(document, change.as_object) + self.dirty = True + yield self.reporter.make_fuss(violation, change.reformatted.strip(), prefix="", fixed=self.autofix) + + @property + def initial_contents(self) -> str: + """Suggest the initial content for this missing file.""" + return "" # FIXME: + # yaml_as_string = YamlFormat(obj=self.expected_config).reformatted + # if self.autofix: + # self.file_path.write_text(yaml_as_string) + # return yaml_as_string + + +@hookimpl +def plugin_class() -> Type["NitpickPlugin"]: + """Handle YAML files.""" + return YamlPlugin + + +@hookimpl +def can_handle(info: FileInfo) -> Optional[Type["NitpickPlugin"]]: + """Handle YAML files.""" + if info.path_from_root == PRE_COMMIT_CONFIG_YAML: + # TODO: For now, this plugin won't touch the current pre-commit config + return None + + if YamlPlugin.identify_tags & info.tags: + return YamlPlugin + return None diff --git a/src/nitpick/style/core.py b/src/nitpick/style/core.py index cd0be9b7..9bae5b59 100644 --- a/src/nitpick/style/core.py +++ b/src/nitpick/style/core.py @@ -215,7 +215,7 @@ def file_field_pair(filename: str, base_file_class: Type[NitpickPlugin]) -> Dict if base_file_class.validation_schema: file_field = fields.Nested(base_file_class.validation_schema, **kwargs) else: - # For some files (e.g.: pyproject.toml, INI files), there is no strict schema; + # For some files (e.g.: TOML/ INI files), there is no strict schema; # it can be anything they allow. # It's out of Nitpick's scope to validate those files. file_field = fields.Dict(fields.String, **kwargs) diff --git a/tests/helpers.py b/tests/helpers.py index 8e070410..0c7922d6 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -48,6 +48,17 @@ def assert_conditions(*args): raise AssertionError() +def from_path_or_str(file_contents: PathOrStr): + """Read file contents from a Path or string.""" + if file_contents is None: + raise RuntimeError("No path and no file contents.") + + if isinstance(file_contents, Path): + return file_contents.read_text() + + return file_contents + + class ProjectMock: """A mocked Python project to help on tests.""" @@ -171,7 +182,7 @@ def read_multiple_files(self, filenames: Iterable[PathOrStr]) -> Dict[PathOrStr, """Read multiple files and return a hash with filename (key) and contents (value).""" return {filename: self.read_file(filename) for filename in filenames} - def save_file(self, filename: PathOrStr, file_contents: str, lint: bool = None) -> "ProjectMock": + def save_file(self, filename: PathOrStr, file_contents: PathOrStr, lint: bool = None) -> "ProjectMock": """Save a file in the root dir with the desired contents and a new line at the end. Create the parent dirs if the file name contains a slash. @@ -188,7 +199,7 @@ def save_file(self, filename: PathOrStr, file_contents: str, lint: bool = None) path.parent.mkdir(parents=True, exist_ok=True) if lint or path.suffix == ".py": self.files_to_lint.append(path) - clean = dedent(file_contents).strip() + clean = dedent(from_path_or_str(file_contents)).strip() path.write_text(f"{clean}\n") return self @@ -196,9 +207,9 @@ def touch_file(self, filename: PathOrStr) -> "ProjectMock": """Save an empty file (like the ``touch`` command).""" return self.save_file(filename, "") - def style(self, file_contents: str) -> "ProjectMock": + def style(self, file_contents: PathOrStr) -> "ProjectMock": """Save the default style file.""" - return self.save_file(NITPICK_STYLE_TOML, file_contents) + return self.save_file(NITPICK_STYLE_TOML, from_path_or_str(file_contents)) # TODO: remove this function, don't test real styles anymore to avoid breaking tests on Renovate updates def load_styles(self, *args: PathOrStr) -> "ProjectMock": @@ -212,26 +223,26 @@ def load_styles(self, *args: PathOrStr) -> "ProjectMock": self.named_style(filename, style_path.read_text()) return self - def named_style(self, filename: PathOrStr, file_contents: str) -> "ProjectMock": + def named_style(self, filename: PathOrStr, file_contents: PathOrStr) -> "ProjectMock": """Save a style file with a name. Add the .toml extension if needed.""" - return self.save_file(self.ensure_toml_extension(filename), file_contents) + return self.save_file(self.ensure_toml_extension(filename), from_path_or_str(file_contents)) @staticmethod def ensure_toml_extension(filename: PathOrStr) -> PathOrStr: """Ensure a file name has the .toml extension.""" return filename if str(filename).endswith(".toml") else f"{filename}.toml" - def setup_cfg(self, file_contents: str) -> "ProjectMock": + def setup_cfg(self, file_contents: PathOrStr) -> "ProjectMock": """Save setup.cfg.""" - return self.save_file(SETUP_CFG, file_contents) + return self.save_file(SETUP_CFG, from_path_or_str(file_contents)) - def pyproject_toml(self, file_contents: str) -> "ProjectMock": + def pyproject_toml(self, file_contents: PathOrStr) -> "ProjectMock": """Save pyproject.toml.""" - return self.save_file(PYPROJECT_TOML, file_contents) + return self.save_file(PYPROJECT_TOML, from_path_or_str(file_contents)) - def pre_commit(self, file_contents: str) -> "ProjectMock": + def pre_commit(self, file_contents: PathOrStr) -> "ProjectMock": """Save .pre-commit-config.yaml.""" - return self.save_file(PreCommitPlugin.filename, file_contents) + return self.save_file(PreCommitPlugin.filename, from_path_or_str(file_contents)) def raise_assertion_error(self, expected_error: str, assertion_message: str = None): """Show detailed errors in case of an assertion failure.""" @@ -392,7 +403,7 @@ def assert_file_contents(self, *name_contents: Union[PathOrStr, Optional[str]]) assert len(name_contents) % 2 == 0, "Supply pairs of arguments: filename (PathOrStr) and file contents (str)" for filename, file_contents in windowed(name_contents, 2, step=2): actual = self.read_file(filename) - expected = None if file_contents is None else dedent(file_contents).lstrip() + expected = None if file_contents is None else dedent(from_path_or_str(file_contents)).lstrip() compare(actual=actual, expected=expected, prefix=f"Filename: {filename}") return self diff --git a/tests/test_toml.py b/tests/test_toml.py index 8f3579d2..432d7594 100644 --- a/tests/test_toml.py +++ b/tests/test_toml.py @@ -1,4 +1,4 @@ -"""pyproject.toml tests.""" +"""TOML tests.""" from nitpick.constants import PYPROJECT_TOML from nitpick.plugins.toml import TomlPlugin from nitpick.violations import Fuss, SharedViolations diff --git a/tests/test_yaml.py b/tests/test_yaml.py new file mode 100644 index 00000000..278fe189 --- /dev/null +++ b/tests/test_yaml.py @@ -0,0 +1,73 @@ +"""YAML tests.""" +from nitpick.plugins.yaml import YamlPlugin +from nitpick.violations import Fuss, SharedViolations +from tests.helpers import ProjectMock + + +def test_suggest_initial_contents(tmp_path): + """Suggest contents when YAML files do not exist.""" + pass # FIXME: + # filename = "my.toml" + # expected_toml = """ + # [section] + # key = "value" + # number = 10 + # """ + # ProjectMock(tmp_path).style( + # f""" + # ["{filename}".section] + # key = "value" + # number = 10 + # """ + # ).api_check_then_fix( + # Fuss( + # True, + # filename, + # SharedViolations.CREATE_FILE_WITH_SUGGESTION.code + TomlPlugin.violation_base_code, + # " was not found. Create it with this content:", + # expected_toml, + # ) + # ).assert_file_contents( + # filename, expected_toml + # ) + + +def test_missing_different_values(tmp_path, datadir): + """Test different and missing values on any YAML.""" + filename = "actual.yaml" + ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "style.toml").api_check_then_fix( + Fuss( + False, + filename, + YamlPlugin.violation_base_code + SharedViolations.DIFFERENT_VALUES.code, + " has different values. Use this:", + """ + python: + install: + - extra_requirements: + - item1 + - item2 + - item3 + version: '3.9' + """, + ), + Fuss( + False, + filename, + YamlPlugin.violation_base_code + SharedViolations.MISSING_VALUES.code, + " has missing values:", + """ + root_key: + a_dict: + - a: string value + - b: 2 + - c: '3.1' + a_nested: + int: 10 + list: + - 0 + - 1 + - 2 + """, + ), + ) # FIXME: .assert_file_contents(filename, datadir / "expected.yaml") diff --git a/tests/test_yaml/actual.yaml b/tests/test_yaml/actual.yaml new file mode 100644 index 00000000..13be98c0 --- /dev/null +++ b/tests/test_yaml/actual.yaml @@ -0,0 +1,7 @@ +python: + version: 3.6 + install: + - method: pip + path: . + extra_requirements: + - doc diff --git a/tests/test_yaml/expected.yaml b/tests/test_yaml/expected.yaml new file mode 100644 index 00000000..3e973fae --- /dev/null +++ b/tests/test_yaml/expected.yaml @@ -0,0 +1,16 @@ +python: + version: 3.9 + install: + - method: pip + path: . + extra_requirements: + - doc +root_key: + a_nested: + list: [0, 1, 2] + int: 10 + a_dict: + - a: "string value" + - b: 2 + # This unquoted float will become "3.1" + - c: 3.10 diff --git a/tests/test_yaml/style.toml b/tests/test_yaml/style.toml new file mode 100644 index 00000000..51e4770a --- /dev/null +++ b/tests/test_yaml/style.toml @@ -0,0 +1,18 @@ +["actual.yaml".python] +version = "3.9" + +[["actual.yaml".python.install]] +extra_requirements = ["item1", "item2", "item3"] + +[["actual.yaml".root_key.a_dict]] +a = "string value" + +[["actual.yaml".root_key.a_dict]] +b = 2 + +[["actual.yaml".root_key.a_dict]] +c = "3.1" + +["actual.yaml".root_key.a_nested] +list = [0, 1, 2] +int = 10 From a53b283ba5ef31c37ccd057f205f9de6bbca7fca Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 01:38:02 +0100 Subject: [PATCH 02/11] test(yaml): suggest initial contents --- docs/ideas/yaml/jmespath-on-section.toml | 27 ------------- docs/ideas/yaml/jmespath-simple.toml | 7 ---- docs/ideas/yaml/jmespath-table.toml | 27 ------------- docs/ideas/yaml/jmespath.toml | 39 ++++++++++++++++++ src/nitpick/formats.py | 20 ++++++++-- src/nitpick/plugins/yaml.py | 11 +++-- tests/test_yaml.py | 40 +++++++------------ .../test_yaml/{actual.yaml => 1-actual.yaml} | 0 tests/test_yaml/1-desired.toml | 18 +++++++++ .../{expected.yaml => 1-expected.yaml} | 0 tests/test_yaml/2-desired.toml | 18 +++++++++ tests/test_yaml/2-expected.yaml | 21 ++++++++++ tests/test_yaml/style.toml | 18 --------- 13 files changed, 133 insertions(+), 113 deletions(-) delete mode 100644 docs/ideas/yaml/jmespath-on-section.toml delete mode 100644 docs/ideas/yaml/jmespath-simple.toml delete mode 100644 docs/ideas/yaml/jmespath-table.toml create mode 100644 docs/ideas/yaml/jmespath.toml rename tests/test_yaml/{actual.yaml => 1-actual.yaml} (100%) create mode 100644 tests/test_yaml/1-desired.toml rename tests/test_yaml/{expected.yaml => 1-expected.yaml} (100%) create mode 100644 tests/test_yaml/2-desired.toml create mode 100644 tests/test_yaml/2-expected.yaml delete mode 100644 tests/test_yaml/style.toml diff --git a/docs/ideas/yaml/jmespath-on-section.toml b/docs/ideas/yaml/jmespath-on-section.toml deleted file mode 100644 index e1f8be16..00000000 --- a/docs/ideas/yaml/jmespath-on-section.toml +++ /dev/null @@ -1,27 +0,0 @@ -# The values below were taken from .github/workflows/python.yaml in this repo - -# 1. JMESPath as part of the section name, after the file name. -# Everything after the file name is considered a JMESPath https://jmespath.org/ -# Format: ["path/to/file.ext".jmes.path.expression] -# -# 2. "jobs.build.strategy.matrix" should have "os" and "python-version" -# 3. Both are lists, and they have to be exactly as described here. -[".github/workflows/python.yaml".jobs.build.strategy.matrix] -os = ["ubuntu-latest", "macos-latest", "windows-latest"] -"python-version" = ["3.6", "3.7", "3.8", "3.9", "3.10"] - -# 4. "jobs.build" should have "runs-on" with value "${{ matrix.os }}" -[".github/workflows/python.yaml".jobs.build] -"runs-on" = "${{ matrix.os }}" - -# 5. "{{" and "}}" will conflict with Jinja https://github.com/andreoliwa/nitpick/issues/283 -# So we need a way to turn on/off Jinja templating. -# Probably "false" will be the default, to keep compatibility. -# Whoever wants to use Jinja will need to set "true" either here or as a global config on .nitpick.toml -__jinja = false - -# 6. Another way to turn off Jinja for a specific key only, not the whole dict -# (using the "__" syntax from Django filters, SQLAlchemy, factoryboy...) -"runs-on__no_jinja" = "${{ matrix.os }}" - -# FIXME: create a style from this diff --git a/docs/ideas/yaml/jmespath-simple.toml b/docs/ideas/yaml/jmespath-simple.toml deleted file mode 100644 index 69300465..00000000 --- a/docs/ideas/yaml/jmespath-simple.toml +++ /dev/null @@ -1,7 +0,0 @@ -# Simplified API, having JMESPath as direct keys -# Read the discussion: https://github.com/andreoliwa/nitpick/pull/353/files#r613816390 -[".github/workflows/python.yaml"] -"jobs.build.strategy.matrix.os" = "foo" -"jobs.build.steps" = ["bar"] -"jobs.build.steps.regex" = "baz d+" -"jobs.build.steps.contains" = "baz" diff --git a/docs/ideas/yaml/jmespath-table.toml b/docs/ideas/yaml/jmespath-table.toml deleted file mode 100644 index 43a1ebf7..00000000 --- a/docs/ideas/yaml/jmespath-table.toml +++ /dev/null @@ -1,27 +0,0 @@ -# 1. Clean approach with JMESPath in tables and no reserved keys (`jmespath` or `__jmespath`) -# https://github.com/andreoliwa/nitpick/pull/353/files#r614633283 -[[".github/workflows/python.yaml".jobs.build.steps]] -uses = "actions/checkout@v2" - -[[".github/workflows/python.yaml".jobs.build.steps]] -name = "Set up Python ${{ matrix.python-version }}" -uses = "actions/setup-python@v2" -with = {"python-version" = "${{ matrix.python-version }}"} - -# 2. Complex JMESPath expressions should be quoted -# (I still don't know how to deal with JMESPath that matches multiple items) -[[".github/workflows/python.yaml"."jobs.build.steps[].{name: name, uses: uses}"]] -uses = "actions/checkout@v2" - -# 3. JMESPath expression that has double quotes, wrapped in single quotes for TOML -[[".github/workflows/python.yaml".'jobs.build.strategy.matrix."python-version"']] -name = "Set up Python ${{ matrix.python-version }}" -uses = "actions/setup-python@v2" -with = {"python-version" = "${{ matrix.python-version }}"} - -# 4. And it allows Jinja tuning in https://github.com/andreoliwa/nitpick/issues/283 -name__jinja = "Set up Python ${{ matrix.python-version }}" -name__no_jinja = "Set up Python ${{ matrix.python-version }}" -name__jinja_off = "Set up Python ${{ matrix.python-version }}" - -# FIXME: create a style from this diff --git a/docs/ideas/yaml/jmespath.toml b/docs/ideas/yaml/jmespath.toml new file mode 100644 index 00000000..a4ddc6fb --- /dev/null +++ b/docs/ideas/yaml/jmespath.toml @@ -0,0 +1,39 @@ +# JMESPath as part of the section name, after the file name. +# Everything after the file name is considered a JMESPath https://jmespath.org/ +# Format: ["path/to/file.ext".jmes.path.expression] +# The values below were taken from .github/workflows/python.yaml in this repo + +# 1. Complex JMESPath expressions should be quoted +# (I still don't know how to deal with JMESPath that matches multiple items) +[[".github/workflows/python.yaml"."jobs.build.steps[].{name: name, uses: uses}"]] +uses = "actions/checkout@v2" + +# 2. JMESPath expression that has double quotes, wrapped in single quotes for TOML +[[".github/workflows/python.yaml".'jobs.build.strategy.matrix."python-version"']] +name = "Set up Python ${{ matrix.python-version }}" +uses = "actions/setup-python@v2" +with = {"python-version" = "${{ matrix.python-version }}"} + +# 3. It allows Jinja tuning in https://github.com/andreoliwa/nitpick/issues/283 +name__jinja = "Set up Python ${{ matrix.python-version }}" +name__no_jinja = "Set up Python ${{ matrix.python-version }}" +name__jinja_off = "Set up Python ${{ matrix.python-version }}" + +# 4. "{{" and "}}" will conflict with Jinja https://github.com/andreoliwa/nitpick/issues/283 +# So we need a way to turn on/off Jinja templating. +# Probably "false" will be the default, to keep compatibility. +# Whoever wants to use Jinja will need to set "true" either here or as a global config on .nitpick.toml +[".github/workflows/python.yaml".jobs.build] +__jinja = false + +# 5. Another way to turn off Jinja for a specific key only, not the whole dict +# (using the "__" syntax from Django filters, SQLAlchemy, factoryboy...) +"runs-on__no_jinja" = "${{ matrix.os }}" + +# 6. Simplified API, having JMESPath as direct keys +# Read the discussion: https://github.com/andreoliwa/nitpick/pull/353/files#r613816390 +[".github/workflows/jmespath-simple.yaml"] +"jobs.build.strategy.matrix.os" = "foo" +"jobs.build.steps" = ["bar"] +"jobs.build.steps.regex" = "baz d+" +"jobs.build.steps.contains" = "baz" diff --git a/src/nitpick/formats.py b/src/nitpick/formats.py index 3f026842..24b091d4 100644 --- a/src/nitpick/formats.py +++ b/src/nitpick/formats.py @@ -207,6 +207,20 @@ def compare_with_dictdiffer( return comparison +class InlineTableTomlDecoder(toml.TomlDecoder): + """A hacky decoder to work around some bug (or unfinished work) in the Python TOML package. + + https://github.com/uiri/toml/issues/362. + """ + + def get_empty_inline_table(self): + """Hackity hack for a crappy unmaintained package. + + Total lack of respect, the guy doesn't even reply: https://github.com/uiri/toml/issues/361 + """ + return self.get_empty_table() + + class TomlFormat(BaseFormat): """TOML configuration format.""" @@ -220,7 +234,7 @@ def load(self) -> bool: # TODO: I tried to replace toml by tomlkit, but lots of tests break. # The conversion to OrderedDict is not being done recursively (although I'm not sure this is a problem). # self._object = OrderedDict(tomlkit.loads(self._string)) - self._object = toml.loads(self._string, _dict=OrderedDict) + self._object = toml.loads(self._string, decoder=InlineTableTomlDecoder(OrderedDict)) if self._object is not None: if isinstance(self._object, BaseFormat): self._reformatted = self._object.reformatted @@ -298,8 +312,8 @@ def cleanup(cls, *args: List[Any]) -> List[Any]: return [str(value).lower() if isinstance(value, (int, float, bool)) else value for value in args] -RoundTripRepresenter.add_representer(SortedDict, RoundTripRepresenter.represent_dict) -RoundTripRepresenter.add_representer(OrderedDict, RoundTripRepresenter.represent_dict) +for dict_class in (SortedDict, OrderedDict): + RoundTripRepresenter.add_representer(dict_class, RoundTripRepresenter.represent_dict) class JsonFormat(BaseFormat): diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py index 6305a84b..88f8a6a8 100644 --- a/src/nitpick/plugins/yaml.py +++ b/src/nitpick/plugins/yaml.py @@ -44,7 +44,7 @@ def enforce_rules(self) -> Iterator[Fuss]: return yaml_format = YamlFormat(path=self.file_path) - comparison = yaml_format.compare_with_flatten(self.expected_config) # FIXME: + comparison = yaml_format.compare_with_flatten(self.expected_config) if not comparison.has_changes: return @@ -68,11 +68,10 @@ def report(self, violation: ViolationEnum, document: Optional[YAML], change: Opt @property def initial_contents(self) -> str: """Suggest the initial content for this missing file.""" - return "" # FIXME: - # yaml_as_string = YamlFormat(obj=self.expected_config).reformatted - # if self.autofix: - # self.file_path.write_text(yaml_as_string) - # return yaml_as_string + yaml_as_string = YamlFormat(obj=self.expected_config).reformatted + if self.autofix: + self.file_path.write_text(yaml_as_string) + return yaml_as_string @hookimpl diff --git a/tests/test_yaml.py b/tests/test_yaml.py index 278fe189..b232a8a2 100644 --- a/tests/test_yaml.py +++ b/tests/test_yaml.py @@ -4,29 +4,19 @@ from tests.helpers import ProjectMock -def test_suggest_initial_contents(tmp_path): +def test_suggest_initial_contents(tmp_path, datadir): """Suggest contents when YAML files do not exist.""" - pass # FIXME: - # filename = "my.toml" - # expected_toml = """ - # [section] - # key = "value" - # number = 10 - # """ - # ProjectMock(tmp_path).style( - # f""" - # ["{filename}".section] - # key = "value" - # number = 10 - # """ - # ).api_check_then_fix( - # Fuss( - # True, - # filename, - # SharedViolations.CREATE_FILE_WITH_SUGGESTION.code + TomlPlugin.violation_base_code, - # " was not found. Create it with this content:", - # expected_toml, - # ) + filename = ".github/workflows/python.yaml" + expected_yaml = (datadir / "2-expected.yaml").read_text() + ProjectMock(tmp_path).style(datadir / "2-desired.toml").api_check_then_fix( + Fuss( + False, + filename, + SharedViolations.CREATE_FILE_WITH_SUGGESTION.code + YamlPlugin.violation_base_code, + " was not found. Create it with this content:", + expected_yaml, + ) + ) # ).assert_file_contents( # filename, expected_toml # ) @@ -34,8 +24,8 @@ def test_suggest_initial_contents(tmp_path): def test_missing_different_values(tmp_path, datadir): """Test different and missing values on any YAML.""" - filename = "actual.yaml" - ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "style.toml").api_check_then_fix( + filename = "1-actual.yaml" + ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "1-style.toml").api_check_then_fix( Fuss( False, filename, @@ -70,4 +60,4 @@ def test_missing_different_values(tmp_path, datadir): - 2 """, ), - ) # FIXME: .assert_file_contents(filename, datadir / "expected.yaml") + ) # FIXME: .assert_file_contents(filename, datadir / "1-expected.yaml") diff --git a/tests/test_yaml/actual.yaml b/tests/test_yaml/1-actual.yaml similarity index 100% rename from tests/test_yaml/actual.yaml rename to tests/test_yaml/1-actual.yaml diff --git a/tests/test_yaml/1-desired.toml b/tests/test_yaml/1-desired.toml new file mode 100644 index 00000000..91ff4086 --- /dev/null +++ b/tests/test_yaml/1-desired.toml @@ -0,0 +1,18 @@ +["1-actual.yaml".python] +version = "3.9" + +[["1-actual.yaml".python.install]] +extra_requirements = ["item1", "item2", "item3"] + +[["1-actual.yaml".root_key.a_dict]] +a = "string value" + +[["1-actual.yaml".root_key.a_dict]] +b = 2 + +[["1-actual.yaml".root_key.a_dict]] +c = "3.1" + +["1-actual.yaml".root_key.a_nested] +list = [0, 1, 2] +int = 10 diff --git a/tests/test_yaml/expected.yaml b/tests/test_yaml/1-expected.yaml similarity index 100% rename from tests/test_yaml/expected.yaml rename to tests/test_yaml/1-expected.yaml diff --git a/tests/test_yaml/2-desired.toml b/tests/test_yaml/2-desired.toml new file mode 100644 index 00000000..2d9c5426 --- /dev/null +++ b/tests/test_yaml/2-desired.toml @@ -0,0 +1,18 @@ +# TOML tables to represent YAML lists +[[".github/workflows/python.yaml".jobs.build.steps]] +uses = "actions/checkout@v2" + +[[".github/workflows/python.yaml".jobs.build.steps]] +name = "Set up Python ${{ matrix.python-version }}" +uses = "actions/setup-python@v2" + +# A dynamic inline table; this causes issues with the TOML decoder +# See https://github.com/uiri/toml/issues/362 +with = {"python-version" = "${{ matrix.python-version }}"} + +[".github/workflows/python.yaml".jobs.build.strategy.matrix] +os = ["ubuntu-latest", "macos-latest", "windows-latest"] +"python-version" = ["3.6", "3.7", "3.8", "3.9", "3.10"] + +[".github/workflows/python.yaml".jobs.build] +"runs-on" = "${{ matrix.os }}" diff --git a/tests/test_yaml/2-expected.yaml b/tests/test_yaml/2-expected.yaml new file mode 100644 index 00000000..6a2e8ed2 --- /dev/null +++ b/tests/test_yaml/2-expected.yaml @@ -0,0 +1,21 @@ +jobs: + build: + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + strategy: + matrix: + os: + - ubuntu-latest + - macos-latest + - windows-latest + python-version: + - "3.6" + - "3.7" + - "3.8" + - "3.9" + - "3.10" diff --git a/tests/test_yaml/style.toml b/tests/test_yaml/style.toml deleted file mode 100644 index 51e4770a..00000000 --- a/tests/test_yaml/style.toml +++ /dev/null @@ -1,18 +0,0 @@ -["actual.yaml".python] -version = "3.9" - -[["actual.yaml".python.install]] -extra_requirements = ["item1", "item2", "item3"] - -[["actual.yaml".root_key.a_dict]] -a = "string value" - -[["actual.yaml".root_key.a_dict]] -b = 2 - -[["actual.yaml".root_key.a_dict]] -c = "3.1" - -["actual.yaml".root_key.a_nested] -list = [0, 1, 2] -int = 10 From a4bf732ba88ecc07d1e201abfe17a2bfe266c9a1 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 02:03:03 +0100 Subject: [PATCH 03/11] test(yaml): write initial contents --- src/nitpick/formats.py | 19 ++++++++----------- src/nitpick/plugins/base.py | 15 +++++++++++++-- src/nitpick/plugins/json.py | 5 +---- src/nitpick/plugins/toml.py | 5 +---- src/nitpick/plugins/yaml.py | 9 +++------ tests/test_yaml.py | 15 ++++++--------- 6 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/nitpick/formats.py b/src/nitpick/formats.py index 24b091d4..bcc806bb 100644 --- a/src/nitpick/formats.py +++ b/src/nitpick/formats.py @@ -16,14 +16,16 @@ from nitpick.generic import flatten, unflatten from nitpick.typedefs import JsonDict, PathOrStr, YamlObject +DictOrYamlObject = Union[JsonDict, YamlObject, "BaseFormat"] + class Comparison: """A comparison between two dictionaries, computing missing items and differences.""" def __init__( self, - actual: Union[JsonDict, YamlObject, "BaseFormat"], - expected: Union[JsonDict, YamlObject, "BaseFormat"], + actual: DictOrYamlObject, + expected: DictOrYamlObject, format_class: Type["BaseFormat"], ) -> None: self.flat_actual = self._normalize_value(actual) @@ -43,7 +45,7 @@ def has_changes(self) -> bool: return bool(self.missing or self.diff) @staticmethod - def _normalize_value(value: Union[JsonDict, YamlObject, "BaseFormat"]) -> JsonDict: + def _normalize_value(value: DictOrYamlObject) -> JsonDict: if isinstance(value, BaseFormat): dict_value: JsonDict = value.as_object else: @@ -98,12 +100,7 @@ class BaseFormat(metaclass=abc.ABCMeta): __repr__ = autorepr(["path"]) def __init__( - self, - *, - path: PathOrStr = None, - string: str = None, - obj: Union[JsonDict, YamlObject, "BaseFormat"] = None, - ignore_keys: List[str] = None + self, *, path: PathOrStr = None, string: str = None, obj: DictOrYamlObject = None, ignore_keys: List[str] = None ) -> None: self.path = path self._string = string @@ -145,14 +142,14 @@ def cleanup(cls, *args: List[Any]) -> List[Any]: """Cleanup similar values according to the specific format. E.g.: YamlFormat accepts 'True' or 'true'.""" return list(*args) - def _create_comparison(self, expected: Union[JsonDict, YamlObject, "BaseFormat"]): + def _create_comparison(self, expected: DictOrYamlObject): if not self._ignore_keys: return Comparison(self.as_object or {}, expected or {}, self.__class__) actual_original: Union[JsonDict, YamlObject] = self.as_object or {} actual_copy = actual_original.copy() if isinstance(actual_original, dict) else actual_original - expected_original: Union[JsonDict, YamlObject, "BaseFormat"] = expected or {} + expected_original: DictOrYamlObject = expected or {} if isinstance(expected_original, dict): expected_copy = expected_original.copy() elif isinstance(expected_original, BaseFormat): diff --git a/src/nitpick/plugins/base.py b/src/nitpick/plugins/base.py index 0df4ab18..03651eb4 100644 --- a/src/nitpick/plugins/base.py +++ b/src/nitpick/plugins/base.py @@ -2,14 +2,14 @@ import abc from functools import lru_cache from pathlib import Path -from typing import Iterator, Optional, Set +from typing import Iterator, Optional, Set, Type import jmespath from autorepr import autotext from loguru import logger from marshmallow import Schema -from nitpick.formats import Comparison +from nitpick.formats import BaseFormat, Comparison, DictOrYamlObject from nitpick.generic import search_dict from nitpick.plugins.info import FileInfo from nitpick.typedefs import JsonDict, mypy_property @@ -127,6 +127,17 @@ def enforce_rules(self) -> Iterator[Fuss]: def initial_contents(self) -> str: """Suggested initial content when the file doesn't exist.""" + def write_initial_contents(self, format_class: Type[BaseFormat], expected_obj: DictOrYamlObject = None) -> str: + """Helper to write initial contents based on a format.""" + if not expected_obj: + expected_obj = self.expected_config + + formatted_str = format_class(obj=expected_obj).reformatted + if self.autofix: + self.file_path.parent.mkdir(exist_ok=True, parents=True) + self.file_path.write_text(formatted_str) + return formatted_str + def warn_missing_different(self, comparison: Comparison, prefix: str = "") -> Iterator[Fuss]: """Warn about missing and different keys.""" # pylint: disable=not-callable diff --git a/src/nitpick/plugins/json.py b/src/nitpick/plugins/json.py index 6a899c76..a02d24fe 100644 --- a/src/nitpick/plugins/json.py +++ b/src/nitpick/plugins/json.py @@ -90,10 +90,7 @@ def initial_contents(self) -> str: """Suggest the initial content for this missing file.""" suggestion = DictBlender(self.expected_dict_from_contains_keys()) suggestion.add(self.expected_dict_from_contains_json()) - json_as_string = JsonFormat(obj=suggestion.mix()).reformatted if suggestion else "" - if self.autofix: - self.file_path.write_text(json_as_string) - return json_as_string + return self.write_initial_contents(JsonFormat, suggestion.mix()) @hookimpl diff --git a/src/nitpick/plugins/toml.py b/src/nitpick/plugins/toml.py index b96399a7..a9af405b 100644 --- a/src/nitpick/plugins/toml.py +++ b/src/nitpick/plugins/toml.py @@ -68,10 +68,7 @@ def report(self, violation: ViolationEnum, document: Optional[TOMLDocument], cha @property def initial_contents(self) -> str: """Suggest the initial content for this missing file.""" - toml_as_string = TomlFormat(obj=self.expected_config).reformatted - if self.autofix: - self.file_path.write_text(toml_as_string) - return toml_as_string + return self.write_initial_contents(TomlFormat) @hookimpl diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py index 88f8a6a8..5f54ee40 100644 --- a/src/nitpick/plugins/yaml.py +++ b/src/nitpick/plugins/yaml.py @@ -34,7 +34,7 @@ class YamlPlugin(NitpickPlugin): identify_tags = {"yaml"} violation_base_code = 360 - fixable = False # FIXME: + fixable = True def enforce_rules(self) -> Iterator[Fuss]: """Enforce rules for missing data in the YAML file.""" @@ -54,7 +54,7 @@ def enforce_rules(self) -> Iterator[Fuss]: self.report(SharedViolations.MISSING_VALUES, document, comparison.missing), ) if self.autofix and self.dirty and document: - document.dump(document, self.file_path) + pass # FIXME: document.dump(document, self.file_path) def report(self, violation: ViolationEnum, document: Optional[YAML], change: Optional[BaseFormat]): """Report a violation while optionally modifying the YAML document.""" @@ -68,10 +68,7 @@ def report(self, violation: ViolationEnum, document: Optional[YAML], change: Opt @property def initial_contents(self) -> str: """Suggest the initial content for this missing file.""" - yaml_as_string = YamlFormat(obj=self.expected_config).reformatted - if self.autofix: - self.file_path.write_text(yaml_as_string) - return yaml_as_string + return self.write_initial_contents(YamlFormat) @hookimpl diff --git a/tests/test_yaml.py b/tests/test_yaml.py index b232a8a2..4a6c4991 100644 --- a/tests/test_yaml.py +++ b/tests/test_yaml.py @@ -10,24 +10,21 @@ def test_suggest_initial_contents(tmp_path, datadir): expected_yaml = (datadir / "2-expected.yaml").read_text() ProjectMock(tmp_path).style(datadir / "2-desired.toml").api_check_then_fix( Fuss( - False, + True, filename, SharedViolations.CREATE_FILE_WITH_SUGGESTION.code + YamlPlugin.violation_base_code, " was not found. Create it with this content:", expected_yaml, ) - ) - # ).assert_file_contents( - # filename, expected_toml - # ) + ).assert_file_contents(filename, expected_yaml) def test_missing_different_values(tmp_path, datadir): """Test different and missing values on any YAML.""" filename = "1-actual.yaml" - ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "1-style.toml").api_check_then_fix( + ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "1-desired.toml").api_check_then_fix( Fuss( - False, + True, filename, YamlPlugin.violation_base_code + SharedViolations.DIFFERENT_VALUES.code, " has different values. Use this:", @@ -42,7 +39,7 @@ def test_missing_different_values(tmp_path, datadir): """, ), Fuss( - False, + True, filename, YamlPlugin.violation_base_code + SharedViolations.MISSING_VALUES.code, " has missing values:", @@ -60,4 +57,4 @@ def test_missing_different_values(tmp_path, datadir): - 2 """, ), - ) # FIXME: .assert_file_contents(filename, datadir / "1-expected.yaml") + ) # FIXME: .assert_file_contents(filename, datadir / "1-expected.yaml") From 9c24e7a2251bd554b53862442766ae07478f8000 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 02:15:43 +0100 Subject: [PATCH 04/11] docs: add to README --- docs/generate_rst.py | 3 +++ src/nitpick/plugins/ini.py | 2 +- src/nitpick/plugins/json.py | 2 +- src/nitpick/plugins/toml.py | 2 +- src/nitpick/plugins/yaml.py | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/generate_rst.py b/docs/generate_rst.py index a5ed61ed..f8ee2c8a 100644 --- a/docs/generate_rst.py +++ b/docs/generate_rst.py @@ -151,6 +151,9 @@ def row(self) -> Tuple[str, str, str]: FileType("Any JSON file", f"{READ_THE_DOCS_URL}plugins.html#json-files", True, True), FileType("Any text file", f"{READ_THE_DOCS_URL}plugins.html#text-files", True, False), FileType("Any TOML file", f"{READ_THE_DOCS_URL}plugins.html#toml-files", True, True), + FileType( + f"Any YAML file (except {PRE_COMMIT_CONFIG_YAML}", f"{READ_THE_DOCS_URL}plugins.html#yaml-files", True, True + ), FileType(EDITOR_CONFIG, f"{READ_THE_DOCS_URL}examples.html#example-editorconfig", True, True), FileType(PRE_COMMIT_CONFIG_YAML, f"{READ_THE_DOCS_URL}plugins.html#pre-commit-config-yaml", True, 282), FileType(PYLINTRC, f"{READ_THE_DOCS_URL}plugins.html#ini-files", True, True), diff --git a/src/nitpick/plugins/ini.py b/src/nitpick/plugins/ini.py index 9ba09e4c..6f5138f4 100644 --- a/src/nitpick/plugins/ini.py +++ b/src/nitpick/plugins/ini.py @@ -31,7 +31,7 @@ class Violations(ViolationEnum): class IniPlugin(NitpickPlugin): - """Enforce config on INI files. + """Enforce configurations and autofix INI files. Examples of ``.ini`` files handled by this plugin: diff --git a/src/nitpick/plugins/json.py b/src/nitpick/plugins/json.py index a02d24fe..e7bb1aff 100644 --- a/src/nitpick/plugins/json.py +++ b/src/nitpick/plugins/json.py @@ -28,7 +28,7 @@ class JsonFileSchema(BaseNitpickSchema): class JsonPlugin(NitpickPlugin): - """Enforce configurations for any JSON file. + """Enforce configurations and autofix JSON files. Add the configurations for the file name you wish to check. Style example: :ref:`the default config for package.json `. diff --git a/src/nitpick/plugins/toml.py b/src/nitpick/plugins/toml.py index a9af405b..bd3ca504 100644 --- a/src/nitpick/plugins/toml.py +++ b/src/nitpick/plugins/toml.py @@ -26,7 +26,7 @@ def change_toml(document: TOMLDocument, dictionary): class TomlPlugin(NitpickPlugin): - """Enforce config on TOML files. + """Enforce configurations and autofix TOML files. E.g.: `pyproject.toml (PEP 518) `_. diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py index 5f54ee40..cae06d56 100644 --- a/src/nitpick/plugins/yaml.py +++ b/src/nitpick/plugins/yaml.py @@ -30,7 +30,7 @@ def change_yaml(document: YAML, dictionary: YamlObject): class YamlPlugin(NitpickPlugin): - """Enforce config on YAML files.""" + """Enforce configurations and autofix YAML files.""" identify_tags = {"yaml"} violation_base_code = 360 From 0ebf7efc2736e400e41fbe465f14e0e4e27c108e Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 02:32:14 +0100 Subject: [PATCH 05/11] docs: add to README --- README.rst | 3 +++ docs/plugins.rst | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 7d7d19fd..528f7dbd 100644 --- a/README.rst +++ b/README.rst @@ -109,6 +109,9 @@ Implemented * - `Any TOML file `_ - ✅ - ✅ + * - `Any YAML file (except .pre-commit-config.yaml `_ + - ✅ + - ✅ * - `.editorconfig `_ - ✅ - ✅ diff --git a/docs/plugins.rst b/docs/plugins.rst index 679ebcd7..668ecd14 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -26,7 +26,7 @@ Style example: :ref:`the default pre-commit hooks `. INI files --------- -Enforce config on INI files. +Enforce configurations and autofix INI files. Examples of ``.ini`` files handled by this plugin: @@ -42,7 +42,7 @@ Style examples enforcing values on INI files: :ref:`flake8 configuration `. @@ -69,7 +69,7 @@ To check if ``some.txt`` file contains the lines ``abc`` and ``def`` (in any ord TOML files ---------- -Enforce config on TOML files. +Enforce configurations and autofix TOML files. E.g.: `pyproject.toml (PEP 518) `_. @@ -84,4 +84,4 @@ There are :ref:`many other examples here `. YAML files ---------- -Enforce config on YAML files. +Enforce configurations and autofix YAML files. From e044b2ca438d4b4dbd30dc28335180e4edbac3f1 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 02:50:30 +0100 Subject: [PATCH 06/11] chore: Prettier uses double quotes, ruamel.yaml uses single quotes --- .prettierignore | 3 +++ tests/test_yaml/2-expected.yaml | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.prettierignore b/.prettierignore index 0018b46e..bfb526ba 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,3 +3,6 @@ index.html _includes/* *.min.css README.md + +# Prettier uses double quotes, ruamel.yaml uses single quotes +tests/**/*.yaml diff --git a/tests/test_yaml/2-expected.yaml b/tests/test_yaml/2-expected.yaml index 6a2e8ed2..b3ec2850 100644 --- a/tests/test_yaml/2-expected.yaml +++ b/tests/test_yaml/2-expected.yaml @@ -14,8 +14,8 @@ jobs: - macos-latest - windows-latest python-version: - - "3.6" - - "3.7" - - "3.8" - - "3.9" - - "3.10" + - '3.6' + - '3.7' + - '3.8' + - '3.9' + - '3.10' From 63dcbe0c0250593775198ed9be41f628feed38e3 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 02:53:16 +0100 Subject: [PATCH 07/11] chore: ignore specific file, not all --- .prettierignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.prettierignore b/.prettierignore index bfb526ba..e0ead595 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,4 +5,4 @@ _includes/* README.md # Prettier uses double quotes, ruamel.yaml uses single quotes -tests/**/*.yaml +tests/test_yaml/2-expected.yaml From 33fb2e03b8c7c4c24c8dd3fa174f174f1c0293c4 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 03:25:05 +0100 Subject: [PATCH 08/11] chore: stupid mypy/pylint --- .pylintrc | 2 +- docs/examples.rst | 5 +++-- setup.cfg | 3 ++- src/nitpick/formats.py | 4 ++-- src/nitpick/resources/python/mypy.toml | 3 ++- src/nitpick/resources/python/pylint.toml | 2 +- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.pylintrc b/.pylintrc index b9204800..7ede5b22 100644 --- a/.pylintrc +++ b/.pylintrc @@ -30,7 +30,7 @@ output-format=colorized # --disable=W" # Configurations for the black formatter disable=bad-continuation,bad-whitespace, - fixme,cyclic-import + fixme,cyclic-import,line-too-long [BASIC] diff --git a/docs/examples.rst b/docs/examples.rst index 6485b800..6d9a164e 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -316,7 +316,8 @@ Contents of `resources/python/mypy.toml bool: # TODO: I tried to replace toml by tomlkit, but lots of tests break. # The conversion to OrderedDict is not being done recursively (although I'm not sure this is a problem). # self._object = OrderedDict(tomlkit.loads(self._string)) - self._object = toml.loads(self._string, decoder=InlineTableTomlDecoder(OrderedDict)) + self._object = toml.loads(self._string, decoder=InlineTableTomlDecoder(OrderedDict)) # type: ignore[call-arg] if self._object is not None: if isinstance(self._object, BaseFormat): self._reformatted = self._object.reformatted diff --git a/src/nitpick/resources/python/mypy.toml b/src/nitpick/resources/python/mypy.toml index 9af77240..c608e222 100644 --- a/src/nitpick/resources/python/mypy.toml +++ b/src/nitpick/resources/python/mypy.toml @@ -13,7 +13,8 @@ warn_no_return = true # Lint-style cleanliness for typing warn_redundant_casts = true -warn_unused_ignores = true +# False positives when running on local machine... it works on pre-commit.ci ¯\_(ツ)_/¯ +warn_unused_ignores = false [[".pre-commit-config.yaml".repos]] yaml = """ diff --git a/src/nitpick/resources/python/pylint.toml b/src/nitpick/resources/python/pylint.toml index cee76504..74ec1b28 100644 --- a/src/nitpick/resources/python/pylint.toml +++ b/src/nitpick/resources/python/pylint.toml @@ -20,7 +20,7 @@ output-format = "colorized" # comma_separated_values = ["MESSAGES CONTROL.disable"] # This syntax will be deprecated anyway, so I won't make it work now # Configurations for the black formatter -#disable = "bad-continuation,bad-whitespace,fixme,cyclic-import" +#disable = "bad-continuation,bad-whitespace,fixme,cyclic-import,line-too-long" [".pylintrc".BASIC] # List of builtins function names that should not be used, separated by a comma From 017477c9aca22d096bd2af6c2f6f57cc278091b4 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 06:13:38 +0100 Subject: [PATCH 09/11] feat: autofix YAML (recursive madness) --- .prettierignore | 2 +- src/nitpick/plugins/yaml.py | 66 ++++++++++++------- src/nitpick/typedefs.py | 2 + tests/helpers.py | 1 + tests/test_ini.py | 2 +- tests/test_yaml.py | 26 ++++---- tests/test_yaml/1-desired.toml | 18 ----- tests/test_yaml/1-expected.yaml | 16 ----- .../{1-actual.yaml => existing-actual.yaml} | 3 +- tests/test_yaml/existing-desired.toml | 18 +++++ tests/test_yaml/existing-expected.yaml | 21 ++++++ .../{2-desired.toml => new-desired.toml} | 0 .../{2-expected.yaml => new-expected.yaml} | 0 13 files changed, 103 insertions(+), 72 deletions(-) delete mode 100644 tests/test_yaml/1-desired.toml delete mode 100644 tests/test_yaml/1-expected.yaml rename tests/test_yaml/{1-actual.yaml => existing-actual.yaml} (60%) create mode 100644 tests/test_yaml/existing-desired.toml create mode 100644 tests/test_yaml/existing-expected.yaml rename tests/test_yaml/{2-desired.toml => new-desired.toml} (100%) rename tests/test_yaml/{2-expected.yaml => new-expected.yaml} (100%) diff --git a/.prettierignore b/.prettierignore index e0ead595..988a563f 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,4 +5,4 @@ _includes/* README.md # Prettier uses double quotes, ruamel.yaml uses single quotes -tests/test_yaml/2-expected.yaml +tests/test_yaml/*.yaml diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py index cae06d56..e21cefc3 100644 --- a/src/nitpick/plugins/yaml.py +++ b/src/nitpick/plugins/yaml.py @@ -1,8 +1,7 @@ """YAML files.""" +from collections import OrderedDict from itertools import chain -from typing import Iterator, Optional, Type - -from ruamel.yaml import YAML +from typing import Iterator, List, Optional, Type, Union, cast from nitpick.constants import PRE_COMMIT_CONFIG_YAML from nitpick.formats import BaseFormat, YamlFormat @@ -10,23 +9,43 @@ from nitpick.plugins.base import NitpickPlugin from nitpick.plugins.info import FileInfo from nitpick.plugins.text import KEY_CONTAINS -from nitpick.typedefs import YamlObject +from nitpick.typedefs import JsonDict, YamlObject, YamlValue from nitpick.violations import Fuss, SharedViolations, ViolationEnum -def change_yaml(document: YAML, dictionary: YamlObject): +def is_scalar(value: YamlValue) -> bool: + """Return True if the value is NOT a dict or a list.""" + return not isinstance(value, (OrderedDict, list)) + + +def traverse_yaml_tree(yaml_obj: YamlObject, change: JsonDict): """Traverse a YAML document recursively and change values, keeping its formatting and comments.""" - del document - del dictionary - # FIXME: - # for key, value in dictionary.items(): - # if isinstance(value, (dict, OrderedDict)): - # if key in document: - # change_toml(document[key], value) - # else: - # document.add(key, value) - # else: - # document[key] = value + for key, value in change.items(): + if key not in yaml_obj: + # Key doesn't exist: we can insert the whole nested OrderedDict at once, no regrets + last_pos = len(yaml_obj.keys()) + 1 + yaml_obj.insert(last_pos, key, value) + continue + + if is_scalar(value): + yaml_obj[key] = value + elif isinstance(value, OrderedDict): + traverse_yaml_tree(yaml_obj[key], value) + elif isinstance(value, list): + _traverse_yaml_list(yaml_obj, key, value) + + +def _traverse_yaml_list(yaml_obj: YamlObject, key: str, value: List[Union[OrderedDict, str, float]]): + for index, element in enumerate(value): + if is_scalar(element): + try: + yaml_obj[key][index] = element + except IndexError: + yaml_obj[key].append(element) + elif isinstance(element, list): + pass # FIXME: a list of lists in YAML? is it possible? + else: + traverse_yaml_tree(yaml_obj[key][index], cast(OrderedDict, element)) class YamlPlugin(NitpickPlugin): @@ -48,20 +67,19 @@ def enforce_rules(self) -> Iterator[Fuss]: if not comparison.has_changes: return - document = yaml_format.document if self.autofix else None yield from chain( - self.report(SharedViolations.DIFFERENT_VALUES, document, comparison.diff), - self.report(SharedViolations.MISSING_VALUES, document, comparison.missing), + self.report(SharedViolations.DIFFERENT_VALUES, yaml_format.as_object, comparison.diff), + self.report(SharedViolations.MISSING_VALUES, yaml_format.as_object, comparison.missing), ) - if self.autofix and self.dirty and document: - pass # FIXME: document.dump(document, self.file_path) + if self.autofix and self.dirty: + yaml_format.document.dump(yaml_format.as_object, self.file_path) - def report(self, violation: ViolationEnum, document: Optional[YAML], change: Optional[BaseFormat]): + def report(self, violation: ViolationEnum, yaml_object: YamlObject, change: Optional[BaseFormat]): """Report a violation while optionally modifying the YAML document.""" if not change: return - if document: - change_yaml(document, change.as_object) + if self.autofix: + traverse_yaml_tree(yaml_object, change.as_object) self.dirty = True yield self.reporter.make_fuss(violation, change.reformatted.strip(), prefix="", fixed=self.autofix) diff --git a/src/nitpick/typedefs.py b/src/nitpick/typedefs.py index e8053ee2..a66e0217 100644 --- a/src/nitpick/typedefs.py +++ b/src/nitpick/typedefs.py @@ -1,4 +1,5 @@ """Type definitions.""" +from collections import OrderedDict from pathlib import Path from typing import Any, Dict, Iterable, List, Tuple, Type, Union @@ -10,6 +11,7 @@ StrOrIterable = Union[str, Iterable[str]] Flake8Error = Tuple[int, int, str, Type] YamlObject = Union[CommentedSeq, CommentedMap] +YamlValue = Union[OrderedDict, list, str, float] # Decorated property not supported · Issue #1362 · python/mypy # https://github.com/python/mypy/issues/1362#issuecomment-562141376 diff --git a/tests/helpers.py b/tests/helpers.py index 0c7922d6..41fe9e28 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -200,6 +200,7 @@ def save_file(self, filename: PathOrStr, file_contents: PathOrStr, lint: bool = if lint or path.suffix == ".py": self.files_to_lint.append(path) clean = dedent(from_path_or_str(file_contents)).strip() + path.parent.mkdir(parents=True, exist_ok=True) path.write_text(f"{clean}\n") return self diff --git a/tests/test_ini.py b/tests/test_ini.py index 0a9f5d87..2d676d7c 100644 --- a/tests/test_ini.py +++ b/tests/test_ini.py @@ -40,7 +40,7 @@ def test_default_style_is_applied(project_default): strict_optional = True warn_no_return = True warn_redundant_casts = True - warn_unused_ignores = True + warn_unused_ignores = False """ expected_editor_config = """ root = True diff --git a/tests/test_yaml.py b/tests/test_yaml.py index 4a6c4991..0197779b 100644 --- a/tests/test_yaml.py +++ b/tests/test_yaml.py @@ -7,8 +7,8 @@ def test_suggest_initial_contents(tmp_path, datadir): """Suggest contents when YAML files do not exist.""" filename = ".github/workflows/python.yaml" - expected_yaml = (datadir / "2-expected.yaml").read_text() - ProjectMock(tmp_path).style(datadir / "2-desired.toml").api_check_then_fix( + expected_yaml = (datadir / "new-expected.yaml").read_text() + ProjectMock(tmp_path).style(datadir / "new-desired.toml").api_check_then_fix( Fuss( True, filename, @@ -21,8 +21,10 @@ def test_suggest_initial_contents(tmp_path, datadir): def test_missing_different_values(tmp_path, datadir): """Test different and missing values on any YAML.""" - filename = "1-actual.yaml" - ProjectMock(tmp_path).save_file(filename, datadir / filename).style(datadir / "1-desired.toml").api_check_then_fix( + filename = "me/deep/rooted.yaml" + ProjectMock(tmp_path).save_file(filename, datadir / "existing-actual.yaml").style( + datadir / "existing-desired.toml" + ).api_check_then_fix( Fuss( True, filename, @@ -32,9 +34,9 @@ def test_missing_different_values(tmp_path, datadir): python: install: - extra_requirements: - - item1 - - item2 - - item3 + - some + - nice + - package version: '3.9' """, ), @@ -46,15 +48,17 @@ def test_missing_different_values(tmp_path, datadir): """ root_key: a_dict: - - a: string value - - b: 2 - c: '3.1' + - b: 2 + - a: string value a_nested: int: 10 list: - 0 - - 1 - 2 + - 1 """, ), - ) # FIXME: .assert_file_contents(filename, datadir / "1-expected.yaml") + ).assert_file_contents( + filename, datadir / "existing-expected.yaml" + ) diff --git a/tests/test_yaml/1-desired.toml b/tests/test_yaml/1-desired.toml deleted file mode 100644 index 91ff4086..00000000 --- a/tests/test_yaml/1-desired.toml +++ /dev/null @@ -1,18 +0,0 @@ -["1-actual.yaml".python] -version = "3.9" - -[["1-actual.yaml".python.install]] -extra_requirements = ["item1", "item2", "item3"] - -[["1-actual.yaml".root_key.a_dict]] -a = "string value" - -[["1-actual.yaml".root_key.a_dict]] -b = 2 - -[["1-actual.yaml".root_key.a_dict]] -c = "3.1" - -["1-actual.yaml".root_key.a_nested] -list = [0, 1, 2] -int = 10 diff --git a/tests/test_yaml/1-expected.yaml b/tests/test_yaml/1-expected.yaml deleted file mode 100644 index 3e973fae..00000000 --- a/tests/test_yaml/1-expected.yaml +++ /dev/null @@ -1,16 +0,0 @@ -python: - version: 3.9 - install: - - method: pip - path: . - extra_requirements: - - doc -root_key: - a_nested: - list: [0, 1, 2] - int: 10 - a_dict: - - a: "string value" - - b: 2 - # This unquoted float will become "3.1" - - c: 3.10 diff --git a/tests/test_yaml/1-actual.yaml b/tests/test_yaml/existing-actual.yaml similarity index 60% rename from tests/test_yaml/1-actual.yaml rename to tests/test_yaml/existing-actual.yaml index 13be98c0..f8bdc4e9 100644 --- a/tests/test_yaml/1-actual.yaml +++ b/tests/test_yaml/existing-actual.yaml @@ -1,5 +1,6 @@ +# Root comment python: - version: 3.6 + version: 3.6 # Python 3.6 EOL this month! install: - method: pip path: . diff --git a/tests/test_yaml/existing-desired.toml b/tests/test_yaml/existing-desired.toml new file mode 100644 index 00000000..29e20ac1 --- /dev/null +++ b/tests/test_yaml/existing-desired.toml @@ -0,0 +1,18 @@ +["me/deep/rooted.yaml".python] +version = "3.9" + +[["me/deep/rooted.yaml".python.install]] +extra_requirements = ["some", "nice", "package"] + +[["me/deep/rooted.yaml".root_key.a_dict]] +c = "3.1" + +[["me/deep/rooted.yaml".root_key.a_dict]] +b = 2 + +[["me/deep/rooted.yaml".root_key.a_dict]] +a = "string value" + +["me/deep/rooted.yaml".root_key.a_nested] +list = [0, 2, 1] +int = 10 diff --git a/tests/test_yaml/existing-expected.yaml b/tests/test_yaml/existing-expected.yaml new file mode 100644 index 00000000..93491b77 --- /dev/null +++ b/tests/test_yaml/existing-expected.yaml @@ -0,0 +1,21 @@ +# Root comment +python: + version: '3.9' # Python 3.6 EOL this month! + install: + - method: pip + path: . + extra_requirements: + - some + - nice + - package +root_key: + a_dict: + - c: '3.1' + - b: 2 + - a: string value + a_nested: + int: 10 + list: + - 0 + - 2 + - 1 diff --git a/tests/test_yaml/2-desired.toml b/tests/test_yaml/new-desired.toml similarity index 100% rename from tests/test_yaml/2-desired.toml rename to tests/test_yaml/new-desired.toml diff --git a/tests/test_yaml/2-expected.yaml b/tests/test_yaml/new-expected.yaml similarity index 100% rename from tests/test_yaml/2-expected.yaml rename to tests/test_yaml/new-expected.yaml From f3106c2517ab18ca6f6ab3afc9fee0045e78df4c Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 07:05:41 +0100 Subject: [PATCH 10/11] test: weird YAML configs --- pyproject.toml | 2 +- src/nitpick/plugins/pre_commit.py | 2 +- src/nitpick/plugins/yaml.py | 27 ++++++++++++++++---------- src/nitpick/typedefs.py | 2 +- tests/test_yaml.py | 11 +++++++++++ tests/test_yaml/existing-actual.yaml | 8 ++++++++ tests/test_yaml/existing-desired.toml | 16 +++++++++++++++ tests/test_yaml/existing-expected.yaml | 13 +++++++++++++ 8 files changed, 68 insertions(+), 13 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f79d440b..c78c1194 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ NIP = "nitpick.flake8:NitpickFlake8Extension" [tool.poetry.plugins.nitpick] text = "nitpick.plugins.text" json = "nitpick.plugins.json" -pre_commit = "nitpick.plugins.pre_commit" # FIXME: remove this when removing the plugin class +pre_commit = "nitpick.plugins.pre_commit" # TODO: remove this when removing the plugin class ini = "nitpick.plugins.ini" toml = "nitpick.plugins.toml" yaml = "nitpick.plugins.yaml" diff --git a/src/nitpick/plugins/pre_commit.py b/src/nitpick/plugins/pre_commit.py index a582fe9c..55fed66b 100644 --- a/src/nitpick/plugins/pre_commit.py +++ b/src/nitpick/plugins/pre_commit.py @@ -208,7 +208,7 @@ def plugin_class() -> Type["NitpickPlugin"]: @hookimpl def can_handle(info: FileInfo) -> Optional[Type["NitpickPlugin"]]: """Handle pre-commit config file.""" - # FIXME: the YAML plugin should handle this file, now or later + # TODO: the YAML plugin should handle this file, now or later if info.path_from_root == PRE_COMMIT_CONFIG_YAML: return PreCommitPlugin return None diff --git a/src/nitpick/plugins/yaml.py b/src/nitpick/plugins/yaml.py index e21cefc3..28c168df 100644 --- a/src/nitpick/plugins/yaml.py +++ b/src/nitpick/plugins/yaml.py @@ -1,7 +1,7 @@ """YAML files.""" from collections import OrderedDict from itertools import chain -from typing import Iterator, List, Optional, Type, Union, cast +from typing import Iterator, List, Optional, Type, Union from nitpick.constants import PRE_COMMIT_CONFIG_YAML from nitpick.formats import BaseFormat, YamlFormat @@ -18,7 +18,7 @@ def is_scalar(value: YamlValue) -> bool: return not isinstance(value, (OrderedDict, list)) -def traverse_yaml_tree(yaml_obj: YamlObject, change: JsonDict): +def traverse_yaml_tree(yaml_obj: YamlObject, change: Union[JsonDict, OrderedDict]): """Traverse a YAML document recursively and change values, keeping its formatting and comments.""" for key, value in change.items(): if key not in yaml_obj: @@ -35,17 +35,24 @@ def traverse_yaml_tree(yaml_obj: YamlObject, change: JsonDict): _traverse_yaml_list(yaml_obj, key, value) -def _traverse_yaml_list(yaml_obj: YamlObject, key: str, value: List[Union[OrderedDict, str, float]]): +def _traverse_yaml_list(yaml_obj: YamlObject, key: str, value: List[YamlValue]): for index, element in enumerate(value): + insert: bool = index >= len(yaml_obj[key]) + + if not insert and is_scalar(yaml_obj[key][index]): + # If the original object is scalar, replace it with whatever element; + # without traversing, even if it's a dict + yaml_obj[key][index] = element + continue + if is_scalar(element): - try: - yaml_obj[key][index] = element - except IndexError: + if insert: yaml_obj[key].append(element) - elif isinstance(element, list): - pass # FIXME: a list of lists in YAML? is it possible? - else: - traverse_yaml_tree(yaml_obj[key][index], cast(OrderedDict, element)) + else: + yaml_obj[key][index] = element + continue + + traverse_yaml_tree(yaml_obj[key][index], element) # type: ignore # mypy kept complaining about the Union class YamlPlugin(NitpickPlugin): diff --git a/src/nitpick/typedefs.py b/src/nitpick/typedefs.py index a66e0217..341e945b 100644 --- a/src/nitpick/typedefs.py +++ b/src/nitpick/typedefs.py @@ -11,7 +11,7 @@ StrOrIterable = Union[str, Iterable[str]] Flake8Error = Tuple[int, int, str, Type] YamlObject = Union[CommentedSeq, CommentedMap] -YamlValue = Union[OrderedDict, list, str, float] +YamlValue = Union[JsonDict, OrderedDict, List[Any], str, float] # Decorated property not supported · Issue #1362 · python/mypy # https://github.com/python/mypy/issues/1362#issuecomment-562141376 diff --git a/tests/test_yaml.py b/tests/test_yaml.py index 0197779b..0b45d612 100644 --- a/tests/test_yaml.py +++ b/tests/test_yaml.py @@ -31,6 +31,17 @@ def test_missing_different_values(tmp_path, datadir): YamlPlugin.violation_base_code + SharedViolations.DIFFERENT_VALUES.code, " has different values. Use this:", """ + mixed: + - lets: + ruin: this + with: + - weird + - '1' + - crap + - second item: also a dict + - c: 1 + b: 2 + a: 3 python: install: - extra_requirements: diff --git a/tests/test_yaml/existing-actual.yaml b/tests/test_yaml/existing-actual.yaml index f8bdc4e9..516e975d 100644 --- a/tests/test_yaml/existing-actual.yaml +++ b/tests/test_yaml/existing-actual.yaml @@ -6,3 +6,11 @@ python: path: . extra_requirements: - doc +mixed: + - 1 + - string + - c: 1 + b: 2 + a: 3 + - and the remaining items are untouched + - [ 5,3,1 ] diff --git a/tests/test_yaml/existing-desired.toml b/tests/test_yaml/existing-desired.toml index 29e20ac1..f24dc87f 100644 --- a/tests/test_yaml/existing-desired.toml +++ b/tests/test_yaml/existing-desired.toml @@ -10,6 +10,22 @@ c = "3.1" [["me/deep/rooted.yaml".root_key.a_dict]] b = 2 +[["me/deep/rooted.yaml".mixed]] +lets = { ruin = "this", with = ["weird", "1", "crap"] } + +[["me/deep/rooted.yaml".mixed]] +"second item" = "also a dict" + +# Even though the third item is the same, +# it will show up as "has different values". +# When it's a list, the diff comparison returns the whole list +# and doesn't compare individual items. +# TODO: Maybe an improvement for the future? Compare and discard list items that are equal +[["me/deep/rooted.yaml".mixed]] +c = 1 +b = 2 +a = 3 + [["me/deep/rooted.yaml".root_key.a_dict]] a = "string value" diff --git a/tests/test_yaml/existing-expected.yaml b/tests/test_yaml/existing-expected.yaml index 93491b77..ee899158 100644 --- a/tests/test_yaml/existing-expected.yaml +++ b/tests/test_yaml/existing-expected.yaml @@ -8,6 +8,19 @@ python: - some - nice - package +mixed: + - lets: + ruin: this + with: + - weird + - '1' + - crap + - second item: also a dict + - c: 1 + b: 2 + a: 3 + - and the remaining items are untouched + - [5, 3, 1] root_key: a_dict: - c: '3.1' From 54e35fb3f87d310b4b14b09285d4b79a248d26c6 Mon Sep 17 00:00:00 2001 From: "W. Augusto Andreoli" Date: Fri, 31 Dec 2021 07:16:51 +0100 Subject: [PATCH 11/11] chore: typo on README --- README.rst | 2 +- docs/generate_rst.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 528f7dbd..08016597 100644 --- a/README.rst +++ b/README.rst @@ -109,7 +109,7 @@ Implemented * - `Any TOML file `_ - ✅ - ✅ - * - `Any YAML file (except .pre-commit-config.yaml `_ + * - `Any YAML file (except .pre-commit-config.yaml) `_ - ✅ - ✅ * - `.editorconfig `_ diff --git a/docs/generate_rst.py b/docs/generate_rst.py index f8ee2c8a..b06102da 100644 --- a/docs/generate_rst.py +++ b/docs/generate_rst.py @@ -152,7 +152,7 @@ def row(self) -> Tuple[str, str, str]: FileType("Any text file", f"{READ_THE_DOCS_URL}plugins.html#text-files", True, False), FileType("Any TOML file", f"{READ_THE_DOCS_URL}plugins.html#toml-files", True, True), FileType( - f"Any YAML file (except {PRE_COMMIT_CONFIG_YAML}", f"{READ_THE_DOCS_URL}plugins.html#yaml-files", True, True + f"Any YAML file (except {PRE_COMMIT_CONFIG_YAML})", f"{READ_THE_DOCS_URL}plugins.html#yaml-files", True, True ), FileType(EDITOR_CONFIG, f"{READ_THE_DOCS_URL}examples.html#example-editorconfig", True, True), FileType(PRE_COMMIT_CONFIG_YAML, f"{READ_THE_DOCS_URL}plugins.html#pre-commit-config-yaml", True, 282),