diff --git a/.prettierignore b/.prettierignore index 0018b46e..988a563f 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/test_yaml/*.yaml 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/README.rst b/README.rst index 7d7d19fd..08016597 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/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 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/docs/ideas/yaml/jmespath-on-section.toml b/docs/ideas/yaml/jmespath-on-section.toml deleted file mode 100644 index 0876ec60..00000000 --- a/docs/ideas/yaml/jmespath-on-section.toml +++ /dev/null @@ -1,25 +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 }}" 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 cc74ce99..00000000 --- a/docs/ideas/yaml/jmespath-table.toml +++ /dev/null @@ -1,25 +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 }}" 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/docs/plugins.rst b/docs/plugins.rst index 132fdc6c..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) `_. @@ -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 configurations and autofix 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..c78c1194 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" # TODO: 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/setup.cfg b/setup.cfg index 1226bc2d..fc58d8a0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,7 +31,8 @@ follow_imports = normal strict_optional = True warn_no_return = True 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 exclude = src/nitpick/compat.py 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..dd15ed83 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,21 +9,23 @@ 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 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) @@ -44,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: @@ -99,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 @@ -146,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): @@ -208,6 +204,20 @@ def compare_with_dictdiffer( return comparison +class InlineTableTomlDecoder(toml.TomlDecoder): # type: ignore[name-defined] + """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.""" @@ -221,7 +231,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)) # type: ignore[call-arg] if self._object is not None: if isinstance(self._object, BaseFormat): self._reformatted = self._object.reformatted @@ -234,30 +244,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 @@ -278,7 +309,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) +for dict_class in (SortedDict, OrderedDict): + RoundTripRepresenter.add_representer(dict_class, RoundTripRepresenter.represent_dict) class JsonFormat(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/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 6a899c76..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 `. @@ -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/pre_commit.py b/src/nitpick/plugins/pre_commit.py index 89d342cd..55fed66b 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.""" + # 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/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/toml.py b/src/nitpick/plugins/toml.py index b96399a7..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) `_. @@ -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 new file mode 100644 index 00000000..28c168df --- /dev/null +++ b/src/nitpick/plugins/yaml.py @@ -0,0 +1,114 @@ +"""YAML files.""" +from collections import OrderedDict +from itertools import chain +from typing import Iterator, List, Optional, Type, Union + +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 JsonDict, YamlObject, YamlValue +from nitpick.violations import Fuss, SharedViolations, ViolationEnum + + +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: 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: + # 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[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): + if insert: + yaml_obj[key].append(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): + """Enforce configurations and autofix YAML files.""" + + identify_tags = {"yaml"} + violation_base_code = 360 + fixable = True + + 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) + if not comparison.has_changes: + return + + yield from chain( + 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: + yaml_format.document.dump(yaml_format.as_object, self.file_path) + + 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 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) + + @property + def initial_contents(self) -> str: + """Suggest the initial content for this missing file.""" + return self.write_initial_contents(YamlFormat) + + +@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/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 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/src/nitpick/typedefs.py b/src/nitpick/typedefs.py index e8053ee2..341e945b 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[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/helpers.py b/tests/helpers.py index 8e070410..41fe9e28 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,8 @@ 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.parent.mkdir(parents=True, exist_ok=True) path.write_text(f"{clean}\n") return self @@ -196,9 +208,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 +224,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 +404,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_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_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..0b45d612 --- /dev/null +++ b/tests/test_yaml.py @@ -0,0 +1,75 @@ +"""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, datadir): + """Suggest contents when YAML files do not exist.""" + filename = ".github/workflows/python.yaml" + expected_yaml = (datadir / "new-expected.yaml").read_text() + ProjectMock(tmp_path).style(datadir / "new-desired.toml").api_check_then_fix( + Fuss( + 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_yaml) + + +def test_missing_different_values(tmp_path, datadir): + """Test different and missing values on any YAML.""" + 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, + 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: + - some + - nice + - package + version: '3.9' + """, + ), + Fuss( + True, + filename, + YamlPlugin.violation_base_code + SharedViolations.MISSING_VALUES.code, + " has missing values:", + """ + root_key: + a_dict: + - c: '3.1' + - b: 2 + - a: string value + a_nested: + int: 10 + list: + - 0 + - 2 + - 1 + """, + ), + ).assert_file_contents( + filename, datadir / "existing-expected.yaml" + ) diff --git a/tests/test_yaml/existing-actual.yaml b/tests/test_yaml/existing-actual.yaml new file mode 100644 index 00000000..516e975d --- /dev/null +++ b/tests/test_yaml/existing-actual.yaml @@ -0,0 +1,16 @@ +# Root comment +python: + version: 3.6 # Python 3.6 EOL this month! + install: + - method: pip + 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 new file mode 100644 index 00000000..f24dc87f --- /dev/null +++ b/tests/test_yaml/existing-desired.toml @@ -0,0 +1,34 @@ +["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".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" + +["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..ee899158 --- /dev/null +++ b/tests/test_yaml/existing-expected.yaml @@ -0,0 +1,34 @@ +# Root comment +python: + version: '3.9' # Python 3.6 EOL this month! + install: + - method: pip + path: . + extra_requirements: + - 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' + - b: 2 + - a: string value + a_nested: + int: 10 + list: + - 0 + - 2 + - 1 diff --git a/tests/test_yaml/new-desired.toml b/tests/test_yaml/new-desired.toml new file mode 100644 index 00000000..2d9c5426 --- /dev/null +++ b/tests/test_yaml/new-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/new-expected.yaml b/tests/test_yaml/new-expected.yaml new file mode 100644 index 00000000..b3ec2850 --- /dev/null +++ b/tests/test_yaml/new-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'