Add analyzer for yamllint (#12)

rosdistro_reviewer/element_analyzer/yamllint.py

@@ -0,0 +1,131 @@
+# Copyright 2024 Open Source Robotics Foundation, Inc.
+# Licensed under the Apache License, Version 2.0
+
+from pathlib import Path
+from pathlib import PurePosixPath
+from typing import List
+from typing import Mapping
+from typing import Optional
+from typing import Sequence
+from typing import Tuple
+
+from colcon_core.logging import colcon_logger
+from colcon_core.plugin_system import satisfies_version
+from git import Repo
+from git.objects import Blob
+from rosdistro_reviewer.element_analyzer \
+    import ElementAnalyzerExtensionPoint
+from rosdistro_reviewer.git_lines import get_added_lines
+from rosdistro_reviewer.review import Annotation
+from rosdistro_reviewer.review import Criterion
+from rosdistro_reviewer.review import Recommendation
+from yamllint import linter
+from yamllint.config import YamlLintConfig
+
+logger = colcon_logger.getChild(__name__)
+
+
+def _is_yaml_blob(item, depth) -> bool:
+    return PurePosixPath(item.path).suffix == '.yaml'
+
+
+def _get_changed_yaml(
+    path: Path,
+    target_ref: Optional[str] = None,
+    head_ref: Optional[str] = None,
+) -> Optional[Mapping[str, Sequence[range]]]:
+    if head_ref:
+        with Repo(path) as repo:
+            tree = repo.tree(head_ref)
+            yaml_files = [
+                str(item.path)
+                for item in tree.traverse(_is_yaml_blob)
+                if isinstance(item, Blob)
+            ]
+    else:
+        yaml_files = [
+            str(p.relative_to(path))
+            for p in path.glob('**/*.yaml')
+            if p.parts[len(path.parts)] != '.git'
+        ]
+    if not yaml_files:
+        logger.info('No YAML files were found in the repository')
+        return None
+
+    changes = get_added_lines(
+        path, target_ref=target_ref, head_ref=head_ref, paths=yaml_files)
+    if not changes:
+        logger.info('No YAML files were modified')
+        return None
+
+    return changes
+
+
+class YamllintAnalyzer(ElementAnalyzerExtensionPoint):
+    """Element analyzer for linting changes to YAML files."""
+
+    def __init__(self):  # noqa: D107
+        super().__init__()
+        satisfies_version(
+            ElementAnalyzerExtensionPoint.EXTENSION_POINT_VERSION, '^1.0')
+
+    def analyze(  # noqa: D102
+        self,
+        path: Path,
+        target_ref: Optional[str] = None,
+        head_ref: Optional[str] = None,
+    ) -> Tuple[Optional[List[Criterion]], Optional[List[Annotation]]]:
+        criteria: List[Criterion] = []
+        annotations: List[Annotation] = []
+
+        changed_yaml = _get_changed_yaml(path, target_ref, head_ref)
+        if not changed_yaml:
+            # Bypass check if no YAML files were changed
+            return None, None
+
+        logger.info('Performing analysis on YAML changes...')
+
+        config_file = path / '.yamllint'
+        if config_file.is_file():
+            logger.debug(f'Using yamllint config: {config_file}')
+            config = YamlLintConfig(file=str(config_file))
+        else:
+            logger.debug('Using default yamllint config')
+            config = YamlLintConfig('extends: default')
+
+        recommendation = Recommendation.APPROVE
+
+        for yaml_path, lines in changed_yaml.items():
+            if not lines:
+                continue
+            logger.debug(f'Reading {yaml_path} for yamllint')
+
+            # It would be better to avoid reading the entire file into memory,
+            # but it seems that yamllint is going to do that anyway and
+            # requires that file-like objects implement IOBase, which
+            # GitPython streams do not (missing readable method).
+            git_yaml_path = str(PurePosixPath(Path(yaml_path)))
+            if head_ref is not None:
+                with Repo(path) as repo:
+                    data = repo.tree(
+                        head_ref
+                    )[git_yaml_path].data_stream.read().decode()
+            else:
+                data = (path / yaml_path).read_text()
+            for problem in linter.run(data, config, filepath=git_yaml_path):
+                if any(problem.line in chunk for chunk in lines):
+                    annotations.append(Annotation(
+                        yaml_path,
+                        range(problem.line, problem.line + 1),
+                        'This line does not pass YAML '
+                        f'linter checks: {problem.desc}'))
+                    recommendation = Recommendation.DISAPPROVE
+
+        if recommendation == Recommendation.APPROVE:
+            message = 'All new lines of YAML pass linter checks'
+        else:
+            message = 'One or more linter violations were added to YAML files'
+
+        criteria.append(Criterion(recommendation, message))
+
+        return criteria, annotations

setup.cfg

@@ -25,6 +25,7 @@ install_requires =
     rosdep
     unidiff
     PyYAML
+    yamllint
 packages = find:
 zip_safe = true
 
@@ -62,6 +63,7 @@ rosdistro_reviewer.environment_variable =
     log_level = rosdistro_reviewer.command:LOG_LEVEL_ENVIRONMENT_VARIABLE
 rosdistro_reviewer.element_analyzer =
     rosdep = rosdistro_reviewer.element_analyzer.rosdep:RosdepAnalyzer
+    yamllint = rosdistro_reviewer.element_analyzer.yamllint:YamllintAnalyzer
 rosdistro_reviewer.submitter =
     github = rosdistro_reviewer.submitter.github:GitHubSubmitter
 

test/conftest.py

@@ -0,0 +1,18 @@
+# Copyright 2024 Open Source Robotics Foundation, Inc.
+# Licensed under the Apache License, Version 2.0
+
+from typing import Iterable
+
+from git import Repo
+import pytest
+
+
+@pytest.fixture
+def empty_repo(tmp_path) -> Iterable[Repo]:
+    with Repo.init(tmp_path) as repo:
+        repo.index.commit('Initial commit')
+
+        base = repo.create_head('main')
+        base.checkout()
+
+        yield repo

test/spell_check.words

@@ -9,14 +9,17 @@ debian
 deserialized
 diffs
 eoan
+filepath
 fred
 github
 https
+india
 iterdir
 itertools
 jessie
 lenny
 linter
+linting
 mantic
 mktemp
 mypy
@@ -52,3 +55,6 @@ waldo
 xenial
 yakkety
 yaml
+yamllint
+yankee
+zulu

test/test_yamllint_checks.py

@@ -0,0 +1,147 @@
+# Copyright 2024 Open Source Robotics Foundation, Inc.
+# Licensed under the Apache License, Version 2.0
+
+from pathlib import Path
+from typing import Iterable
+
+from git import Repo
+import pytest
+from rosdistro_reviewer.element_analyzer.yamllint import YamllintAnalyzer
+from rosdistro_reviewer.review import Recommendation
+
+# The control prefix intentionally contains a violation
+# to verify that we only surface problems related to new lines
+CONTROL_PREFIX = 'alpha: \n  bravo: charlie\n'
+CONTROL_SUFFIX = 'yankee:\n  - zulu\n'
+
+
+@pytest.fixture
+def repo_with_yaml(empty_repo) -> Iterable[Repo]:
+    repo_dir = Path(empty_repo.working_tree_dir)
+    (repo_dir / 'subdir').mkdir()
+
+    yaml_file = repo_dir / 'subdir' / 'file.yaml'
+    yaml_file.write_text(CONTROL_PREFIX)
+
+    empty_repo.index.add(str(yaml_file))
+    empty_repo.index.commit('Add YAML files')
+
+    return empty_repo
+
+
+def test_no_files(empty_repo):
+    repo_dir = Path(empty_repo.working_tree_dir)
+    extension = YamllintAnalyzer()
+    assert (None, None) == extension.analyze(repo_dir)
+
+
+def test_no_changes(repo_with_yaml):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+    assert (None, None) == extension.analyze(repo_dir)
+
+
+VIOLATIONS = (
+    # Trailing whitespace
+    'delta: null ',
+    # Not enough indentation
+    'echo:\n foxtrot: null',
+    # Too much indentation
+    'golf:\n   hotel: null',
+    # Too many spaces
+    'india:  null',
+)
+
+
+def test_control(repo_with_yaml):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+
+    (repo_dir / 'subdir' / 'file.yaml').write_text(''.join((
+        CONTROL_PREFIX,
+        CONTROL_SUFFIX,
+    )))
+
+    criteria, annotations = extension.analyze(repo_dir)
+    assert criteria and not annotations
+    assert all(Recommendation.APPROVE == c.recommendation for c in criteria)
+
+
+def test_target_ref(repo_with_yaml):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+
+    yaml_file = repo_dir / 'subdir' / 'file.yaml'
+    yaml_file.write_text(''.join((
+        CONTROL_PREFIX,
+        CONTROL_SUFFIX,
+    )))
+
+    repo_with_yaml.index.add(str(yaml_file))
+    repo_with_yaml.index.commit('Add more to the YAML file')
+
+    yaml_file.write_text(''.join((
+        CONTROL_PREFIX,
+        'problem:  line \n',
+        CONTROL_SUFFIX,
+    )))
+
+    criteria, annotations = extension.analyze(repo_dir, head_ref='HEAD')
+    assert criteria and not annotations
+    assert all(Recommendation.APPROVE == c.recommendation for c in criteria)
+
+    criteria, annotations = extension.analyze(repo_dir)
+    assert criteria and annotations
+    assert any(Recommendation.APPROVE != c.recommendation for c in criteria)
+
+
+def test_yamllint_config(repo_with_yaml):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+
+    (repo_dir / 'subdir' / 'file.yaml').write_text(''.join((
+        CONTROL_PREFIX,
+        'delta: null  # comment\n',
+        CONTROL_SUFFIX,
+    )))
+
+    criteria, annotations = extension.analyze(repo_dir)
+    assert criteria and not annotations
+    assert all(Recommendation.APPROVE == c.recommendation for c in criteria)
+
+    (repo_dir / '.yamllint').write_text('\n'.join((
+        'extends: default',
+        'rules:',
+        '  comments:',
+        '    min-spaces-from-content: 4',
+    )))
+
+    criteria, annotations = extension.analyze(repo_dir)
+    assert criteria and annotations
+    assert any(Recommendation.APPROVE != c.recommendation for c in criteria)
+
+
+def test_removal_only(repo_with_yaml):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+
+    (repo_dir / 'subdir' / 'file.yaml').write_text('')
+
+    assert (None, None) == extension.analyze(repo_dir)
+
+
+@pytest.mark.parametrize(
+    'violation', VIOLATIONS, ids=range(len(VIOLATIONS)))
+def test_violation(repo_with_yaml, violation):
+    repo_dir = Path(repo_with_yaml.working_tree_dir)
+    extension = YamllintAnalyzer()
+
+    (repo_dir / 'subdir' / 'file.yaml').write_text(''.join((
+        CONTROL_PREFIX,
+        violation + '\n',
+        CONTROL_SUFFIX,
+    )))
+
+    criteria, annotations = extension.analyze(repo_dir)
+    assert criteria and annotations
+    assert any(Recommendation.APPROVE != c.recommendation for c in criteria)