From 367bbf21c37e5d549a8892de9230d666c37e89e2 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Wed, 25 Mar 2026 22:02:46 -0700 Subject: [PATCH 1/6] ruff impl Signed-off-by: Michael Carlstrom --- ament_pep257/ament_pep257/main.py | 117 +++++------------- ament_pep257/ament_pep257/pydocstyle_impl.py | 103 +++++++++++++++ ament_pep257/ament_pep257/ruff_impl.py | 90 ++++++++++++++ ament_pep257/package.xml | 3 + ament_pep257/test/test_ament_pep257.py | 34 +++++ .../test/test_generate_pep257_report.py | 11 +- .../test/test_generate_ruff_report.py | 52 ++++++++ 7 files changed, 319 insertions(+), 91 deletions(-) create mode 100644 ament_pep257/ament_pep257/pydocstyle_impl.py create mode 100644 ament_pep257/ament_pep257/ruff_impl.py create mode 100644 ament_pep257/test/test_ament_pep257.py create mode 100644 ament_pep257/test/test_generate_ruff_report.py diff --git a/ament_pep257/ament_pep257/main.py b/ament_pep257/ament_pep257/main.py index fb573ba8b..10eff4c22 100755 --- a/ament_pep257/ament_pep257/main.py +++ b/ament_pep257/ament_pep257/main.py @@ -14,32 +14,33 @@ # See the License for the specific language governing permissions and # limitations under the License. + import argparse -import logging +from importlib.util import find_spec import os +import shutil import sys import time from typing import Literal from xml.sax.saxutils import escape from xml.sax.saxutils import quoteattr -import pydocstyle -from pydocstyle import check - -try: # as of version 1.1.0 - from pydocstyle.config import ConfigurationParser - from pydocstyle.violations import Error - from pydocstyle.utils import log -except ImportError: # try version 1.0.0 - from pydocstyle import ConfigurationParser - from pydocstyle import Error - from pydocstyle import log - -log.setLevel(logging.INFO) +ruff_installed = False +pydocstyle_installed = False +if shutil.which('ruff'): + ruff_installed = True +else: + ruff_installed = False -_conventions = set(pydocstyle.conventions.keys()) -_conventions.add('ament') +if find_spec('pydocstyle'): + pydocstyle_installed = True + import pydocstyle + _conventions = set(pydocstyle.conventions.keys()) + _conventions.add('ament') +else: + pydocstyle_installed = False + _conventions = {'pep257', 'numpy', 'google', 'ament'} _ament_ignore = [ 'D100', @@ -123,8 +124,18 @@ def main(argv: list[str] = sys.argv[1:]) -> Literal[0, 1]: args.ignore = ','.join(_ament_ignore) excludes = [os.path.abspath(e) for e in args.excludes] - report = generate_pep257_report(args.paths, excludes, args.ignore, args.select, - args.convention, args.add_ignore, args.add_select) + + if ruff_installed: + from ament_pep257.ruff_impl import generate_ruff_report + report = generate_ruff_report(args.paths, excludes, args.ignore, args.select, + args.convention, args.add_ignore, args.add_select) + elif pydocstyle_installed: + from ament_pep257.pydocstyle_impl import generate_pep257_report + report = generate_pep257_report(args.paths, excludes, args.ignore, args.select, + args.convention, args.add_ignore, args.add_select) + else: + print('Neither ruff or pydocstyle installed') + return 1 error_count = sum(len(r[1]) for r in report) # print summary @@ -162,76 +173,6 @@ def _filename_in_excludes(filename, excludes): return any(os.path.commonpath([absname, e]) == e for e in excludes) -def generate_pep257_report(paths, excludes, ignore, select, convention, add_ignore, add_select): - conf = ConfigurationParser() - sys_argv = sys.argv - sys.argv = [ - 'main', - '--match', r'.*\.py', - '--match-dir', r'[^\._].*', - ] - if ignore: - sys.argv += ['--ignore', ignore] - elif select: - sys.argv += ['--select', select] - else: - sys.argv += ['--convention', convention] - if add_ignore: - sys.argv += ['--add-ignore', add_ignore] - if add_select: - sys.argv += ['--add-select', add_select] - sys.argv += paths - conf.parse() - sys.argv = sys_argv - files_to_check = conf.get_files_to_check() - - report = [] - - files_dict = {} - # Unpack 3 values for pydocstyle <= 6.1.1 and 4 values for pydocstyle >= 6.2.0 - for filename, checked_codes, ignore_decorators, *_ in files_to_check: - if _filename_in_excludes(filename, excludes): - continue - files_dict[filename] = { - 'select': checked_codes, - 'ignore_decorators': ignore_decorators, - } - - for filename in sorted(files_dict.keys()): - print('checking', filename) - errors = [] - pep257_errors = check( - [filename], - **files_dict[filename]) - for pep257_error in pep257_errors: - if isinstance(pep257_error, Error): - errors.append({ - 'category': pep257_error.code, - 'linenumber': pep257_error.line, - 'message': pep257_error.message, - }) - print( - '%s:%d %s: %s' % - (pep257_error.filename, pep257_error.line, pep257_error.definition, - pep257_error.message), file=sys.stderr) - elif isinstance(pep257_error, SyntaxError): - errors.append({ - 'category': str(type(pep257_error)), - 'linenumber': '-', - 'message': 'invalid syntax in file', - }) - print('%s: invalid syntax' % filename, file=sys.stderr) - else: - errors.append({ - 'category': 'unknown', - 'linenumber': '-', - 'message': str(pep257_error), - }) - print('%s: %s' % (filename, pep257_error), file=sys.stderr) - report.append((filename, errors)) - return report - - def get_xunit_content(report, testname, elapsed): test_count = sum(max(len(r[1]), 1) for r in report) error_count = sum(len(r[1]) for r in report) diff --git a/ament_pep257/ament_pep257/pydocstyle_impl.py b/ament_pep257/ament_pep257/pydocstyle_impl.py new file mode 100644 index 000000000..8c6815645 --- /dev/null +++ b/ament_pep257/ament_pep257/pydocstyle_impl.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 + +# Copyright 2026 Open Source Robotics Foundation, Inc. +# +# 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 logging +import sys + +from ament_pep257.main import _filename_in_excludes +from pydocstyle import check + +try: # as of version 1.1.0 + from pydocstyle.config import ConfigurationParser + from pydocstyle.violations import Error + from pydocstyle.utils import log +except ImportError: # try version 1.0.0 + from pydocstyle import ConfigurationParser + from pydocstyle import Error + from pydocstyle import log + + +log.setLevel(logging.INFO) + + +def generate_pep257_report(paths, excludes, ignore, select, convention, add_ignore, add_select): + conf = ConfigurationParser() + sys_argv = sys.argv + sys.argv = [ + 'main', + '--match', r'.*\.py', + '--match-dir', r'[^\._].*', + ] + if ignore: + sys.argv += ['--ignore', ignore] + elif select: + sys.argv += ['--select', select] + else: + sys.argv += ['--convention', convention] + if add_ignore: + sys.argv += ['--add-ignore', add_ignore] + if add_select: + sys.argv += ['--add-select', add_select] + sys.argv += paths + conf.parse() + sys.argv = sys_argv + files_to_check = conf.get_files_to_check() + + report = [] + + files_dict = {} + # Unpack 3 values for pydocstyle <= 6.1.1 and 4 values for pydocstyle >= 6.2.0 + for filename, checked_codes, ignore_decorators, *_ in files_to_check: + if _filename_in_excludes(filename, excludes): + continue + files_dict[filename] = { + 'select': checked_codes, + 'ignore_decorators': ignore_decorators, + } + + for filename in sorted(files_dict.keys()): + print('checking', filename) + errors = [] + pep257_errors = check( + [filename], + **files_dict[filename]) + for pep257_error in pep257_errors: + if isinstance(pep257_error, Error): + errors.append({ + 'category': pep257_error.code, + 'linenumber': pep257_error.line, + 'message': pep257_error.message, + }) + print( + '%s:%d %s: %s' % + (pep257_error.filename, pep257_error.line, pep257_error.definition, + pep257_error.message), file=sys.stderr) + elif isinstance(pep257_error, SyntaxError): + errors.append({ + 'category': str(type(pep257_error)), + 'linenumber': '-', + 'message': 'invalid syntax in file', + }) + print('%s: invalid syntax' % filename, file=sys.stderr) + else: + errors.append({ + 'category': 'unknown', + 'linenumber': '-', + 'message': str(pep257_error), + }) + print('%s: %s' % (filename, pep257_error), file=sys.stderr) + report.append((filename, errors)) + return report diff --git a/ament_pep257/ament_pep257/ruff_impl.py b/ament_pep257/ament_pep257/ruff_impl.py new file mode 100644 index 000000000..5161f1f9e --- /dev/null +++ b/ament_pep257/ament_pep257/ruff_impl.py @@ -0,0 +1,90 @@ +# Copyright 2026 Open Source Robotics Foundation, Inc. +# +# 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 json +import os +import subprocess +import sys + +from ament_pep257.main import _filename_in_excludes + + +def generate_ruff_report(paths: str, excludes: str, ignore: str, select: str, convention: str, + add_ignore: str, add_select: str): + cmd = ['ruff', 'check', '--output-format', 'json'] + + if ignore: + cmd += ['--ignore', ignore] + elif select: + cmd += ['--select', select] + else: + cmd += ['--convention', convention] + + if add_ignore: + cmd += ['--ignore', add_ignore] + + if add_select: + cmd += ['--select', add_select] + + # excludes + for e in excludes: + cmd += ['--exclude', e] + + # paths + cmd += paths + + result = subprocess.run( + cmd, + capture_output=True, + text=True, + ) + + data = json.loads(result.stdout) + + report = [] + files_dict = {} + + for item in data: + filename = os.path.abspath(item['filename']) + + if _filename_in_excludes(filename, excludes): + continue + + files_dict.setdefault(filename, []).append(item) + + for filename in sorted(files_dict.keys()): + print('checking', filename) + errors = [] + + for err in files_dict[filename]: + errors.append({ + 'category': err.get('code', 'unknown'), + 'linenumber': err.get('location', {}).get('row', '-'), + 'message': err.get('message', ''), + }) + + print( + '%s:%s %s: %s' % ( + filename, + err.get('location', {}).get('row', '-'), + err.get('code', 'unknown'), + err.get('message', ''), + ), + file=sys.stderr, + ) + + report.append((filename, errors)) + + return report diff --git a/ament_pep257/package.xml b/ament_pep257/package.xml index 6df74cf42..8e5e2bca5 100644 --- a/ament_pep257/package.xml +++ b/ament_pep257/package.xml @@ -20,6 +20,9 @@ Michel Hidalgo ament_lint + + + pydocstyle ament_flake8 diff --git a/ament_pep257/test/test_ament_pep257.py b/ament_pep257/test/test_ament_pep257.py new file mode 100644 index 000000000..fe082c910 --- /dev/null +++ b/ament_pep257/test/test_ament_pep257.py @@ -0,0 +1,34 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# +# 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 pathlib +import tempfile + +from ament_pep257.main import main + + +def test_invalid_file(): + report = main(['non_existent_file.py']) + assert report == 1 + + +def test_valid_file(): + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + py_file = temp_dir / 'foobar.py' + py_file.write_text('a = 1+2\n') + + report = main([str(py_file)]) + + assert report == 0 diff --git a/ament_pep257/test/test_generate_pep257_report.py b/ament_pep257/test/test_generate_pep257_report.py index 1174c3cad..0d5dda5a5 100644 --- a/ament_pep257/test/test_generate_pep257_report.py +++ b/ament_pep257/test/test_generate_pep257_report.py @@ -1,13 +1,13 @@ # Copyright 2023 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the "License"); +# 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, +# 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. @@ -16,10 +16,13 @@ import tempfile from ament_pep257.main import _ament_ignore -from ament_pep257.main import generate_pep257_report +from ament_pep257.main import pydocstyle_installed +import pytest +@pytest.mark.skipif(not pydocstyle_installed, reason='pydocstyle not installed') def test_invalid_file(): + from ament_pep257.pydocstyle_impl import generate_pep257_report ignore = ','.join(_ament_ignore) report = generate_pep257_report(['non_existent_file.py'], [], ignore, [], 'ament', [], []) @@ -32,7 +35,9 @@ def test_invalid_file(): assert error['category'] == 'unknown' +@pytest.mark.skipif(not pydocstyle_installed, reason='pydocstyle not installed') def test_valid_file(): + from ament_pep257.pydocstyle_impl import generate_pep257_report ignore = ','.join(_ament_ignore) with tempfile.TemporaryDirectory() as temp_dir: diff --git a/ament_pep257/test/test_generate_ruff_report.py b/ament_pep257/test/test_generate_ruff_report.py new file mode 100644 index 000000000..7f2b24371 --- /dev/null +++ b/ament_pep257/test/test_generate_ruff_report.py @@ -0,0 +1,52 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# +# 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 pathlib +import tempfile + +from ament_pep257.main import _ament_ignore +from ament_pep257.main import ruff_installed +import pytest + + +@pytest.mark.skipif(not ruff_installed, reason='ruff not installed') +def test_invalid_file(): + from ament_pep257.ruff_impl import generate_ruff_report + ignore = ','.join(_ament_ignore) + + report = generate_ruff_report(['non_existent_file.py'], [], ignore, [], 'ament', [], []) + assert len(report) == 1 + filename, errors = report[0] + assert filename == 'non_existent_file.py' + assert len(errors) == 1 + error = errors[0] + assert error['linenumber'] == '-' + assert error['category'] == 'unknown' + + +@pytest.mark.skipif(not ruff_installed, reason='ruff not installed') +def test_valid_file(): + from ament_pep257.ruff_impl import generate_ruff_report + ignore = ','.join(_ament_ignore) + + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + py_file = temp_dir / 'foobar.py' + py_file.write_text('a = 1+2\n') + + report = generate_ruff_report([str(temp_dir)], [], ignore, [], 'ament', [], []) + + assert len(report) == 1 + filename, errors = report[0] + assert errors == [] From edbe001074ca564bfb9fe2b6b65eefcb2257004d Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Wed, 25 Mar 2026 22:05:35 -0700 Subject: [PATCH 2/6] Fix licencse Signed-off-by: Michael Carlstrom --- ament_pep257/ament_pep257/ruff_impl.py | 4 ++-- ament_pep257/test/test_generate_pep257_report.py | 4 ++-- ament_pep257/test/test_generate_ruff_report.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ament_pep257/ament_pep257/ruff_impl.py b/ament_pep257/ament_pep257/ruff_impl.py index 5161f1f9e..cf28966b4 100644 --- a/ament_pep257/ament_pep257/ruff_impl.py +++ b/ament_pep257/ament_pep257/ruff_impl.py @@ -1,13 +1,13 @@ # Copyright 2026 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the 'License'); +# 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, +# 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. diff --git a/ament_pep257/test/test_generate_pep257_report.py b/ament_pep257/test/test_generate_pep257_report.py index 0d5dda5a5..31cb5aa24 100644 --- a/ament_pep257/test/test_generate_pep257_report.py +++ b/ament_pep257/test/test_generate_pep257_report.py @@ -1,13 +1,13 @@ # Copyright 2023 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the 'License'); +# 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, +# 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. diff --git a/ament_pep257/test/test_generate_ruff_report.py b/ament_pep257/test/test_generate_ruff_report.py index 7f2b24371..fba271a8a 100644 --- a/ament_pep257/test/test_generate_ruff_report.py +++ b/ament_pep257/test/test_generate_ruff_report.py @@ -1,13 +1,13 @@ # Copyright 2023 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the 'License'); +# 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, +# 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. From 8bd565f9deb93cf11cd03f864f8d217383a03058 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Wed, 25 Mar 2026 22:10:27 -0700 Subject: [PATCH 3/6] Fix License capitalization Signed-off-by: Michael Carlstrom --- ament_pep257/ament_pep257/ruff_impl.py | 2 +- ament_pep257/test/test_generate_pep257_report.py | 2 +- ament_pep257/test/test_generate_ruff_report.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ament_pep257/ament_pep257/ruff_impl.py b/ament_pep257/ament_pep257/ruff_impl.py index cf28966b4..6f05b7d23 100644 --- a/ament_pep257/ament_pep257/ruff_impl.py +++ b/ament_pep257/ament_pep257/ruff_impl.py @@ -1,6 +1,6 @@ # Copyright 2026 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the "license"); +# 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 # diff --git a/ament_pep257/test/test_generate_pep257_report.py b/ament_pep257/test/test_generate_pep257_report.py index 31cb5aa24..c3806df60 100644 --- a/ament_pep257/test/test_generate_pep257_report.py +++ b/ament_pep257/test/test_generate_pep257_report.py @@ -1,6 +1,6 @@ # Copyright 2023 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the "license"); +# 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 # diff --git a/ament_pep257/test/test_generate_ruff_report.py b/ament_pep257/test/test_generate_ruff_report.py index fba271a8a..00f8d81c3 100644 --- a/ament_pep257/test/test_generate_ruff_report.py +++ b/ament_pep257/test/test_generate_ruff_report.py @@ -1,6 +1,6 @@ # Copyright 2023 Open Source Robotics Foundation, Inc. # -# Licensed under the Apache License, Version 2.0 (the "license"); +# 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 # From 54651b9ee0ae072bce2d36b265683a902da5b0a6 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 26 Mar 2026 09:08:17 -0700 Subject: [PATCH 4/6] Add positive flag test cases Signed-off-by: Michael Carlstrom --- ament_pep257/ament_pep257/ruff_impl.py | 42 +++++++++++++++++++++++--- ament_pep257/test/test_ament_pep257.py | 16 ++++++++++ ament_pep257/test/test_pep257.py | 23 ++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 ament_pep257/test/test_pep257.py diff --git a/ament_pep257/ament_pep257/ruff_impl.py b/ament_pep257/ament_pep257/ruff_impl.py index 6f05b7d23..4176f0ae5 100644 --- a/ament_pep257/ament_pep257/ruff_impl.py +++ b/ament_pep257/ament_pep257/ruff_impl.py @@ -21,8 +21,25 @@ from ament_pep257.main import _filename_in_excludes -def generate_ruff_report(paths: str, excludes: str, ignore: str, select: str, convention: str, - add_ignore: str, add_select: str): +def generate_ruff_report(paths: list[str], excludes: list[str], ignore: str, select: str, + convention: str, add_ignore: str, add_select: str): + + # If files don't exist fail fast + report = [] + existing_paths = [] + for path in paths: + if not os.path.exists(path): + report.append((path, [{ + 'category': 'unknown', + 'linenumber': '-', + 'message': 'file does not exist' + }])) + else: + existing_paths.append(path) + + if not existing_paths: + return report + cmd = ['ruff', 'check', '--output-format', 'json'] if ignore: @@ -43,7 +60,7 @@ def generate_ruff_report(paths: str, excludes: str, ignore: str, select: str, co cmd += ['--exclude', e] # paths - cmd += paths + cmd += existing_paths result = subprocess.run( cmd, @@ -53,11 +70,10 @@ def generate_ruff_report(paths: str, excludes: str, ignore: str, select: str, co data = json.loads(result.stdout) - report = [] files_dict = {} for item in data: - filename = os.path.abspath(item['filename']) + filename = os.path.relpath(item['filename']) if _filename_in_excludes(filename, excludes): continue @@ -87,4 +103,20 @@ def generate_ruff_report(paths: str, excludes: str, ignore: str, select: str, co report.append((filename, errors)) + # If no violations count as a pass + reported_files = {filename for filename, _ in report} + + for path in existing_paths: + if os.path.isdir(path): + for root, _, files in os.walk(path): + for f in files: + if f.endswith('.py'): + rel = os.path.relpath(os.path.join(root, f)) + if rel not in reported_files and not _filename_in_excludes(rel, excludes): + report.append((rel, [])) + else: + rel = os.path.relpath(path) + if rel not in reported_files and not _filename_in_excludes(rel, excludes): + report.append((rel, [])) + return report diff --git a/ament_pep257/test/test_ament_pep257.py b/ament_pep257/test/test_ament_pep257.py index fe082c910..bd4984290 100644 --- a/ament_pep257/test/test_ament_pep257.py +++ b/ament_pep257/test/test_ament_pep257.py @@ -32,3 +32,19 @@ def test_valid_file(): report = main([str(py_file)]) assert report == 0 + + +def test_valid_with_violations(): + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + py_file = temp_dir / 'foobar.py' + + py_file.write_text( + 'def foo():\n' + ' """bad docstring"""\n' + ' pass\n' + ) + + report = main([str(py_file)]) + + assert report == 1 diff --git a/ament_pep257/test/test_pep257.py b/ament_pep257/test/test_pep257.py new file mode 100644 index 000000000..6c951516f --- /dev/null +++ b/ament_pep257/test/test_pep257.py @@ -0,0 +1,23 @@ +# Copyright 2026 Open Source Robotics Foundation, Inc. +# +# 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. + +from ament_pep257.main import main +import pytest + + +@pytest.mark.linter +@pytest.mark.pep257 +def test_pep257(): + rc = main(argv=[]) + assert rc == 0, 'Found docblock style errors' From a5d2281e53a9082aee5a072882c5dd3ac5bb17e0 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 26 Mar 2026 16:44:52 -0700 Subject: [PATCH 5/6] unify test Signed-off-by: Michael Carlstrom --- ament_pep257/test/test_ament_pep257.py | 43 +++++++++++---- .../test/test_generate_pep257_report.py | 52 ------------------- .../test/test_generate_ruff_report.py | 52 ------------------- 3 files changed, 34 insertions(+), 113 deletions(-) delete mode 100644 ament_pep257/test/test_generate_pep257_report.py delete mode 100644 ament_pep257/test/test_generate_ruff_report.py diff --git a/ament_pep257/test/test_ament_pep257.py b/ament_pep257/test/test_ament_pep257.py index bd4984290..9508cef32 100644 --- a/ament_pep257/test/test_ament_pep257.py +++ b/ament_pep257/test/test_ament_pep257.py @@ -14,27 +14,53 @@ import pathlib import tempfile +import typing -from ament_pep257.main import main +from _pytest.fixtures import FixtureRequest +from _pytest.monkeypatch import MonkeyPatch +import pytest -def test_invalid_file(): - report = main(['non_existent_file.py']) +class MainFunc(typing.Protocol): + + def __call__(self, argv: list[str] = ...) -> typing.Literal[0, 1]: ... + + +@pytest.fixture(params=['ruff', 'pydocstyle']) +def backend(request: FixtureRequest, monkeypatch: MonkeyPatch) -> MainFunc: + import ament_pep257.main as m + + if request.param == 'ruff': + if not m.ruff_installed: + pytest.skip('ruff not installed') + monkeypatch.setattr(m, 'ruff_installed', True) + monkeypatch.setattr(m, 'pydocstyle_installed', False) + + elif request.param == 'pydocstyle': + if not m.pydocstyle_installed: + pytest.skip('pydocstyle not installed') + monkeypatch.setattr(m, 'ruff_installed', False) + monkeypatch.setattr(m, 'pydocstyle_installed', True) + + return m.main + + +def test_invalid_file(backend: MainFunc) -> None: + report = backend(['non_existent_file.py']) assert report == 1 -def test_valid_file(): +def test_valid_file(backend: MainFunc) -> None: with tempfile.TemporaryDirectory() as temp_dir: temp_dir = pathlib.Path(temp_dir) py_file = temp_dir / 'foobar.py' py_file.write_text('a = 1+2\n') - report = main([str(py_file)]) - + report = backend([str(py_file)]) assert report == 0 -def test_valid_with_violations(): +def test_valid_with_violations(backend: MainFunc) -> None: with tempfile.TemporaryDirectory() as temp_dir: temp_dir = pathlib.Path(temp_dir) py_file = temp_dir / 'foobar.py' @@ -45,6 +71,5 @@ def test_valid_with_violations(): ' pass\n' ) - report = main([str(py_file)]) - + report = backend([str(py_file)]) assert report == 1 diff --git a/ament_pep257/test/test_generate_pep257_report.py b/ament_pep257/test/test_generate_pep257_report.py deleted file mode 100644 index c3806df60..000000000 --- a/ament_pep257/test/test_generate_pep257_report.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright 2023 Open Source Robotics Foundation, Inc. -# -# 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 pathlib -import tempfile - -from ament_pep257.main import _ament_ignore -from ament_pep257.main import pydocstyle_installed -import pytest - - -@pytest.mark.skipif(not pydocstyle_installed, reason='pydocstyle not installed') -def test_invalid_file(): - from ament_pep257.pydocstyle_impl import generate_pep257_report - ignore = ','.join(_ament_ignore) - - report = generate_pep257_report(['non_existent_file.py'], [], ignore, [], 'ament', [], []) - assert len(report) == 1 - filename, errors = report[0] - assert filename == 'non_existent_file.py' - assert len(errors) == 1 - error = errors[0] - assert error['linenumber'] == '-' - assert error['category'] == 'unknown' - - -@pytest.mark.skipif(not pydocstyle_installed, reason='pydocstyle not installed') -def test_valid_file(): - from ament_pep257.pydocstyle_impl import generate_pep257_report - ignore = ','.join(_ament_ignore) - - with tempfile.TemporaryDirectory() as temp_dir: - temp_dir = pathlib.Path(temp_dir) - py_file = temp_dir / 'foobar.py' - py_file.write_text('a = 1+2\n') - - report = generate_pep257_report([str(temp_dir)], [], ignore, [], 'ament', [], []) - - assert len(report) == 1 - filename, errors = report[0] - assert errors == [] diff --git a/ament_pep257/test/test_generate_ruff_report.py b/ament_pep257/test/test_generate_ruff_report.py deleted file mode 100644 index 00f8d81c3..000000000 --- a/ament_pep257/test/test_generate_ruff_report.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright 2023 Open Source Robotics Foundation, Inc. -# -# 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 pathlib -import tempfile - -from ament_pep257.main import _ament_ignore -from ament_pep257.main import ruff_installed -import pytest - - -@pytest.mark.skipif(not ruff_installed, reason='ruff not installed') -def test_invalid_file(): - from ament_pep257.ruff_impl import generate_ruff_report - ignore = ','.join(_ament_ignore) - - report = generate_ruff_report(['non_existent_file.py'], [], ignore, [], 'ament', [], []) - assert len(report) == 1 - filename, errors = report[0] - assert filename == 'non_existent_file.py' - assert len(errors) == 1 - error = errors[0] - assert error['linenumber'] == '-' - assert error['category'] == 'unknown' - - -@pytest.mark.skipif(not ruff_installed, reason='ruff not installed') -def test_valid_file(): - from ament_pep257.ruff_impl import generate_ruff_report - ignore = ','.join(_ament_ignore) - - with tempfile.TemporaryDirectory() as temp_dir: - temp_dir = pathlib.Path(temp_dir) - py_file = temp_dir / 'foobar.py' - py_file.write_text('a = 1+2\n') - - report = generate_ruff_report([str(temp_dir)], [], ignore, [], 'ament', [], []) - - assert len(report) == 1 - filename, errors = report[0] - assert errors == [] From 188a039d50b95bdd419a6c1c8dacf09a00e179c2 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 26 Mar 2026 19:14:17 -0700 Subject: [PATCH 6/6] fail on both missing Signed-off-by: Michael Carlstrom --- ament_pep257/ament_pep257/ruff_impl.py | 60 +++++++++++--------------- ament_pep257/test/test_ament_pep257.py | 44 ++++++++++++++++++- 2 files changed, 68 insertions(+), 36 deletions(-) diff --git a/ament_pep257/ament_pep257/ruff_impl.py b/ament_pep257/ament_pep257/ruff_impl.py index 4176f0ae5..381e25c73 100644 --- a/ament_pep257/ament_pep257/ruff_impl.py +++ b/ament_pep257/ament_pep257/ruff_impl.py @@ -17,6 +17,7 @@ import os import subprocess import sys +import re from ament_pep257.main import _filename_in_excludes @@ -24,23 +25,7 @@ def generate_ruff_report(paths: list[str], excludes: list[str], ignore: str, select: str, convention: str, add_ignore: str, add_select: str): - # If files don't exist fail fast - report = [] - existing_paths = [] - for path in paths: - if not os.path.exists(path): - report.append((path, [{ - 'category': 'unknown', - 'linenumber': '-', - 'message': 'file does not exist' - }])) - else: - existing_paths.append(path) - - if not existing_paths: - return report - - cmd = ['ruff', 'check', '--output-format', 'json'] + cmd = ['ruff', 'check', '--output-format', 'json', '--select', 'D'] if ignore: cmd += ['--ignore', ignore] @@ -60,7 +45,7 @@ def generate_ruff_report(paths: list[str], excludes: list[str], ignore: str, sel cmd += ['--exclude', e] # paths - cmd += existing_paths + cmd += paths result = subprocess.run( cmd, @@ -70,6 +55,29 @@ def generate_ruff_report(paths: list[str], excludes: list[str], ignore: str, sel data = json.loads(result.stdout) + report = [] + + # Handles missing files + if result.stderr: + warnings = result.stderr.splitlines() + for line in warnings: + match = re.search(r"Failed to lint (.*?):", line) + if match: + filename = match.group(1) + report.append((filename, [ + {'category': 'unknown', + 'linenumber': 'N/A', + 'message': line + }])) + print( + '%s:%s %s: %s' % ( + filename, + 'N/A', + 'N/A', + line, + ), + file=sys.stderr, + ) files_dict = {} for item in data: @@ -103,20 +111,4 @@ def generate_ruff_report(paths: list[str], excludes: list[str], ignore: str, sel report.append((filename, errors)) - # If no violations count as a pass - reported_files = {filename for filename, _ in report} - - for path in existing_paths: - if os.path.isdir(path): - for root, _, files in os.walk(path): - for f in files: - if f.endswith('.py'): - rel = os.path.relpath(os.path.join(root, f)) - if rel not in reported_files and not _filename_in_excludes(rel, excludes): - report.append((rel, [])) - else: - rel = os.path.relpath(path) - if rel not in reported_files and not _filename_in_excludes(rel, excludes): - report.append((rel, [])) - return report diff --git a/ament_pep257/test/test_ament_pep257.py b/ament_pep257/test/test_ament_pep257.py index 9508cef32..967b916f0 100644 --- a/ament_pep257/test/test_ament_pep257.py +++ b/ament_pep257/test/test_ament_pep257.py @@ -18,9 +18,14 @@ from _pytest.fixtures import FixtureRequest from _pytest.monkeypatch import MonkeyPatch +import ament_pep257.main as m import pytest +if not m.ruff_installed or not m.pydocstyle_installed: + pytest.fail('Neither ruff or pydocstyle installed') + + class MainFunc(typing.Protocol): def __call__(self, argv: list[str] = ...) -> typing.Literal[0, 1]: ... @@ -28,8 +33,6 @@ def __call__(self, argv: list[str] = ...) -> typing.Literal[0, 1]: ... @pytest.fixture(params=['ruff', 'pydocstyle']) def backend(request: FixtureRequest, monkeypatch: MonkeyPatch) -> MainFunc: - import ament_pep257.main as m - if request.param == 'ruff': if not m.ruff_installed: pytest.skip('ruff not installed') @@ -60,6 +63,17 @@ def test_valid_file(backend: MainFunc) -> None: assert report == 0 +def test_valid_and_invalid_file(backend: MainFunc) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + py_file = temp_dir / 'foobar.py' + py_file2 = temp_dir / 'barfoo.py' + py_file.write_text('a = 1+2\n') + + report = backend([str(py_file), str(py_file2)]) + assert report == 1 + + def test_valid_with_violations(backend: MainFunc) -> None: with tempfile.TemporaryDirectory() as temp_dir: temp_dir = pathlib.Path(temp_dir) @@ -73,3 +87,29 @@ def test_valid_with_violations(backend: MainFunc) -> None: report = backend([str(py_file)]) assert report == 1 + + +def test_ignore_codes(backend: MainFunc) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + py_file = temp_dir / 'foobar.py' + + # This normally triggers D100 (missing docstring in module) + py_file.write_text('') + + report = backend([str(py_file), '--ignore', 'D100']) + assert report == 0 + + +def test_directory_with_multiple_files(backend: MainFunc) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = pathlib.Path(temp_dir) + files = { + 'good.py': 'a = 1\n', + 'bad.py': 'def f():\n """foo"""\n pass\n', + } + for name, content in files.items(): + (temp_dir / name).write_text(content) + + report = backend([str(temp_dir)]) + assert report == 1