From 90c3c6137a4eaa80e3f431707e6e16f15cdd5917 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Fri, 18 Feb 2022 21:40:45 +0900 Subject: [PATCH 01/23] feat: ros include guard Signed-off-by: Takagi, Isamu --- .pre-commit-hooks.yaml | 7 ++ README.md | 6 + pre_commit_hooks/ros_include_guard.py | 154 ++++++++++++++++++++++++++ setup.cfg | 1 + 4 files changed, 168 insertions(+) create mode 100644 pre_commit_hooks/ros_include_guard.py diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index a9b56e6..f351320 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -28,3 +28,10 @@ entry: sort-package-xml language: python files: package.xml$ + +- id: ros-include-guard + name: check include guard + description: Check include guard macro name + entry: ros-include-guard + language: python + files: .*\.(h|hpp)$ diff --git a/README.md b/README.md index 26c3afe..6ca55ef 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ repos: - id: prettier-launch-xml - id: prettier-package-xml - id: sort-package-xml + - id: ros-include-guard ``` ## Hooks available @@ -40,6 +41,11 @@ If you want to exclude a tag from sorting, add `` at the begi rclcpp ``` +### ros-include-guard + +Fix the macro name of include guards. +If you want to disable this check, use `--allow-nolint` option and comment `NOLINT` on the same line. + [ros]: https://ros.org/ diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py new file mode 100644 index 0000000..e9d9220 --- /dev/null +++ b/pre_commit_hooks/ros_include_guard.py @@ -0,0 +1,154 @@ +# Copyright 2022 Tier IV, Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse +import pathlib +import re + + +class Directive: + + def __init__(self): + self.line = None + self.text = None + self.expected = None + self.tokens = None + self.nolint = False + + def update(self, line, text): + self.line = line + self.text = text + + def is_none(self): + return self.line is None + + def mismatch(self): + if self.nolint: return False + return self.text != self.expected + + def overwrite(self, lines): + if self.nolint: return + lines[self.line] = self.expected + + +class OpeningDirective(Directive): + + def prepare(self, macro_name, allow_nolint): + self.tokens = re.split(R'\b', self.text) + if 4 <= len(self.tokens): + self.tokens[3] = macro_name + self.expected = ''.join(self.tokens) + self.nolint = allow_nolint and ('NOLINT' in self.tokens) + + +class ClosingDirective(Directive): + + def prepare(self, macro_name, allow_nolint): + self.tokens = re.split(R'\b', self.text) + self.expected = '#endif // {}'.format(macro_name) + self.nolint = allow_nolint and ('NOLINT' in self.tokens) + + +class IncludeGuard: + + def __init__(self): + self.ifndef = OpeningDirective() + self.define = OpeningDirective() + self.endif = ClosingDirective() + self.pragma = False + + def items(self): + yield self.ifndef + yield self.define + yield self.endif + + def is_none(self): + return any(item.is_none() for item in self.items()) + + def mismatch(self): + return any(item.mismatch() for item in self.items()) + + def overwrite(self, lines): + return any(item.overwrite(lines) for item in self.items()) + + def prepare(self, macro_name, allow_nolint): + for item in self.items(): item.prepare(macro_name, allow_nolint) + + +def get_include_guard_info(lines): + + guard = IncludeGuard() + for line, text in enumerate(lines): + if text.startswith('#pragma once'): + guard.pragma = True + if text.startswith('#ifndef') and guard.ifndef.is_none(): + guard.ifndef.update(line, text) + if text.startswith('#define') and guard.define.is_none(): + guard.define.update(line, text) + if text.startswith('#endif'): + guard.endif.update(line, text) + return guard + + +def get_parts_after(parts: list, targets: set): + result = [] + for part in reversed(parts): + if part in targets: + break + result.append(part) + return reversed(result) + + +def get_include_guard_macro_name(filepath: pathlib.Path): + targets = {'include', 'src', 'test'} + for path in filepath.parents: + if path.joinpath('package.xml').exists(): + parts = get_parts_after(filepath.relative_to(path).parts, targets) + return '__'.join(parts).replace('.', '_').upper() + '_' + return None + + +def main(argv=None): + + parser = argparse.ArgumentParser() + parser.add_argument('filenames', nargs='*', help='Filenames to fix') + parser.add_argument('--allow-nolint', action='store_true', help='Allow nolint comment') + args = parser.parse_args(argv) + + return_code = 0 + for filename in args.filenames: + filepath = pathlib.Path(filename) + macro_name = get_include_guard_macro_name(filepath) + if macro_name: + lines = filepath.read_text().split('\n') + guard = get_include_guard_info(lines) + if guard.pragma: + continue + if guard.is_none(): + return_code = 1 + print('No include guard in {}'.format(filepath)) + continue + guard.prepare(macro_name, args.allow_nolint) + if guard.mismatch(): + return_code = 1 + print('Fix include guard in {}'.format(filepath)) + guard.overwrite(lines) + filepath.write_text('\n'.join(lines)) + continue + + return return_code + + +if __name__ == '__main__': + exit(main()) diff --git a/setup.cfg b/setup.cfg index 9be0a59..e61bbc0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -13,3 +13,4 @@ python_requires = >=3.8 [options.entry_points] console_scripts = sort-package-xml = pre_commit_hooks.sort_package_xml:main + ros-include-guard = pre_commit_hooks.ros_include_guard:main From c475144db162b62536331e584089339295b47424 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Sun, 20 Feb 2022 16:13:26 +0900 Subject: [PATCH 02/23] feat: tests Signed-off-by: Takagi, Isamu --- tests/resources/rospkg1/CMakeLists.txt | 12 ++++++ .../resources/rospkg1/include/rospkg/foobar.h | 3 ++ .../rospkg1/include/rospkg/foobar.hpp | 3 ++ tests/resources/rospkg1/package.xml | 19 ++++++++++ tests/resources/rospkg1/src/foo/bar/baz.hpp | 3 ++ tests/resources/rospkg1/src/foo/bar_baz.hpp | 3 ++ tests/resources/rospkg1/src/foo_bar/baz.hpp | 3 ++ tests/resources/rospkg1/src/foo_bar_baz.hpp | 3 ++ tests/resources/rospkg2/CMakeLists.txt | 12 ++++++ .../resources/rospkg2/include/rospkg/foobar.h | 3 ++ .../rospkg2/include/rospkg/foobar.hpp | 3 ++ .../rospkg2/include/rospkg/nolint.hpp | 3 ++ tests/resources/rospkg2/package.xml | 19 ++++++++++ tests/resources/rospkg2/src/foo/bar/baz.hpp | 3 ++ tests/resources/rospkg2/src/foo/bar_baz.hpp | 3 ++ tests/resources/rospkg2/src/foo_bar/baz.hpp | 3 ++ tests/resources/rospkg2/src/foo_bar_baz.hpp | 3 ++ tests/resources/rospkg3/CMakeLists.txt | 12 ++++++ .../resources/rospkg3/include/rospkg/foobar.h | 3 ++ .../rospkg3/include/rospkg/foobar.hpp | 3 ++ .../rospkg3/include/rospkg/nolint.hpp | 3 ++ tests/resources/rospkg3/package.xml | 19 ++++++++++ tests/resources/rospkg3/src/foo/bar/baz.hpp | 3 ++ tests/resources/rospkg3/src/foo/bar_baz.hpp | 3 ++ tests/resources/rospkg3/src/foo_bar/baz.hpp | 3 ++ tests/resources/rospkg3/src/foo_bar_baz.hpp | 3 ++ tests/test_ros_include_guard.py | 38 +++++++++++++++++++ tests/test_sort_package_xml.py | 18 +++++++++ 28 files changed, 209 insertions(+) create mode 100644 tests/resources/rospkg1/CMakeLists.txt create mode 100644 tests/resources/rospkg1/include/rospkg/foobar.h create mode 100644 tests/resources/rospkg1/include/rospkg/foobar.hpp create mode 100644 tests/resources/rospkg1/package.xml create mode 100644 tests/resources/rospkg1/src/foo/bar/baz.hpp create mode 100644 tests/resources/rospkg1/src/foo/bar_baz.hpp create mode 100644 tests/resources/rospkg1/src/foo_bar/baz.hpp create mode 100644 tests/resources/rospkg1/src/foo_bar_baz.hpp create mode 100644 tests/resources/rospkg2/CMakeLists.txt create mode 100644 tests/resources/rospkg2/include/rospkg/foobar.h create mode 100644 tests/resources/rospkg2/include/rospkg/foobar.hpp create mode 100644 tests/resources/rospkg2/include/rospkg/nolint.hpp create mode 100644 tests/resources/rospkg2/package.xml create mode 100644 tests/resources/rospkg2/src/foo/bar/baz.hpp create mode 100644 tests/resources/rospkg2/src/foo/bar_baz.hpp create mode 100644 tests/resources/rospkg2/src/foo_bar/baz.hpp create mode 100644 tests/resources/rospkg2/src/foo_bar_baz.hpp create mode 100644 tests/resources/rospkg3/CMakeLists.txt create mode 100644 tests/resources/rospkg3/include/rospkg/foobar.h create mode 100644 tests/resources/rospkg3/include/rospkg/foobar.hpp create mode 100644 tests/resources/rospkg3/include/rospkg/nolint.hpp create mode 100644 tests/resources/rospkg3/package.xml create mode 100644 tests/resources/rospkg3/src/foo/bar/baz.hpp create mode 100644 tests/resources/rospkg3/src/foo/bar_baz.hpp create mode 100644 tests/resources/rospkg3/src/foo_bar/baz.hpp create mode 100644 tests/resources/rospkg3/src/foo_bar_baz.hpp create mode 100644 tests/test_ros_include_guard.py create mode 100644 tests/test_sort_package_xml.py diff --git a/tests/resources/rospkg1/CMakeLists.txt b/tests/resources/rospkg1/CMakeLists.txt new file mode 100644 index 0000000..763d1a0 --- /dev/null +++ b/tests/resources/rospkg1/CMakeLists.txt @@ -0,0 +1,12 @@ +cmake_minimum_required(VERSION 3.8) +project(rospkg) + +find_package(ament_cmake_auto REQUIRED) +ament_auto_find_build_dependencies() + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() +endif() + +ament_auto_package() diff --git a/tests/resources/rospkg1/include/rospkg/foobar.h b/tests/resources/rospkg1/include/rospkg/foobar.h new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/include/rospkg/foobar.h @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg1/include/rospkg/foobar.hpp b/tests/resources/rospkg1/include/rospkg/foobar.hpp new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/include/rospkg/foobar.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg1/package.xml b/tests/resources/rospkg1/package.xml new file mode 100644 index 0000000..773011d --- /dev/null +++ b/tests/resources/rospkg1/package.xml @@ -0,0 +1,19 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + ament_lint_common + ament_lint_auto + + + ament_cmake + + + diff --git a/tests/resources/rospkg1/src/foo/bar/baz.hpp b/tests/resources/rospkg1/src/foo/bar/baz.hpp new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/src/foo/bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg1/src/foo/bar_baz.hpp b/tests/resources/rospkg1/src/foo/bar_baz.hpp new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/src/foo/bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg1/src/foo_bar/baz.hpp b/tests/resources/rospkg1/src/foo_bar/baz.hpp new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/src/foo_bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg1/src/foo_bar_baz.hpp b/tests/resources/rospkg1/src/foo_bar_baz.hpp new file mode 100644 index 0000000..6aaccb0 --- /dev/null +++ b/tests/resources/rospkg1/src/foo_bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY +#define DUMMY +#endif // DUMMY diff --git a/tests/resources/rospkg2/CMakeLists.txt b/tests/resources/rospkg2/CMakeLists.txt new file mode 100644 index 0000000..763d1a0 --- /dev/null +++ b/tests/resources/rospkg2/CMakeLists.txt @@ -0,0 +1,12 @@ +cmake_minimum_required(VERSION 3.8) +project(rospkg) + +find_package(ament_cmake_auto REQUIRED) +ament_auto_find_build_dependencies() + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() +endif() + +ament_auto_package() diff --git a/tests/resources/rospkg2/include/rospkg/foobar.h b/tests/resources/rospkg2/include/rospkg/foobar.h new file mode 100644 index 0000000..8cb9b88 --- /dev/null +++ b/tests/resources/rospkg2/include/rospkg/foobar.h @@ -0,0 +1,3 @@ +#ifndef ROSPKG__FOOBAR_H_ +#define ROSPKG__FOOBAR_H_ +#endif // ROSPKG__FOOBAR_H_ diff --git a/tests/resources/rospkg2/include/rospkg/foobar.hpp b/tests/resources/rospkg2/include/rospkg/foobar.hpp new file mode 100644 index 0000000..dba637f --- /dev/null +++ b/tests/resources/rospkg2/include/rospkg/foobar.hpp @@ -0,0 +1,3 @@ +#ifndef ROSPKG__FOOBAR_HPP_ +#define ROSPKG__FOOBAR_HPP_ +#endif // ROSPKG__FOOBAR_HPP_ diff --git a/tests/resources/rospkg2/include/rospkg/nolint.hpp b/tests/resources/rospkg2/include/rospkg/nolint.hpp new file mode 100644 index 0000000..8bfd40a --- /dev/null +++ b/tests/resources/rospkg2/include/rospkg/nolint.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY // NOLINT +#define DUMMY // NOLINT +#endif // DUMMY // NOLINT diff --git a/tests/resources/rospkg2/package.xml b/tests/resources/rospkg2/package.xml new file mode 100644 index 0000000..f738444 --- /dev/null +++ b/tests/resources/rospkg2/package.xml @@ -0,0 +1,19 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + ament_lint_auto + ament_lint_common + + + ament_cmake + + + diff --git a/tests/resources/rospkg2/src/foo/bar/baz.hpp b/tests/resources/rospkg2/src/foo/bar/baz.hpp new file mode 100644 index 0000000..174120f --- /dev/null +++ b/tests/resources/rospkg2/src/foo/bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO__BAR__BAZ_HPP_ +#define FOO__BAR__BAZ_HPP_ +#endif // FOO__BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg2/src/foo/bar_baz.hpp b/tests/resources/rospkg2/src/foo/bar_baz.hpp new file mode 100644 index 0000000..e15d1da --- /dev/null +++ b/tests/resources/rospkg2/src/foo/bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO__BAR_BAZ_HPP_ +#define FOO__BAR_BAZ_HPP_ +#endif // FOO__BAR_BAZ_HPP_ diff --git a/tests/resources/rospkg2/src/foo_bar/baz.hpp b/tests/resources/rospkg2/src/foo_bar/baz.hpp new file mode 100644 index 0000000..0ff1033 --- /dev/null +++ b/tests/resources/rospkg2/src/foo_bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO_BAR__BAZ_HPP_ +#define FOO_BAR__BAZ_HPP_ +#endif // FOO_BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg2/src/foo_bar_baz.hpp b/tests/resources/rospkg2/src/foo_bar_baz.hpp new file mode 100644 index 0000000..fbdaf7a --- /dev/null +++ b/tests/resources/rospkg2/src/foo_bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO_BAR_BAZ_HPP_ +#define FOO_BAR_BAZ_HPP_ +#endif // FOO_BAR_BAZ_HPP_ diff --git a/tests/resources/rospkg3/CMakeLists.txt b/tests/resources/rospkg3/CMakeLists.txt new file mode 100644 index 0000000..763d1a0 --- /dev/null +++ b/tests/resources/rospkg3/CMakeLists.txt @@ -0,0 +1,12 @@ +cmake_minimum_required(VERSION 3.8) +project(rospkg) + +find_package(ament_cmake_auto REQUIRED) +ament_auto_find_build_dependencies() + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() +endif() + +ament_auto_package() diff --git a/tests/resources/rospkg3/include/rospkg/foobar.h b/tests/resources/rospkg3/include/rospkg/foobar.h new file mode 100644 index 0000000..8cb9b88 --- /dev/null +++ b/tests/resources/rospkg3/include/rospkg/foobar.h @@ -0,0 +1,3 @@ +#ifndef ROSPKG__FOOBAR_H_ +#define ROSPKG__FOOBAR_H_ +#endif // ROSPKG__FOOBAR_H_ diff --git a/tests/resources/rospkg3/include/rospkg/foobar.hpp b/tests/resources/rospkg3/include/rospkg/foobar.hpp new file mode 100644 index 0000000..dba637f --- /dev/null +++ b/tests/resources/rospkg3/include/rospkg/foobar.hpp @@ -0,0 +1,3 @@ +#ifndef ROSPKG__FOOBAR_HPP_ +#define ROSPKG__FOOBAR_HPP_ +#endif // ROSPKG__FOOBAR_HPP_ diff --git a/tests/resources/rospkg3/include/rospkg/nolint.hpp b/tests/resources/rospkg3/include/rospkg/nolint.hpp new file mode 100644 index 0000000..8bfd40a --- /dev/null +++ b/tests/resources/rospkg3/include/rospkg/nolint.hpp @@ -0,0 +1,3 @@ +#ifndef DUMMY // NOLINT +#define DUMMY // NOLINT +#endif // DUMMY // NOLINT diff --git a/tests/resources/rospkg3/package.xml b/tests/resources/rospkg3/package.xml new file mode 100644 index 0000000..f738444 --- /dev/null +++ b/tests/resources/rospkg3/package.xml @@ -0,0 +1,19 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + ament_lint_auto + ament_lint_common + + + ament_cmake + + + diff --git a/tests/resources/rospkg3/src/foo/bar/baz.hpp b/tests/resources/rospkg3/src/foo/bar/baz.hpp new file mode 100644 index 0000000..174120f --- /dev/null +++ b/tests/resources/rospkg3/src/foo/bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO__BAR__BAZ_HPP_ +#define FOO__BAR__BAZ_HPP_ +#endif // FOO__BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo/bar_baz.hpp b/tests/resources/rospkg3/src/foo/bar_baz.hpp new file mode 100644 index 0000000..e15d1da --- /dev/null +++ b/tests/resources/rospkg3/src/foo/bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO__BAR_BAZ_HPP_ +#define FOO__BAR_BAZ_HPP_ +#endif // FOO__BAR_BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo_bar/baz.hpp b/tests/resources/rospkg3/src/foo_bar/baz.hpp new file mode 100644 index 0000000..0ff1033 --- /dev/null +++ b/tests/resources/rospkg3/src/foo_bar/baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO_BAR__BAZ_HPP_ +#define FOO_BAR__BAZ_HPP_ +#endif // FOO_BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo_bar_baz.hpp b/tests/resources/rospkg3/src/foo_bar_baz.hpp new file mode 100644 index 0000000..fbdaf7a --- /dev/null +++ b/tests/resources/rospkg3/src/foo_bar_baz.hpp @@ -0,0 +1,3 @@ +#ifndef FOO_BAR_BAZ_HPP_ +#define FOO_BAR_BAZ_HPP_ +#endif // FOO_BAR_BAZ_HPP_ diff --git a/tests/test_ros_include_guard.py b/tests/test_ros_include_guard.py new file mode 100644 index 0000000..c7eed67 --- /dev/null +++ b/tests/test_ros_include_guard.py @@ -0,0 +1,38 @@ +import pathlib +import pytest +from pre_commit_hooks import ros_include_guard + +rospkg1 = 'tests/resources/rospkg1/' +rospkg2 = 'tests/resources/rospkg2/' +rospkg3 = 'tests/resources/rospkg3/' + +files1 = [ + 'include/rospkg/foobar.h', + 'include/rospkg/foobar.hpp', + 'src/foo_bar_baz.hpp', + 'src/foo_bar/baz.hpp', + 'src/foo/bar_baz.hpp', + 'src/foo/bar/baz.hpp', +] + +files2 = [ + 'include/rospkg/nolint.hpp', +] + +cases = [] +for file in files1: + cases.append((rospkg1 + file, rospkg3 + file, 1)) + cases.append((rospkg2 + file, rospkg3 + file, 0)) +for file in files2: + cases.append((rospkg2 + file, rospkg3 + file, 0)) + + +@pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) +def test(target_file, answer_file, answer_code): + + return_code = ros_include_guard.main([target_file, '--allow-nolint']) + target_text = pathlib.Path(target_file).read_text() + answer_text = pathlib.Path(answer_file).read_text() + + assert return_code == answer_code + assert target_text == answer_text diff --git a/tests/test_sort_package_xml.py b/tests/test_sort_package_xml.py new file mode 100644 index 0000000..4d5f3da --- /dev/null +++ b/tests/test_sort_package_xml.py @@ -0,0 +1,18 @@ +import pathlib +import pytest +from pre_commit_hooks import sort_package_xml + +cases = [ + ('tests/resources/rospkg1/package.xml', 'tests/resources/rospkg3/package.xml', 1), + ('tests/resources/rospkg2/package.xml', 'tests/resources/rospkg3/package.xml', 0), +] + +@pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) +def test(target_file, answer_file, answer_code): + + return_code = sort_package_xml.main([target_file]) + target_text = pathlib.Path(target_file).read_text() + answer_text = pathlib.Path(answer_file).read_text() + + assert return_code == answer_code + assert target_text == answer_text From f5f2dc8c1df3176f88b27d06b12da13a84b6e9cb Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Sun, 20 Feb 2022 17:29:22 +0900 Subject: [PATCH 03/23] feat: CI for tests --- .github/workflows/pytest.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .github/workflows/pytest.yaml diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml new file mode 100644 index 0000000..f1e4c62 --- /dev/null +++ b/.github/workflows/pytest.yaml @@ -0,0 +1,14 @@ +name: pytest + +on: + pull_request: + +jobs: + pytest: + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v2 + + - name: Run pytest + run: pytest From f24bee4ba01eb6d9172beed48bc989629393696c Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Sun, 20 Feb 2022 17:33:25 +0900 Subject: [PATCH 04/23] Fix: CI --- .github/workflows/pytest.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index f1e4c62..4986132 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -11,4 +11,4 @@ jobs: uses: actions/checkout@v2 - name: Run pytest - run: pytest + run: python -m pytest From 19f10b643bf2e318b5e8e333673c671364644048 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Sun, 20 Feb 2022 17:35:01 +0900 Subject: [PATCH 05/23] fix: CI --- .github/workflows/pytest.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index 4986132..f1d2894 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -10,5 +10,8 @@ jobs: - name: Check out repository uses: actions/checkout@v2 + - name: Install pytest + run: python3 -m pip install pytest + - name: Run pytest - run: python -m pytest + run: python3 -m pytest From 07b6654d6b633f936b218c3aaf67d3f9dd59e0e1 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Tue, 22 Feb 2022 20:42:59 +0900 Subject: [PATCH 06/23] fix: rename class and remove type hint and nolint option --- pre_commit_hooks/ros_include_guard.py | 35 ++++++++++----------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index e9d9220..e7d9e7b 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -17,14 +17,13 @@ import re -class Directive: +class DirectiveBase: def __init__(self): self.line = None self.text = None self.expected = None self.tokens = None - self.nolint = False def update(self, line, text): self.line = line @@ -34,30 +33,26 @@ def is_none(self): return self.line is None def mismatch(self): - if self.nolint: return False return self.text != self.expected def overwrite(self, lines): - if self.nolint: return lines[self.line] = self.expected -class OpeningDirective(Directive): +class OpeningDirective(DirectiveBase): - def prepare(self, macro_name, allow_nolint): + def prepare(self, macro_name): self.tokens = re.split(R'\b', self.text) if 4 <= len(self.tokens): self.tokens[3] = macro_name self.expected = ''.join(self.tokens) - self.nolint = allow_nolint and ('NOLINT' in self.tokens) -class ClosingDirective(Directive): +class ClosingDirective(DirectiveBase): - def prepare(self, macro_name, allow_nolint): + def prepare(self, macro_name): self.tokens = re.split(R'\b', self.text) - self.expected = '#endif // {}'.format(macro_name) - self.nolint = allow_nolint and ('NOLINT' in self.tokens) + self.expected = F'#endif // {macro_name}' class IncludeGuard: @@ -82,8 +77,8 @@ def mismatch(self): def overwrite(self, lines): return any(item.overwrite(lines) for item in self.items()) - def prepare(self, macro_name, allow_nolint): - for item in self.items(): item.prepare(macro_name, allow_nolint) + def prepare(self, macro_name): + for item in self.items(): item.prepare(macro_name) def get_include_guard_info(lines): @@ -101,7 +96,7 @@ def get_include_guard_info(lines): return guard -def get_parts_after(parts: list, targets: set): +def get_parts_after(parts, targets): result = [] for part in reversed(parts): if part in targets: @@ -110,20 +105,16 @@ def get_parts_after(parts: list, targets: set): return reversed(result) -def get_include_guard_macro_name(filepath: pathlib.Path): +def get_include_guard_macro_name(filepath): targets = {'include', 'src', 'test'} - for path in filepath.parents: - if path.joinpath('package.xml').exists(): - parts = get_parts_after(filepath.relative_to(path).parts, targets) - return '__'.join(parts).replace('.', '_').upper() + '_' - return None + parts = get_parts_after(filepath.parts, targets) + return '__'.join(parts).replace('.', '_').upper() + '_' def main(argv=None): parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs='*', help='Filenames to fix') - parser.add_argument('--allow-nolint', action='store_true', help='Allow nolint comment') args = parser.parse_args(argv) return_code = 0 @@ -139,7 +130,7 @@ def main(argv=None): return_code = 1 print('No include guard in {}'.format(filepath)) continue - guard.prepare(macro_name, args.allow_nolint) + guard.prepare(macro_name) if guard.mismatch(): return_code = 1 print('Fix include guard in {}'.format(filepath)) From 223d87666c04b9bcbda04187bfd61e2efc6c328d Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Tue, 22 Feb 2022 20:55:33 +0900 Subject: [PATCH 07/23] fix: modify endif format --- pre_commit_hooks/ros_include_guard.py | 40 +++++++++++++++++++++------ 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index e7d9e7b..d57911d 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -19,7 +19,8 @@ class DirectiveBase: - def __init__(self): + def __init__(self, directive): + self.directive = directive self.line = None self.text = None self.expected = None @@ -42,25 +43,32 @@ def overwrite(self, lines): class OpeningDirective(DirectiveBase): def prepare(self, macro_name): - self.tokens = re.split(R'\b', self.text) - if 4 <= len(self.tokens): - self.tokens[3] = macro_name + self.tokens = split_space_boundary(self.text) + if 3 <= len(self.tokens): + self.tokens[2] = macro_name self.expected = ''.join(self.tokens) + else: + self.expected = F'{self.directive} {macro_name}' class ClosingDirective(DirectiveBase): def prepare(self, macro_name): - self.tokens = re.split(R'\b', self.text) - self.expected = F'#endif // {macro_name}' + self.tokens = split_space_boundary(self.text) + if 5 <= len(self.tokens): + self.tokens[4] = macro_name + print(self.tokens) + self.expected = ''.join(self.tokens) + else: + self.expected = F'{self.directive} // {macro_name}' class IncludeGuard: def __init__(self): - self.ifndef = OpeningDirective() - self.define = OpeningDirective() - self.endif = ClosingDirective() + self.ifndef = OpeningDirective('#ifndef') + self.define = OpeningDirective('#define') + self.endif = ClosingDirective('#endif') self.pragma = False def items(self): @@ -81,6 +89,20 @@ def prepare(self, macro_name): for item in self.items(): item.prepare(macro_name) +def split_space_boundary(text, delimiters={' '}): + + result = [] + prev = None + for char in text: + curr = char in delimiters + if curr != prev: + result.append(char) + else: + result[-1] += char + prev = curr + return result + + def get_include_guard_info(lines): guard = IncludeGuard() From f1f2a94e7df7f37a63660bd4d7753954b7b19497 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Tue, 22 Feb 2022 21:25:25 +0900 Subject: [PATCH 08/23] fix: optimize processing --- pre_commit_hooks/ros_include_guard.py | 37 ++++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index d57911d..9539650 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -14,7 +14,6 @@ import argparse import pathlib -import re class DirectiveBase: @@ -57,7 +56,6 @@ def prepare(self, macro_name): self.tokens = split_space_boundary(self.text) if 5 <= len(self.tokens): self.tokens[4] = macro_name - print(self.tokens) self.expected = ''.join(self.tokens) else: self.expected = F'{self.directive} // {macro_name}' @@ -141,24 +139,27 @@ def main(argv=None): return_code = 0 for filename in args.filenames: + # Search the include guard lines. filepath = pathlib.Path(filename) + lines = filepath.read_text().split('\n') + guard = get_include_guard_info(lines) + # Skip if the file using pragma once. + if guard.pragma: + continue + # Error if the include guard is not found. + if guard.is_none(): + print('No include guard in {}'.format(filepath)) + return_code = 1 + continue + # Error and auto fix if the macro name is not correct. macro_name = get_include_guard_macro_name(filepath) - if macro_name: - lines = filepath.read_text().split('\n') - guard = get_include_guard_info(lines) - if guard.pragma: - continue - if guard.is_none(): - return_code = 1 - print('No include guard in {}'.format(filepath)) - continue - guard.prepare(macro_name) - if guard.mismatch(): - return_code = 1 - print('Fix include guard in {}'.format(filepath)) - guard.overwrite(lines) - filepath.write_text('\n'.join(lines)) - continue + guard.prepare(macro_name) + if guard.mismatch(): + print('Fix include guard in {}'.format(filepath)) + return_code = 1 + guard.overwrite(lines) + filepath.write_text('\n'.join(lines)) + continue return return_code From 1074ab2c929fc3b5c076255eb63cb730ea8eace1 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Tue, 22 Feb 2022 21:32:59 +0900 Subject: [PATCH 09/23] fix: pre-commit hook names --- .pre-commit-hooks.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index f351320..56a33cf 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,5 +1,5 @@ - id: prettier-xacro - name: Prettier xacro + name: prettier xacro description: Apply Prettier with plugin-xml to xacro. entry: prettier --write --list-different --ignore-unknown --print-width 120 --xml-self-closing-space false language: node @@ -7,7 +7,7 @@ additional_dependencies: ["prettier@2.4.1", "@prettier/plugin-xml@1.1.0"] - id: prettier-launch-xml - name: Prettier launch.xml + name: prettier launch.xml description: Apply Prettier with plugin-xml to launch.xml. entry: prettier --write --list-different --ignore-unknown --print-width 200 --xml-self-closing-space false language: node @@ -15,7 +15,7 @@ additional_dependencies: ["prettier@2.4.1", "@prettier/plugin-xml@1.1.0"] - id: prettier-package-xml - name: Prettier package.xml + name: prettier package.xml description: Apply Prettier with plugin-xml to package.xml. entry: prettier --write --list-different --ignore-unknown --print-width 1000 --xml-self-closing-space false language: node @@ -23,15 +23,15 @@ additional_dependencies: ["prettier@2.4.1", "@prettier/plugin-xml@1.1.0"] - id: sort-package-xml - name: Sort package.xml + name: sort package.xml description: Sort the dependent packages in package.xml. entry: sort-package-xml language: python files: package.xml$ - id: ros-include-guard - name: check include guard - description: Check include guard macro name + name: fix include guard + description: Fix the include guard macro name. entry: ros-include-guard language: python files: .*\.(h|hpp)$ From 2d5144891a86f4658535b3518dd445ce926b0d58 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Tue, 22 Feb 2022 21:50:48 +0900 Subject: [PATCH 10/23] fix: remove nolint option --- README.md | 1 - tests/test_ros_include_guard.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 6ca55ef..62b3824 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,6 @@ If you want to exclude a tag from sorting, add `` at the begi ### ros-include-guard Fix the macro name of include guards. -If you want to disable this check, use `--allow-nolint` option and comment `NOLINT` on the same line. diff --git a/tests/test_ros_include_guard.py b/tests/test_ros_include_guard.py index c7eed67..f0da5db 100644 --- a/tests/test_ros_include_guard.py +++ b/tests/test_ros_include_guard.py @@ -30,7 +30,7 @@ @pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) def test(target_file, answer_file, answer_code): - return_code = ros_include_guard.main([target_file, '--allow-nolint']) + return_code = ros_include_guard.main([target_file]) target_text = pathlib.Path(target_file).read_text() answer_text = pathlib.Path(answer_file).read_text() From d0c2bb29f5f6d60e071384390334d02c6ec69b1b Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 00:41:12 +0900 Subject: [PATCH 11/23] fix: test for sort-pakcage-xml --- tests/test_sort_package_xml.py | 18 ++++++++------- tests/test_sort_package_xml/package.ans.xml | 25 +++++++++++++++++++++ tests/test_sort_package_xml/package.ng.xml | 25 +++++++++++++++++++++ tests/test_sort_package_xml/package.ok.xml | 25 +++++++++++++++++++++ 4 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 tests/test_sort_package_xml/package.ans.xml create mode 100644 tests/test_sort_package_xml/package.ng.xml create mode 100644 tests/test_sort_package_xml/package.ok.xml diff --git a/tests/test_sort_package_xml.py b/tests/test_sort_package_xml.py index 4d5f3da..44761e2 100644 --- a/tests/test_sort_package_xml.py +++ b/tests/test_sort_package_xml.py @@ -3,16 +3,18 @@ from pre_commit_hooks import sort_package_xml cases = [ - ('tests/resources/rospkg1/package.xml', 'tests/resources/rospkg3/package.xml', 1), - ('tests/resources/rospkg2/package.xml', 'tests/resources/rospkg3/package.xml', 0), + ('package.ok.xml', 'package.ans.xml', 0), + ('package.ng.xml', 'package.ans.xml', 1), ] @pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) -def test(target_file, answer_file, answer_code): +def test(target_file, answer_file, answer_code, datadir): - return_code = sort_package_xml.main([target_file]) - target_text = pathlib.Path(target_file).read_text() - answer_text = pathlib.Path(answer_file).read_text() + target_path = datadir.joinpath(target_file) + answer_path = datadir.joinpath(answer_file) + return_code = sort_package_xml.main([str(target_path)]) + target_text = pathlib.Path(target_path).read_text() + answer_text = pathlib.Path(answer_path).read_text() - assert return_code == answer_code - assert target_text == answer_text + assert return_code == answer_code + assert target_text == answer_text diff --git a/tests/test_sort_package_xml/package.ans.xml b/tests/test_sort_package_xml/package.ans.xml new file mode 100644 index 0000000..bd15183 --- /dev/null +++ b/tests/test_sort_package_xml/package.ans.xml @@ -0,0 +1,25 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + aaaaa + bbbbb + ccccc + ddddd + eeeee + + ament_lint_auto + ament_lint_common + + + ament_cmake + + + diff --git a/tests/test_sort_package_xml/package.ng.xml b/tests/test_sort_package_xml/package.ng.xml new file mode 100644 index 0000000..ff2097c --- /dev/null +++ b/tests/test_sort_package_xml/package.ng.xml @@ -0,0 +1,25 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + ccccc + bbbbb + ddddd + eeeee + aaaaa + + ament_lint_common + ament_lint_auto + + + ament_cmake + + + diff --git a/tests/test_sort_package_xml/package.ok.xml b/tests/test_sort_package_xml/package.ok.xml new file mode 100644 index 0000000..bd15183 --- /dev/null +++ b/tests/test_sort_package_xml/package.ok.xml @@ -0,0 +1,25 @@ + + + + rospkg + 0.0.0 + The dummy package to test pre-commit hooks + Takagi, Isamu + Apache License 2.0 + + ament_cmake + + aaaaa + bbbbb + ccccc + ddddd + eeeee + + ament_lint_auto + ament_lint_common + + + ament_cmake + + + From 29caafbf7bc36b8308acafe26a784a321086c872 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 04:26:59 +0900 Subject: [PATCH 12/23] fix: test cases --- tests/resources/rospkg1/CMakeLists.txt | 12 ----- tests/resources/rospkg1/package.xml | 19 ------- tests/resources/rospkg2/CMakeLists.txt | 12 ----- tests/resources/rospkg2/package.xml | 19 ------- tests/resources/rospkg3/CMakeLists.txt | 12 ----- .../include/rospkg/.hpp} | 0 .../resources/rospkg3/include/rospkg/foobar.h | 3 -- .../rospkg3/include/rospkg/nolint.hpp | 3 -- tests/resources/rospkg3/package.xml | 19 ------- tests/resources/rospkg3/src/foo/bar/baz.hpp | 3 -- tests/resources/rospkg3/src/foo/bar_baz.hpp | 3 -- tests/resources/rospkg3/src/foo_bar/baz.hpp | 3 -- tests/resources/rospkg3/src/foo_bar_baz.hpp | 3 -- tests/test_ros_include_guard.py | 51 +++++++++---------- .../include/rospkg/foobar.ng.h} | 0 .../include/rospkg/foobar.ng.hpp} | 0 .../include/rospkg/foobar.ok.h} | 0 .../include/rospkg/foobar.ok.hpp} | 0 .../include/rospkg/nolint.ng.hpp} | 0 .../include/rospkg/nolint.ok.hpp | 3 ++ .../src/foo/bar/baz.ng.hpp} | 0 .../src/foo/bar/baz.ok.hpp} | 0 .../src/foo/bar_baz.ng.hpp} | 0 .../src/foo/bar_baz.ok.hpp} | 0 .../src/foo_bar/baz.ng.hpp} | 0 .../src/foo_bar/baz.ok.hpp} | 0 .../src/foo_bar_baz.ng.hpp} | 0 .../src/foo_bar_baz.ok.hpp} | 0 28 files changed, 28 insertions(+), 137 deletions(-) delete mode 100644 tests/resources/rospkg1/CMakeLists.txt delete mode 100644 tests/resources/rospkg1/package.xml delete mode 100644 tests/resources/rospkg2/CMakeLists.txt delete mode 100644 tests/resources/rospkg2/package.xml delete mode 100644 tests/resources/rospkg3/CMakeLists.txt rename tests/resources/{rospkg2/include/rospkg/foobar.hpp => rospkg3/include/rospkg/.hpp} (100%) delete mode 100644 tests/resources/rospkg3/include/rospkg/foobar.h delete mode 100644 tests/resources/rospkg3/include/rospkg/nolint.hpp delete mode 100644 tests/resources/rospkg3/package.xml delete mode 100644 tests/resources/rospkg3/src/foo/bar/baz.hpp delete mode 100644 tests/resources/rospkg3/src/foo/bar_baz.hpp delete mode 100644 tests/resources/rospkg3/src/foo_bar/baz.hpp delete mode 100644 tests/resources/rospkg3/src/foo_bar_baz.hpp rename tests/{resources/rospkg1/include/rospkg/foobar.h => test_ros_include_guard/include/rospkg/foobar.ng.h} (100%) rename tests/{resources/rospkg1/include/rospkg/foobar.hpp => test_ros_include_guard/include/rospkg/foobar.ng.hpp} (100%) rename tests/{resources/rospkg2/include/rospkg/foobar.h => test_ros_include_guard/include/rospkg/foobar.ok.h} (100%) rename tests/{resources/rospkg3/include/rospkg/foobar.hpp => test_ros_include_guard/include/rospkg/foobar.ok.hpp} (100%) rename tests/{resources/rospkg2/include/rospkg/nolint.hpp => test_ros_include_guard/include/rospkg/nolint.ng.hpp} (100%) create mode 100644 tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp rename tests/{resources/rospkg1/src/foo/bar/baz.hpp => test_ros_include_guard/src/foo/bar/baz.ng.hpp} (100%) rename tests/{resources/rospkg2/src/foo/bar/baz.hpp => test_ros_include_guard/src/foo/bar/baz.ok.hpp} (100%) rename tests/{resources/rospkg1/src/foo/bar_baz.hpp => test_ros_include_guard/src/foo/bar_baz.ng.hpp} (100%) rename tests/{resources/rospkg2/src/foo/bar_baz.hpp => test_ros_include_guard/src/foo/bar_baz.ok.hpp} (100%) rename tests/{resources/rospkg1/src/foo_bar/baz.hpp => test_ros_include_guard/src/foo_bar/baz.ng.hpp} (100%) rename tests/{resources/rospkg2/src/foo_bar/baz.hpp => test_ros_include_guard/src/foo_bar/baz.ok.hpp} (100%) rename tests/{resources/rospkg1/src/foo_bar_baz.hpp => test_ros_include_guard/src/foo_bar_baz.ng.hpp} (100%) rename tests/{resources/rospkg2/src/foo_bar_baz.hpp => test_ros_include_guard/src/foo_bar_baz.ok.hpp} (100%) diff --git a/tests/resources/rospkg1/CMakeLists.txt b/tests/resources/rospkg1/CMakeLists.txt deleted file mode 100644 index 763d1a0..0000000 --- a/tests/resources/rospkg1/CMakeLists.txt +++ /dev/null @@ -1,12 +0,0 @@ -cmake_minimum_required(VERSION 3.8) -project(rospkg) - -find_package(ament_cmake_auto REQUIRED) -ament_auto_find_build_dependencies() - -if(BUILD_TESTING) - find_package(ament_lint_auto REQUIRED) - ament_lint_auto_find_test_dependencies() -endif() - -ament_auto_package() diff --git a/tests/resources/rospkg1/package.xml b/tests/resources/rospkg1/package.xml deleted file mode 100644 index 773011d..0000000 --- a/tests/resources/rospkg1/package.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - ament_lint_common - ament_lint_auto - - - ament_cmake - - - diff --git a/tests/resources/rospkg2/CMakeLists.txt b/tests/resources/rospkg2/CMakeLists.txt deleted file mode 100644 index 763d1a0..0000000 --- a/tests/resources/rospkg2/CMakeLists.txt +++ /dev/null @@ -1,12 +0,0 @@ -cmake_minimum_required(VERSION 3.8) -project(rospkg) - -find_package(ament_cmake_auto REQUIRED) -ament_auto_find_build_dependencies() - -if(BUILD_TESTING) - find_package(ament_lint_auto REQUIRED) - ament_lint_auto_find_test_dependencies() -endif() - -ament_auto_package() diff --git a/tests/resources/rospkg2/package.xml b/tests/resources/rospkg2/package.xml deleted file mode 100644 index f738444..0000000 --- a/tests/resources/rospkg2/package.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - ament_lint_auto - ament_lint_common - - - ament_cmake - - - diff --git a/tests/resources/rospkg3/CMakeLists.txt b/tests/resources/rospkg3/CMakeLists.txt deleted file mode 100644 index 763d1a0..0000000 --- a/tests/resources/rospkg3/CMakeLists.txt +++ /dev/null @@ -1,12 +0,0 @@ -cmake_minimum_required(VERSION 3.8) -project(rospkg) - -find_package(ament_cmake_auto REQUIRED) -ament_auto_find_build_dependencies() - -if(BUILD_TESTING) - find_package(ament_lint_auto REQUIRED) - ament_lint_auto_find_test_dependencies() -endif() - -ament_auto_package() diff --git a/tests/resources/rospkg2/include/rospkg/foobar.hpp b/tests/resources/rospkg3/include/rospkg/.hpp similarity index 100% rename from tests/resources/rospkg2/include/rospkg/foobar.hpp rename to tests/resources/rospkg3/include/rospkg/.hpp diff --git a/tests/resources/rospkg3/include/rospkg/foobar.h b/tests/resources/rospkg3/include/rospkg/foobar.h deleted file mode 100644 index 8cb9b88..0000000 --- a/tests/resources/rospkg3/include/rospkg/foobar.h +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef ROSPKG__FOOBAR_H_ -#define ROSPKG__FOOBAR_H_ -#endif // ROSPKG__FOOBAR_H_ diff --git a/tests/resources/rospkg3/include/rospkg/nolint.hpp b/tests/resources/rospkg3/include/rospkg/nolint.hpp deleted file mode 100644 index 8bfd40a..0000000 --- a/tests/resources/rospkg3/include/rospkg/nolint.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef DUMMY // NOLINT -#define DUMMY // NOLINT -#endif // DUMMY // NOLINT diff --git a/tests/resources/rospkg3/package.xml b/tests/resources/rospkg3/package.xml deleted file mode 100644 index f738444..0000000 --- a/tests/resources/rospkg3/package.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - ament_lint_auto - ament_lint_common - - - ament_cmake - - - diff --git a/tests/resources/rospkg3/src/foo/bar/baz.hpp b/tests/resources/rospkg3/src/foo/bar/baz.hpp deleted file mode 100644 index 174120f..0000000 --- a/tests/resources/rospkg3/src/foo/bar/baz.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef FOO__BAR__BAZ_HPP_ -#define FOO__BAR__BAZ_HPP_ -#endif // FOO__BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo/bar_baz.hpp b/tests/resources/rospkg3/src/foo/bar_baz.hpp deleted file mode 100644 index e15d1da..0000000 --- a/tests/resources/rospkg3/src/foo/bar_baz.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef FOO__BAR_BAZ_HPP_ -#define FOO__BAR_BAZ_HPP_ -#endif // FOO__BAR_BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo_bar/baz.hpp b/tests/resources/rospkg3/src/foo_bar/baz.hpp deleted file mode 100644 index 0ff1033..0000000 --- a/tests/resources/rospkg3/src/foo_bar/baz.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef FOO_BAR__BAZ_HPP_ -#define FOO_BAR__BAZ_HPP_ -#endif // FOO_BAR__BAZ_HPP_ diff --git a/tests/resources/rospkg3/src/foo_bar_baz.hpp b/tests/resources/rospkg3/src/foo_bar_baz.hpp deleted file mode 100644 index fbdaf7a..0000000 --- a/tests/resources/rospkg3/src/foo_bar_baz.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef FOO_BAR_BAZ_HPP_ -#define FOO_BAR_BAZ_HPP_ -#endif // FOO_BAR_BAZ_HPP_ diff --git a/tests/test_ros_include_guard.py b/tests/test_ros_include_guard.py index f0da5db..0cbde2e 100644 --- a/tests/test_ros_include_guard.py +++ b/tests/test_ros_include_guard.py @@ -1,38 +1,37 @@ -import pathlib import pytest +import shutil +from pathlib import Path from pre_commit_hooks import ros_include_guard -rospkg1 = 'tests/resources/rospkg1/' -rospkg2 = 'tests/resources/rospkg2/' -rospkg3 = 'tests/resources/rospkg3/' - -files1 = [ +files = [ 'include/rospkg/foobar.h', 'include/rospkg/foobar.hpp', + 'include/rospkg/nolint.hpp', 'src/foo_bar_baz.hpp', 'src/foo_bar/baz.hpp', 'src/foo/bar_baz.hpp', 'src/foo/bar/baz.hpp', ] -files2 = [ - 'include/rospkg/nolint.hpp', -] - cases = [] -for file in files1: - cases.append((rospkg1 + file, rospkg3 + file, 1)) - cases.append((rospkg2 + file, rospkg3 + file, 0)) -for file in files2: - cases.append((rospkg2 + file, rospkg3 + file, 0)) - - -@pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) -def test(target_file, answer_file, answer_code): - - return_code = ros_include_guard.main([target_file]) - target_text = pathlib.Path(target_file).read_text() - answer_text = pathlib.Path(answer_file).read_text() - - assert return_code == answer_code - assert target_text == answer_text +for file in files: + cases.append((Path(file), 0)) + cases.append((Path(file), 1)) + + +@pytest.mark.parametrize(('target_file', 'answer_code'), cases) +def test(target_file, answer_code, datadir): + + ok_ng = '.ng' if answer_code else '.ok' + source_file = target_file.with_suffix(ok_ng + target_file.suffix) + answer_file = target_file.with_suffix('.ok' + target_file.suffix) + target_path = datadir.joinpath(target_file) + source_path = datadir.joinpath(source_file) + answer_path = datadir.joinpath(answer_file) + shutil.copy(source_path, target_path) + + return_code = ros_include_guard.main([str(target_path)]) + target_text = target_path.read_text() + answer_text = answer_path.read_text() + assert return_code == answer_code + assert target_text == answer_text diff --git a/tests/resources/rospkg1/include/rospkg/foobar.h b/tests/test_ros_include_guard/include/rospkg/foobar.ng.h similarity index 100% rename from tests/resources/rospkg1/include/rospkg/foobar.h rename to tests/test_ros_include_guard/include/rospkg/foobar.ng.h diff --git a/tests/resources/rospkg1/include/rospkg/foobar.hpp b/tests/test_ros_include_guard/include/rospkg/foobar.ng.hpp similarity index 100% rename from tests/resources/rospkg1/include/rospkg/foobar.hpp rename to tests/test_ros_include_guard/include/rospkg/foobar.ng.hpp diff --git a/tests/resources/rospkg2/include/rospkg/foobar.h b/tests/test_ros_include_guard/include/rospkg/foobar.ok.h similarity index 100% rename from tests/resources/rospkg2/include/rospkg/foobar.h rename to tests/test_ros_include_guard/include/rospkg/foobar.ok.h diff --git a/tests/resources/rospkg3/include/rospkg/foobar.hpp b/tests/test_ros_include_guard/include/rospkg/foobar.ok.hpp similarity index 100% rename from tests/resources/rospkg3/include/rospkg/foobar.hpp rename to tests/test_ros_include_guard/include/rospkg/foobar.ok.hpp diff --git a/tests/resources/rospkg2/include/rospkg/nolint.hpp b/tests/test_ros_include_guard/include/rospkg/nolint.ng.hpp similarity index 100% rename from tests/resources/rospkg2/include/rospkg/nolint.hpp rename to tests/test_ros_include_guard/include/rospkg/nolint.ng.hpp diff --git a/tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp b/tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp new file mode 100644 index 0000000..1e8cca2 --- /dev/null +++ b/tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp @@ -0,0 +1,3 @@ +#ifndef ROSPKG__NOLINT_HPP_ // NOLINT +#define ROSPKG__NOLINT_HPP_ // NOLINT +#endif // ROSPKG__NOLINT_HPP_ // NOLINT diff --git a/tests/resources/rospkg1/src/foo/bar/baz.hpp b/tests/test_ros_include_guard/src/foo/bar/baz.ng.hpp similarity index 100% rename from tests/resources/rospkg1/src/foo/bar/baz.hpp rename to tests/test_ros_include_guard/src/foo/bar/baz.ng.hpp diff --git a/tests/resources/rospkg2/src/foo/bar/baz.hpp b/tests/test_ros_include_guard/src/foo/bar/baz.ok.hpp similarity index 100% rename from tests/resources/rospkg2/src/foo/bar/baz.hpp rename to tests/test_ros_include_guard/src/foo/bar/baz.ok.hpp diff --git a/tests/resources/rospkg1/src/foo/bar_baz.hpp b/tests/test_ros_include_guard/src/foo/bar_baz.ng.hpp similarity index 100% rename from tests/resources/rospkg1/src/foo/bar_baz.hpp rename to tests/test_ros_include_guard/src/foo/bar_baz.ng.hpp diff --git a/tests/resources/rospkg2/src/foo/bar_baz.hpp b/tests/test_ros_include_guard/src/foo/bar_baz.ok.hpp similarity index 100% rename from tests/resources/rospkg2/src/foo/bar_baz.hpp rename to tests/test_ros_include_guard/src/foo/bar_baz.ok.hpp diff --git a/tests/resources/rospkg1/src/foo_bar/baz.hpp b/tests/test_ros_include_guard/src/foo_bar/baz.ng.hpp similarity index 100% rename from tests/resources/rospkg1/src/foo_bar/baz.hpp rename to tests/test_ros_include_guard/src/foo_bar/baz.ng.hpp diff --git a/tests/resources/rospkg2/src/foo_bar/baz.hpp b/tests/test_ros_include_guard/src/foo_bar/baz.ok.hpp similarity index 100% rename from tests/resources/rospkg2/src/foo_bar/baz.hpp rename to tests/test_ros_include_guard/src/foo_bar/baz.ok.hpp diff --git a/tests/resources/rospkg1/src/foo_bar_baz.hpp b/tests/test_ros_include_guard/src/foo_bar_baz.ng.hpp similarity index 100% rename from tests/resources/rospkg1/src/foo_bar_baz.hpp rename to tests/test_ros_include_guard/src/foo_bar_baz.ng.hpp diff --git a/tests/resources/rospkg2/src/foo_bar_baz.hpp b/tests/test_ros_include_guard/src/foo_bar_baz.ok.hpp similarity index 100% rename from tests/resources/rospkg2/src/foo_bar_baz.hpp rename to tests/test_ros_include_guard/src/foo_bar_baz.ok.hpp From 606b673326c4344b1b695d6dfe508d28bfea942c Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 04:29:54 +0900 Subject: [PATCH 13/23] fix: test cases --- tests/test_sort_package_xml.py | 29 ++++++++++++++------- tests/test_sort_package_xml/package.ans.xml | 25 ------------------ 2 files changed, 20 insertions(+), 34 deletions(-) delete mode 100644 tests/test_sort_package_xml/package.ans.xml diff --git a/tests/test_sort_package_xml.py b/tests/test_sort_package_xml.py index 44761e2..1cd570e 100644 --- a/tests/test_sort_package_xml.py +++ b/tests/test_sort_package_xml.py @@ -1,20 +1,31 @@ -import pathlib import pytest +import shutil +from pathlib import Path from pre_commit_hooks import sort_package_xml -cases = [ - ('package.ok.xml', 'package.ans.xml', 0), - ('package.ng.xml', 'package.ans.xml', 1), +files = [ + 'package.xml', ] -@pytest.mark.parametrize(('target_file', 'answer_file', 'answer_code'), cases) -def test(target_file, answer_file, answer_code, datadir): +cases = [] +for file in files: + cases.append((Path(file), 0)) + cases.append((Path(file), 1)) + +@pytest.mark.parametrize(('target_file', 'answer_code'), cases) +def test(target_file, answer_code, datadir): + + ok_ng = '.ng' if answer_code else '.ok' + source_file = target_file.with_suffix(ok_ng + target_file.suffix) + answer_file = target_file.with_suffix('.ok' + target_file.suffix) target_path = datadir.joinpath(target_file) + source_path = datadir.joinpath(source_file) answer_path = datadir.joinpath(answer_file) - return_code = sort_package_xml.main([str(target_path)]) - target_text = pathlib.Path(target_path).read_text() - answer_text = pathlib.Path(answer_path).read_text() + shutil.copy(source_path, target_path) + return_code = sort_package_xml.main([str(target_path)]) + target_text = target_path.read_text() + answer_text = answer_path.read_text() assert return_code == answer_code assert target_text == answer_text diff --git a/tests/test_sort_package_xml/package.ans.xml b/tests/test_sort_package_xml/package.ans.xml deleted file mode 100644 index bd15183..0000000 --- a/tests/test_sort_package_xml/package.ans.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - aaaaa - bbbbb - ccccc - ddddd - eeeee - - ament_lint_auto - ament_lint_common - - - ament_cmake - - - From 910f1d6c19ff50909beafa528fd50ccbcfb45d86 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 04:40:51 +0900 Subject: [PATCH 14/23] fix: pre-commit --- pre_commit_hooks/ros_include_guard.py | 225 +++++++++++++------------- setup.cfg | 1 + tests/test_ros_include_guard.py | 52 +++--- tests/test_sort_package_xml.py | 40 ++--- 4 files changed, 160 insertions(+), 158 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index 9539650..1195764 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -17,152 +17,149 @@ class DirectiveBase: + def __init__(self, directive): + self.directive = directive + self.line = None + self.text = None + self.expected = None + self.tokens = None - def __init__(self, directive): - self.directive = directive - self.line = None - self.text = None - self.expected = None - self.tokens = None + def update(self, line, text): + self.line = line + self.text = text - def update(self, line, text): - self.line = line - self.text = text + def is_none(self): + return self.line is None - def is_none(self): - return self.line is None + def mismatch(self): + return self.text != self.expected - def mismatch(self): - return self.text != self.expected - - def overwrite(self, lines): - lines[self.line] = self.expected + def overwrite(self, lines): + lines[self.line] = self.expected class OpeningDirective(DirectiveBase): - - def prepare(self, macro_name): - self.tokens = split_space_boundary(self.text) - if 3 <= len(self.tokens): - self.tokens[2] = macro_name - self.expected = ''.join(self.tokens) - else: - self.expected = F'{self.directive} {macro_name}' + def prepare(self, macro_name): + self.tokens = split_space_boundary(self.text) + if 3 <= len(self.tokens): + self.tokens[2] = macro_name + self.expected = "".join(self.tokens) + else: + self.expected = f"{self.directive} {macro_name}" class ClosingDirective(DirectiveBase): - - def prepare(self, macro_name): - self.tokens = split_space_boundary(self.text) - if 5 <= len(self.tokens): - self.tokens[4] = macro_name - self.expected = ''.join(self.tokens) - else: - self.expected = F'{self.directive} // {macro_name}' + def prepare(self, macro_name): + self.tokens = split_space_boundary(self.text) + if 5 <= len(self.tokens): + self.tokens[4] = macro_name + self.expected = "".join(self.tokens) + else: + self.expected = f"{self.directive} // {macro_name}" class IncludeGuard: + def __init__(self): + self.ifndef = OpeningDirective("#ifndef") + self.define = OpeningDirective("#define") + self.endif = ClosingDirective("#endif") + self.pragma = False - def __init__(self): - self.ifndef = OpeningDirective('#ifndef') - self.define = OpeningDirective('#define') - self.endif = ClosingDirective('#endif') - self.pragma = False - - def items(self): - yield self.ifndef - yield self.define - yield self.endif + def items(self): + yield self.ifndef + yield self.define + yield self.endif - def is_none(self): - return any(item.is_none() for item in self.items()) + def is_none(self): + return any(item.is_none() for item in self.items()) - def mismatch(self): - return any(item.mismatch() for item in self.items()) + def mismatch(self): + return any(item.mismatch() for item in self.items()) - def overwrite(self, lines): - return any(item.overwrite(lines) for item in self.items()) + def overwrite(self, lines): + return any(item.overwrite(lines) for item in self.items()) - def prepare(self, macro_name): - for item in self.items(): item.prepare(macro_name) + def prepare(self, macro_name): + for item in self.items(): + item.prepare(macro_name) -def split_space_boundary(text, delimiters={' '}): +def split_space_boundary(text, delimiters={" "}): - result = [] - prev = None - for char in text: - curr = char in delimiters - if curr != prev: - result.append(char) - else: - result[-1] += char - prev = curr - return result + result = [] + prev = None + for char in text: + curr = char in delimiters + if curr != prev: + result.append(char) + else: + result[-1] += char + prev = curr + return result def get_include_guard_info(lines): - guard = IncludeGuard() - for line, text in enumerate(lines): - if text.startswith('#pragma once'): - guard.pragma = True - if text.startswith('#ifndef') and guard.ifndef.is_none(): - guard.ifndef.update(line, text) - if text.startswith('#define') and guard.define.is_none(): - guard.define.update(line, text) - if text.startswith('#endif'): - guard.endif.update(line, text) - return guard + guard = IncludeGuard() + for line, text in enumerate(lines): + if text.startswith("#pragma once"): + guard.pragma = True + if text.startswith("#ifndef") and guard.ifndef.is_none(): + guard.ifndef.update(line, text) + if text.startswith("#define") and guard.define.is_none(): + guard.define.update(line, text) + if text.startswith("#endif"): + guard.endif.update(line, text) + return guard def get_parts_after(parts, targets): - result = [] - for part in reversed(parts): - if part in targets: - break - result.append(part) - return reversed(result) + result = [] + for part in reversed(parts): + if part in targets: + break + result.append(part) + return reversed(result) def get_include_guard_macro_name(filepath): - targets = {'include', 'src', 'test'} - parts = get_parts_after(filepath.parts, targets) - return '__'.join(parts).replace('.', '_').upper() + '_' + targets = {"include", "src", "test"} + parts = get_parts_after(filepath.parts, targets) + return "__".join(parts).replace(".", "_").upper() + "_" def main(argv=None): - parser = argparse.ArgumentParser() - parser.add_argument('filenames', nargs='*', help='Filenames to fix') - args = parser.parse_args(argv) - - return_code = 0 - for filename in args.filenames: - # Search the include guard lines. - filepath = pathlib.Path(filename) - lines = filepath.read_text().split('\n') - guard = get_include_guard_info(lines) - # Skip if the file using pragma once. - if guard.pragma: - continue - # Error if the include guard is not found. - if guard.is_none(): - print('No include guard in {}'.format(filepath)) - return_code = 1 - continue - # Error and auto fix if the macro name is not correct. - macro_name = get_include_guard_macro_name(filepath) - guard.prepare(macro_name) - if guard.mismatch(): - print('Fix include guard in {}'.format(filepath)) - return_code = 1 - guard.overwrite(lines) - filepath.write_text('\n'.join(lines)) - continue - - return return_code - - -if __name__ == '__main__': - exit(main()) + parser = argparse.ArgumentParser() + parser.add_argument("filenames", nargs="*", help="Filenames to fix") + args = parser.parse_args(argv) + + return_code = 0 + for filename in args.filenames: + # Search the include guard lines. + filepath = pathlib.Path(filename) + lines = filepath.read_text().split("\n") + guard = get_include_guard_info(lines) + # Skip if the file using pragma once. + if guard.pragma: + continue + # Error if the include guard is not found. + if guard.is_none(): + print("No include guard in {}".format(filepath)) + return_code = 1 + continue + # Error and auto fix if the macro name is not correct. + macro_name = get_include_guard_macro_name(filepath) + guard.prepare(macro_name) + if guard.mismatch(): + print("Fix include guard in {}".format(filepath)) + return_code = 1 + guard.overwrite(lines) + filepath.write_text("\n".join(lines)) + continue + + return return_code + + +if __name__ == "__main__": + exit(main()) diff --git a/setup.cfg b/setup.cfg index 9697f48..586bdf9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,6 +17,7 @@ console_scripts = [flake8] # Modified from https://github.com/ament/ament_lint/blob/ebd524bb9973d5ec1dc48a670ce54f958a5a0243/ament_flake8/ament_flake8/configuration/ament_flake8.ini +application_import_names = pre_commit_hooks extend-ignore = B902,C816,D100,D101,D102,D103,D104,D105,D106,D107,D203,D212,D404,I202,CNL100,E203,E501,Q000 import-order-style = google max-line-length = 100 diff --git a/tests/test_ros_include_guard.py b/tests/test_ros_include_guard.py index 0cbde2e..1cf24fc 100644 --- a/tests/test_ros_include_guard.py +++ b/tests/test_ros_include_guard.py @@ -1,37 +1,39 @@ -import pytest -import shutil from pathlib import Path +import shutil + +import pytest + from pre_commit_hooks import ros_include_guard files = [ - 'include/rospkg/foobar.h', - 'include/rospkg/foobar.hpp', - 'include/rospkg/nolint.hpp', - 'src/foo_bar_baz.hpp', - 'src/foo_bar/baz.hpp', - 'src/foo/bar_baz.hpp', - 'src/foo/bar/baz.hpp', + "include/rospkg/foobar.h", + "include/rospkg/foobar.hpp", + "include/rospkg/nolint.hpp", + "src/foo_bar_baz.hpp", + "src/foo_bar/baz.hpp", + "src/foo/bar_baz.hpp", + "src/foo/bar/baz.hpp", ] cases = [] for file in files: - cases.append((Path(file), 0)) - cases.append((Path(file), 1)) + cases.append((Path(file), 0)) + cases.append((Path(file), 1)) -@pytest.mark.parametrize(('target_file', 'answer_code'), cases) +@pytest.mark.parametrize(("target_file", "answer_code"), cases) def test(target_file, answer_code, datadir): - ok_ng = '.ng' if answer_code else '.ok' - source_file = target_file.with_suffix(ok_ng + target_file.suffix) - answer_file = target_file.with_suffix('.ok' + target_file.suffix) - target_path = datadir.joinpath(target_file) - source_path = datadir.joinpath(source_file) - answer_path = datadir.joinpath(answer_file) - shutil.copy(source_path, target_path) - - return_code = ros_include_guard.main([str(target_path)]) - target_text = target_path.read_text() - answer_text = answer_path.read_text() - assert return_code == answer_code - assert target_text == answer_text + ok_ng = ".ng" if answer_code else ".ok" + source_file = target_file.with_suffix(ok_ng + target_file.suffix) + answer_file = target_file.with_suffix(".ok" + target_file.suffix) + target_path = datadir.joinpath(target_file) + source_path = datadir.joinpath(source_file) + answer_path = datadir.joinpath(answer_file) + shutil.copy(source_path, target_path) + + return_code = ros_include_guard.main([str(target_path)]) + target_text = target_path.read_text() + answer_text = answer_path.read_text() + assert return_code == answer_code + assert target_text == answer_text diff --git a/tests/test_sort_package_xml.py b/tests/test_sort_package_xml.py index 1cd570e..ce41ba8 100644 --- a/tests/test_sort_package_xml.py +++ b/tests/test_sort_package_xml.py @@ -1,31 +1,33 @@ -import pytest -import shutil from pathlib import Path +import shutil + +import pytest + from pre_commit_hooks import sort_package_xml files = [ - 'package.xml', + "package.xml", ] cases = [] for file in files: - cases.append((Path(file), 0)) - cases.append((Path(file), 1)) + cases.append((Path(file), 0)) + cases.append((Path(file), 1)) -@pytest.mark.parametrize(('target_file', 'answer_code'), cases) +@pytest.mark.parametrize(("target_file", "answer_code"), cases) def test(target_file, answer_code, datadir): - ok_ng = '.ng' if answer_code else '.ok' - source_file = target_file.with_suffix(ok_ng + target_file.suffix) - answer_file = target_file.with_suffix('.ok' + target_file.suffix) - target_path = datadir.joinpath(target_file) - source_path = datadir.joinpath(source_file) - answer_path = datadir.joinpath(answer_file) - shutil.copy(source_path, target_path) - - return_code = sort_package_xml.main([str(target_path)]) - target_text = target_path.read_text() - answer_text = answer_path.read_text() - assert return_code == answer_code - assert target_text == answer_text + ok_ng = ".ng" if answer_code else ".ok" + source_file = target_file.with_suffix(ok_ng + target_file.suffix) + answer_file = target_file.with_suffix(".ok" + target_file.suffix) + target_path = datadir.joinpath(target_file) + source_path = datadir.joinpath(source_file) + answer_path = datadir.joinpath(answer_file) + shutil.copy(source_path, target_path) + + return_code = sort_package_xml.main([str(target_path)]) + target_text = target_path.read_text() + answer_text = answer_path.read_text() + assert return_code == answer_code + assert target_text == answer_text From a65eb9b838df4e7132f7a967cee0a25f87ca46a2 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 04:41:58 +0900 Subject: [PATCH 15/23] fix: spell --- pre_commit_hooks/ros_include_guard.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index 1195764..f34ca52 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -89,12 +89,12 @@ def split_space_boundary(text, delimiters={" "}): result = [] prev = None for char in text: - curr = char in delimiters - if curr != prev: + current = char in delimiters + if current != prev: result.append(char) else: result[-1] += char - prev = curr + prev = current return result From 7b37cb7f7139e63353787a872da26d110eec78f8 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Wed, 23 Feb 2022 04:43:57 +0900 Subject: [PATCH 16/23] fix: pytest-datadir --- .github/workflows/pytest.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index f1d2894..68ca151 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -11,7 +11,7 @@ jobs: uses: actions/checkout@v2 - name: Install pytest - run: python3 -m pip install pytest + run: python3 -m pip install pytest pytest-datadir - name: Run pytest run: python3 -m pytest From 18ed048775c5e43282fad3ad02d0ec7b27b40e96 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 14:20:37 +0900 Subject: [PATCH 17/23] Fix: remove unused file --- tests/resources/rospkg3/include/rospkg/.hpp | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tests/resources/rospkg3/include/rospkg/.hpp diff --git a/tests/resources/rospkg3/include/rospkg/.hpp b/tests/resources/rospkg3/include/rospkg/.hpp deleted file mode 100644 index dba637f..0000000 --- a/tests/resources/rospkg3/include/rospkg/.hpp +++ /dev/null @@ -1,3 +0,0 @@ -#ifndef ROSPKG__FOOBAR_HPP_ -#define ROSPKG__FOOBAR_HPP_ -#endif // ROSPKG__FOOBAR_HPP_ From e6663a2ba8d6f1d4fefc0defc69acd1bf7017696 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 14:21:27 +0900 Subject: [PATCH 18/23] fix: move the test for sort-package-xml --- tests/test_sort_package_xml.py | 33 ---------------------- tests/test_sort_package_xml/package.ng.xml | 25 ---------------- tests/test_sort_package_xml/package.ok.xml | 25 ---------------- 3 files changed, 83 deletions(-) delete mode 100644 tests/test_sort_package_xml.py delete mode 100644 tests/test_sort_package_xml/package.ng.xml delete mode 100644 tests/test_sort_package_xml/package.ok.xml diff --git a/tests/test_sort_package_xml.py b/tests/test_sort_package_xml.py deleted file mode 100644 index ce41ba8..0000000 --- a/tests/test_sort_package_xml.py +++ /dev/null @@ -1,33 +0,0 @@ -from pathlib import Path -import shutil - -import pytest - -from pre_commit_hooks import sort_package_xml - -files = [ - "package.xml", -] - -cases = [] -for file in files: - cases.append((Path(file), 0)) - cases.append((Path(file), 1)) - - -@pytest.mark.parametrize(("target_file", "answer_code"), cases) -def test(target_file, answer_code, datadir): - - ok_ng = ".ng" if answer_code else ".ok" - source_file = target_file.with_suffix(ok_ng + target_file.suffix) - answer_file = target_file.with_suffix(".ok" + target_file.suffix) - target_path = datadir.joinpath(target_file) - source_path = datadir.joinpath(source_file) - answer_path = datadir.joinpath(answer_file) - shutil.copy(source_path, target_path) - - return_code = sort_package_xml.main([str(target_path)]) - target_text = target_path.read_text() - answer_text = answer_path.read_text() - assert return_code == answer_code - assert target_text == answer_text diff --git a/tests/test_sort_package_xml/package.ng.xml b/tests/test_sort_package_xml/package.ng.xml deleted file mode 100644 index ff2097c..0000000 --- a/tests/test_sort_package_xml/package.ng.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - ccccc - bbbbb - ddddd - eeeee - aaaaa - - ament_lint_common - ament_lint_auto - - - ament_cmake - - - diff --git a/tests/test_sort_package_xml/package.ok.xml b/tests/test_sort_package_xml/package.ok.xml deleted file mode 100644 index bd15183..0000000 --- a/tests/test_sort_package_xml/package.ok.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - rospkg - 0.0.0 - The dummy package to test pre-commit hooks - Takagi, Isamu - Apache License 2.0 - - ament_cmake - - aaaaa - bbbbb - ccccc - ddddd - eeeee - - ament_lint_auto - ament_lint_common - - - ament_cmake - - - From 9a7b84e5450faa71584a63bb44b5646cd4ddf463 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 14:44:03 +0900 Subject: [PATCH 19/23] fix: rename test file and merge test cases --- tests/test_ros_include_guard.py | 34 ++++++++----------- .../rospkg/{foobar.ok.h => foobar.right.h} | 0 .../{foobar.ok.hpp => foobar.right.hpp} | 0 .../rospkg/{foobar.ng.h => foobar.wrong.h} | 0 .../{foobar.ng.hpp => foobar.wrong.hpp} | 0 .../{nolint.ok.hpp => nolint.right.hpp} | 0 .../{nolint.ng.hpp => nolint.wrong.hpp} | 0 .../src/foo/bar/{baz.ok.hpp => baz.right.hpp} | 0 .../src/foo/bar/{baz.ng.hpp => baz.wrong.hpp} | 0 .../foo/{bar_baz.ok.hpp => bar_baz.right.hpp} | 0 .../foo/{bar_baz.ng.hpp => bar_baz.wrong.hpp} | 0 .../src/foo_bar/{baz.ok.hpp => baz.right.hpp} | 0 .../src/foo_bar/{baz.ng.hpp => baz.wrong.hpp} | 0 ...o_bar_baz.ok.hpp => foo_bar_baz.right.hpp} | 0 ...o_bar_baz.ng.hpp => foo_bar_baz.wrong.hpp} | 0 15 files changed, 15 insertions(+), 19 deletions(-) rename tests/test_ros_include_guard/include/rospkg/{foobar.ok.h => foobar.right.h} (100%) rename tests/test_ros_include_guard/include/rospkg/{foobar.ok.hpp => foobar.right.hpp} (100%) rename tests/test_ros_include_guard/include/rospkg/{foobar.ng.h => foobar.wrong.h} (100%) rename tests/test_ros_include_guard/include/rospkg/{foobar.ng.hpp => foobar.wrong.hpp} (100%) rename tests/test_ros_include_guard/include/rospkg/{nolint.ok.hpp => nolint.right.hpp} (100%) rename tests/test_ros_include_guard/include/rospkg/{nolint.ng.hpp => nolint.wrong.hpp} (100%) rename tests/test_ros_include_guard/src/foo/bar/{baz.ok.hpp => baz.right.hpp} (100%) rename tests/test_ros_include_guard/src/foo/bar/{baz.ng.hpp => baz.wrong.hpp} (100%) rename tests/test_ros_include_guard/src/foo/{bar_baz.ok.hpp => bar_baz.right.hpp} (100%) rename tests/test_ros_include_guard/src/foo/{bar_baz.ng.hpp => bar_baz.wrong.hpp} (100%) rename tests/test_ros_include_guard/src/foo_bar/{baz.ok.hpp => baz.right.hpp} (100%) rename tests/test_ros_include_guard/src/foo_bar/{baz.ng.hpp => baz.wrong.hpp} (100%) rename tests/test_ros_include_guard/src/{foo_bar_baz.ok.hpp => foo_bar_baz.right.hpp} (100%) rename tests/test_ros_include_guard/src/{foo_bar_baz.ng.hpp => foo_bar_baz.wrong.hpp} (100%) diff --git a/tests/test_ros_include_guard.py b/tests/test_ros_include_guard.py index 1cf24fc..f17da82 100644 --- a/tests/test_ros_include_guard.py +++ b/tests/test_ros_include_guard.py @@ -1,11 +1,10 @@ -from pathlib import Path import shutil import pytest from pre_commit_hooks import ros_include_guard -files = [ +cases = [ "include/rospkg/foobar.h", "include/rospkg/foobar.hpp", "include/rospkg/nolint.hpp", @@ -15,25 +14,22 @@ "src/foo/bar/baz.hpp", ] -cases = [] -for file in files: - cases.append((Path(file), 0)) - cases.append((Path(file), 1)) +@pytest.mark.parametrize(("target_file"), cases) +def test(target_file, datadir): -@pytest.mark.parametrize(("target_file", "answer_code"), cases) -def test(target_file, answer_code, datadir): - - ok_ng = ".ng" if answer_code else ".ok" - source_file = target_file.with_suffix(ok_ng + target_file.suffix) - answer_file = target_file.with_suffix(".ok" + target_file.suffix) target_path = datadir.joinpath(target_file) - source_path = datadir.joinpath(source_file) - answer_path = datadir.joinpath(answer_file) - shutil.copy(source_path, target_path) + right_path = target_path.with_suffix(".right" + target_path.suffix) + wrong_path = target_path.with_suffix(".wrong" + target_path.suffix) + + # Test wrong file. + shutil.copy(wrong_path, target_path) + return_code = ros_include_guard.main([str(target_path)]) + assert return_code == 1 + assert target_path.read_text() == right_path.read_text() + # Test right file. + shutil.copy(right_path, target_path) return_code = ros_include_guard.main([str(target_path)]) - target_text = target_path.read_text() - answer_text = answer_path.read_text() - assert return_code == answer_code - assert target_text == answer_text + assert return_code == 0 + assert target_path.read_text() == right_path.read_text() diff --git a/tests/test_ros_include_guard/include/rospkg/foobar.ok.h b/tests/test_ros_include_guard/include/rospkg/foobar.right.h similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/foobar.ok.h rename to tests/test_ros_include_guard/include/rospkg/foobar.right.h diff --git a/tests/test_ros_include_guard/include/rospkg/foobar.ok.hpp b/tests/test_ros_include_guard/include/rospkg/foobar.right.hpp similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/foobar.ok.hpp rename to tests/test_ros_include_guard/include/rospkg/foobar.right.hpp diff --git a/tests/test_ros_include_guard/include/rospkg/foobar.ng.h b/tests/test_ros_include_guard/include/rospkg/foobar.wrong.h similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/foobar.ng.h rename to tests/test_ros_include_guard/include/rospkg/foobar.wrong.h diff --git a/tests/test_ros_include_guard/include/rospkg/foobar.ng.hpp b/tests/test_ros_include_guard/include/rospkg/foobar.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/foobar.ng.hpp rename to tests/test_ros_include_guard/include/rospkg/foobar.wrong.hpp diff --git a/tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp b/tests/test_ros_include_guard/include/rospkg/nolint.right.hpp similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/nolint.ok.hpp rename to tests/test_ros_include_guard/include/rospkg/nolint.right.hpp diff --git a/tests/test_ros_include_guard/include/rospkg/nolint.ng.hpp b/tests/test_ros_include_guard/include/rospkg/nolint.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/include/rospkg/nolint.ng.hpp rename to tests/test_ros_include_guard/include/rospkg/nolint.wrong.hpp diff --git a/tests/test_ros_include_guard/src/foo/bar/baz.ok.hpp b/tests/test_ros_include_guard/src/foo/bar/baz.right.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo/bar/baz.ok.hpp rename to tests/test_ros_include_guard/src/foo/bar/baz.right.hpp diff --git a/tests/test_ros_include_guard/src/foo/bar/baz.ng.hpp b/tests/test_ros_include_guard/src/foo/bar/baz.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo/bar/baz.ng.hpp rename to tests/test_ros_include_guard/src/foo/bar/baz.wrong.hpp diff --git a/tests/test_ros_include_guard/src/foo/bar_baz.ok.hpp b/tests/test_ros_include_guard/src/foo/bar_baz.right.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo/bar_baz.ok.hpp rename to tests/test_ros_include_guard/src/foo/bar_baz.right.hpp diff --git a/tests/test_ros_include_guard/src/foo/bar_baz.ng.hpp b/tests/test_ros_include_guard/src/foo/bar_baz.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo/bar_baz.ng.hpp rename to tests/test_ros_include_guard/src/foo/bar_baz.wrong.hpp diff --git a/tests/test_ros_include_guard/src/foo_bar/baz.ok.hpp b/tests/test_ros_include_guard/src/foo_bar/baz.right.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo_bar/baz.ok.hpp rename to tests/test_ros_include_guard/src/foo_bar/baz.right.hpp diff --git a/tests/test_ros_include_guard/src/foo_bar/baz.ng.hpp b/tests/test_ros_include_guard/src/foo_bar/baz.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo_bar/baz.ng.hpp rename to tests/test_ros_include_guard/src/foo_bar/baz.wrong.hpp diff --git a/tests/test_ros_include_guard/src/foo_bar_baz.ok.hpp b/tests/test_ros_include_guard/src/foo_bar_baz.right.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo_bar_baz.ok.hpp rename to tests/test_ros_include_guard/src/foo_bar_baz.right.hpp diff --git a/tests/test_ros_include_guard/src/foo_bar_baz.ng.hpp b/tests/test_ros_include_guard/src/foo_bar_baz.wrong.hpp similarity index 100% rename from tests/test_ros_include_guard/src/foo_bar_baz.ng.hpp rename to tests/test_ros_include_guard/src/foo_bar_baz.wrong.hpp From f32eaf547b6dc435af99df2a9b7fcc36149cc934 Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 18:47:02 +0900 Subject: [PATCH 20/23] fix: add comment and fix variable name --- pre_commit_hooks/ros_include_guard.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index f34ca52..fccb7b8 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -41,7 +41,7 @@ def overwrite(self, lines): class OpeningDirective(DirectiveBase): def prepare(self, macro_name): self.tokens = split_space_boundary(self.text) - if 3 <= len(self.tokens): + if 3 <= len(self.tokens): # 3 is len(directive, space, macro_name) self.tokens[2] = macro_name self.expected = "".join(self.tokens) else: @@ -51,7 +51,7 @@ def prepare(self, macro_name): class ClosingDirective(DirectiveBase): def prepare(self, macro_name): self.tokens = split_space_boundary(self.text) - if 5 <= len(self.tokens): + if 5 <= len(self.tokens): # 5 is len(directive, space, //, space, macro_name) self.tokens[4] = macro_name self.expected = "".join(self.tokens) else: @@ -63,7 +63,7 @@ def __init__(self): self.ifndef = OpeningDirective("#ifndef") self.define = OpeningDirective("#define") self.endif = ClosingDirective("#endif") - self.pragma = False + self.has_pragma_once = False def items(self): yield self.ifndef @@ -103,7 +103,7 @@ def get_include_guard_info(lines): guard = IncludeGuard() for line, text in enumerate(lines): if text.startswith("#pragma once"): - guard.pragma = True + guard.has_pragma_once = True if text.startswith("#ifndef") and guard.ifndef.is_none(): guard.ifndef.update(line, text) if text.startswith("#define") and guard.define.is_none(): @@ -140,8 +140,13 @@ def main(argv=None): filepath = pathlib.Path(filename) lines = filepath.read_text().split("\n") guard = get_include_guard_info(lines) - # Skip if the file using pragma once. - if guard.pragma: + # Error if there are include guard and pragma once. + if guard.has_pragma_once and not guard.is_none(): + print("There are include guard and pragma once in {}".format(filepath)) + return_code = 1 + continue + # Skip if the file uses pragma once. + if guard.has_pragma_once: continue # Error if the include guard is not found. if guard.is_none(): From 15d1af7fbb4249dd4f9d091b0900afe273dbd44e Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 18:48:50 +0900 Subject: [PATCH 21/23] fix: import --- pre_commit_hooks/ros_include_guard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index fccb7b8..45b25a9 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -13,7 +13,7 @@ # limitations under the License. import argparse -import pathlib +from pathlib import Path class DirectiveBase: @@ -137,7 +137,7 @@ def main(argv=None): return_code = 0 for filename in args.filenames: # Search the include guard lines. - filepath = pathlib.Path(filename) + filepath = Path(filename) lines = filepath.read_text().split("\n") guard = get_include_guard_info(lines) # Error if there are include guard and pragma once. From 04cf4719e85c87a2b7b5a52a6841fd5be8945fea Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 18:55:06 +0900 Subject: [PATCH 22/23] fix: pragma once --- pre_commit_hooks/ros_include_guard.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index 45b25a9..739ba18 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -140,16 +140,8 @@ def main(argv=None): filepath = Path(filename) lines = filepath.read_text().split("\n") guard = get_include_guard_info(lines) - # Error if there are include guard and pragma once. - if guard.has_pragma_once and not guard.is_none(): - print("There are include guard and pragma once in {}".format(filepath)) - return_code = 1 - continue - # Skip if the file uses pragma once. - if guard.has_pragma_once: - continue # Error if the include guard is not found. - if guard.is_none(): + if guard.is_none() and not guard.has_pragma_once: print("No include guard in {}".format(filepath)) return_code = 1 continue From 2822002473e76e7aca99b39199622cc353bad78d Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 24 Feb 2022 19:13:34 +0900 Subject: [PATCH 23/23] fix: remove tokens --- pre_commit_hooks/ros_include_guard.py | 33 +++++---------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/pre_commit_hooks/ros_include_guard.py b/pre_commit_hooks/ros_include_guard.py index 739ba18..78d5d77 100644 --- a/pre_commit_hooks/ros_include_guard.py +++ b/pre_commit_hooks/ros_include_guard.py @@ -22,7 +22,6 @@ def __init__(self, directive): self.line = None self.text = None self.expected = None - self.tokens = None def update(self, line, text): self.line = line @@ -40,22 +39,16 @@ def overwrite(self, lines): class OpeningDirective(DirectiveBase): def prepare(self, macro_name): - self.tokens = split_space_boundary(self.text) - if 3 <= len(self.tokens): # 3 is len(directive, space, macro_name) - self.tokens[2] = macro_name - self.expected = "".join(self.tokens) - else: - self.expected = f"{self.directive} {macro_name}" + self.expected = f"{self.directive} {macro_name}" + if "// NOLINT" in self.text: + self.expected += " // NOLINT" class ClosingDirective(DirectiveBase): def prepare(self, macro_name): - self.tokens = split_space_boundary(self.text) - if 5 <= len(self.tokens): # 5 is len(directive, space, //, space, macro_name) - self.tokens[4] = macro_name - self.expected = "".join(self.tokens) - else: - self.expected = f"{self.directive} // {macro_name}" + self.expected = f"{self.directive} // {macro_name}" + if "// NOLINT" in self.text: + self.expected += " // NOLINT" class IncludeGuard: @@ -84,20 +77,6 @@ def prepare(self, macro_name): item.prepare(macro_name) -def split_space_boundary(text, delimiters={" "}): - - result = [] - prev = None - for char in text: - current = char in delimiters - if current != prev: - result.append(char) - else: - result[-1] += char - prev = current - return result - - def get_include_guard_info(lines): guard = IncludeGuard()