refactor: Enable ruff and ament_mypy checks and fix lint errors (#1063)

* Enable flake8-bugbear and fix lint errors

* Enable flake8-builtins and fix lint errors

* Enable flake8-datetimez and fix lint errors

* Enable flake8-executable and fix lint errors

* Fix lint error in rosapi_node

* Enable flake8-future-annotations and fix lint errors

* Enable flake8-implicit-str-concat and fix lint errors

* Enable flake8-pie and fix lint errors

* Enable flake8-raise and fix lint errors

* Enable flake8-return and fix lint errors

* Enable flake8-simplify and fix lint errors

* Enable flake8-type-checking and fix lint errors

* Enable flake8-unused-arguments and fix lint errors

* Enable flake8-use-pathlib and fix lint errors

* Enable Perflint and fix lint errors

* Enable pyupgrade and fix lint errors

* Enable refurb and fix lint errors

* Enable ruff-specific checks and fix errors

* Fix subscribe test

* Fix mypy errors

* rosapi: type check test methods

* Fix rosapi_node lint errors

* Set max line length to 120 and fix some of the errors

* Fix the rest of 'line too long' errors

* Don't change argument names in public API methods

* Enable flake8-errmsg and fix lint errors

* Enable flake8-logging and fix lint error

* Enable flake8-pyi and fix lint errors

* Fix pylint errors

* Add comments to ruff config

* Fix trailing whitespace

* Enable flake8-logging-format

* Simplify message handler transitions

* Fix other ruff lint errors

* Add ament_mypy test and fix mypy errors

* Fix ruff check in rosapi_node

Author: bjsowa

pyproject.toml

@@ -7,71 +7,73 @@ line-length = 100
 
 [tool.ruff.lint]
 select = [
-    # "AIR", # Airflow
-    # "ERA", # Eradicate
-    # "FAST", # FastAPI
+    # "AIR",  # Airflow, We don't use this framework
+    # "ERA",  # Eradicate, We allow commented-out code
+    # "FAST", # FastAPI, We don't use this framework
 
     # flake8 extensions
-    "YTT", # flake8-2020
-    # "ANN", # flake8-annotations
-    "ASYNC", # flake8-async
-    "S", # flake8-bandit
-    # "BLE", # flake8-blind-except
-    # "FBT", # flake8-boolean-trap
-    # "B", # flake8-bugbear
-    # "A", # flake8-builtins
-    # "COM", # flake8-commas
-    "C4", # flake8-comprehensions
-    # "CPY", # flake8-copyright
-    # "DTZ", # flake8-datetimez
-    "T10", # flake8-debugger
-    # "DJ", # flake8-django
-    # "EM", # flake8-errmsg
-    # "EXE", # flake8-executable
-    # "FIX", # flake8-fixme
-    # "FA", # flake8-future-annotations
-    "INT", # flake8-gettext
-    # "ISC", # flake8-implicit-str-concat
-    "ICN", # flake8-import-conventions
-    # "LOG", # flake8-logging
-    # "G", # flake8-logging-format
-    # "INP", # flake8-no-pep420
-    # "PIE", # flake8-pie
-    # "T20", # flake8-print
-    # "PYI", # flake8-pyi
-    # "PT", # flake8-pytest-style
-    "Q", # flake8-quotes
-    # "RSE", # flake8-raise
-    # "RET", # flake8-return
-    # "SLF", # flake8-self
-    # "SIM", # flake8-simplify
-    "SLOT", # flake8-slots
-    "TID", # flake8-tidy-imports
-    # "TD", # flake8-todos
-    # "TC", # flake8-type-checking
-    # "ARG", # flake8-unused-arguments
-    # "PTH", # flake8-use-pathlib
+    "YTT",    # flake8-2020
+    # "ANN",  # flake8-annotations, There are too many missing annotations right now (TODO)
+    "ASYNC",  # flake8-async
+    "S",      # flake8-bandit
+    # "BLE",  # flake8-blind-except, Too much hassle to fix (TODO)
+    # "FBT",  # flake8-boolean-trap, We allow boolean traps
+    "B",      # flake8-bugbear
+    "A",      # flake8-builtins
+    # "COM",  # flake8-commas, We don't require trailing commas
+    "C4",     # flake8-comprehensions
+    # "CPY",  # flake8-copyright, preview feature
+    "DTZ",    # flake8-datetimez
+    "T10",    # flake8-debugger
+    # "DJ",   # flake8-django, We don't use this framework
+    "EM",     # flake8-errmsg
+    "EXE",    # flake8-executable
+    # "FIX",  # flake8-fixme, too many TODOs to fix them now
+    "FA",     # flake8-future-annotations
+    "INT",    # flake8-gettext
+    "ISC",    # flake8-implicit-str-concat
+    "ICN",    # flake8-import-conventions
+    "LOG",    # flake8-logging
+    "G",      # flake8-logging-format
+    # "INP",  # flake8-no-pep420, false positives with launch tests
+    "PIE",    # flake8-pie
+    # "T20",  # flake8-print, We allow some print statements
+    "PYI",    # flake8-pyi
+    # "PT",   # flake8-pytest-style, Too many errors to fix for now (TODO)
+    "Q",      # flake8-quotes
+    "RSE",    # flake8-raise
+    "RET",    # flake8-return
+    # "SLF",  # flake8-self, Too many "Private member accessed" errors (TODO)
+    "SIM",    # flake8-simplify
+    "SLOT",   # flake8-slots
+    "TID",    # flake8-tidy-imports
+    # "TD",   # flake8-todos, We don't enforce format for TODO comments
+    "TC",     # flake8-type-checking
+    "ARG",    # flake8-unused-arguments
+    "PTH",    # flake8-use-pathlib
 
-    "FLY", # flynt
-    "I", # isort
-    "C90", # mccabe
-    "NPY", # numpy
-    "PD", # pandas-vet
-    # "N", # pep8-naming
-    # "PERF", # Perflint
-    "E", # pycodestyle errors
-    "W", # pycodestyle warnings
-    # "DOC", # pydoclint
-    "D", # pydocstyle
-    "F", # pyflakes
-    "PGH", # pygrep-hooks
-    # "PL", # pylint
-    # "UP", # pyupgrade
-    # "FURB", # refurb
-    # "RUF", # ruff
-    # "TRY", # tryceratops
+    "FLY",    # flynt
+    "I",      # isort
+    "C90",    # mccabe
+    "NPY",    # numpy
+    "PD",     # pandas-vet
+    # "N",    # pep8-naming, Can't fix without breaking API
+    "PERF",   # Perflint
+    "E",      # pycodestyle errors
+    "W",      # pycodestyle warnings
+    # "DOC",  # pydoclint, preview feature
+    "D",      # pydocstyle
+    "F",      # pyflakes
+    "PGH",    # pygrep-hooks
+    "PL",     # pylint
+    "UP",     # pyupgrade
+    "FURB",   # refurb
+    "RUF",    # ruff
+    # "TRY",  # tryceratops, Too many "Create your own exception" errors (TODO)
 ]
 ignore = [
+    "SIM108", # Opinionated: ternary operator does not always look simpler
+
     # Undocumented code
     "D100",
     "D101",
@@ -89,8 +91,20 @@ ignore = [
     "S101", # assert
     "S110", # try-except-pass
     "S311", # suspicious-non-cryptographic-random-usage
+
+    # Pylint "too-many" errors
+    "PLR0911",
+    "PLR0912",
+    "PLR0913",
+    "PLR0914",
+    "PLR0915",
+    "PLR0916",
+    "PLR0917",
+
+    "PLR2004", # magic-value-comparison, Too many to fix for now (TODO)
+    "PLW0603", # global-statement, Will need a more sophisticated solution to fix this (TODO)
 ]
 
 mccabe.max-complexity = 36
 
-pycodestyle.max-line-length = 217
+pycodestyle.max-line-length = 120

rosapi/CMakeLists.txt

@@ -26,4 +26,7 @@ if(BUILD_TESTING)
   find_package(ament_cmake_pytest REQUIRED)
   ament_add_pytest_test(${PROJECT_NAME}_test_stringify_field_types test/test_stringify_field_types.py)
   ament_add_pytest_test(${PROJECT_NAME}_test_typedefs test/test_typedefs.py)
+
+  find_package(ament_cmake_mypy REQUIRED)
+  ament_mypy()
 endif()

rosapi/package.xml

@@ -33,6 +33,7 @@ action servers and managing ROS parameters.
   <exec_depend>rosidl_adapter</exec_depend>
   <exec_depend>rosidl_runtime_py</exec_depend>
 
+  <test_depend>ament_cmake_mypy</test_depend>
   <test_depend>ament_cmake_pytest</test_depend>
   <test_depend>geometry_msgs</test_depend>
   <test_depend>rmw_dds_common</test_depend>

rosapi/scripts/rosapi_node

@@ -159,7 +159,7 @@ class Rosapi(Node):
     def get_globs(self):
         return glob_helper.get_globs(self)
 
-    def get_topics(self, request, response):
+    def get_topics(self, _request, response):
         """
         Return a list of all the topics being published.
 
@@ -168,7 +168,7 @@ class Rosapi(Node):
         response.topics, response.types = proxy.get_topics_and_types(self.globs.topics)
         return response
 
-    def get_interfaces(self, request, response):
+    def get_interfaces(self, _request, response):
         """
         Return a list of all the interfaces in the system.
 
@@ -186,7 +186,7 @@ class Rosapi(Node):
         response.topics = proxy.get_topics_for_type(request.type, self.globs.topics)
         return response
 
-    def get_topics_and_raw_types(self, request, response):
+    def get_topics_and_raw_types(self, _request, response):
         """
         Return a list of all the topics being published, and their raw types.
 
@@ -196,11 +196,11 @@ class Rosapi(Node):
         """
         response.topics, response.types = proxy.get_topics_and_types(self.globs.topics)
         response.typedefs_full_text = [
-            objectutils.get_typedef_full_text(type) for type in response.types
+            objectutils.get_typedef_full_text(type_name) for type_name in response.types
         ]
         return response
 
-    def get_services(self, request, response):
+    def get_services(self, _request, response):
         """
         Return a list of all the services being advertised.
 
@@ -218,7 +218,7 @@ class Rosapi(Node):
         response.services = proxy.get_services_for_type(request.type, self.globs.services)
         return response
 
-    def get_nodes(self, request, response):
+    def get_nodes(self, _request, response):
         """
         Return a list of all the nodes that are registered.
 
@@ -240,7 +240,7 @@ class Rosapi(Node):
         ) = proxy.get_node_info(request.node)
         return response
 
-    def get_action_servers(self, request, response):
+    def get_action_servers(self, _request, response):
         """
         Return a list of action servers based on actions standard topics.
 
@@ -377,11 +377,11 @@ class Rosapi(Node):
         params.delete_param(request.node_name, request.name, self.globs.params)
         return response
 
-    def get_param_names(self, request, response):
+    def get_param_names(self, _request, response):
         response.names = params.get_param_names(self.globs.params)
         return response
 
-    def get_time(self, request, response):
+    def get_time(self, _request, response):
         response.time = Clock(clock_type=ClockType.ROS_TIME).now().to_msg()
         return response
 
@@ -390,10 +390,10 @@ class Rosapi(Node):
 
     def _print_malformed_param_name_warning(self, param_name):
         self.get_logger().warn(
-            "Malformed parameter name: {}; expecting <node_name>:<param_name>".format(param_name)
+            f"Malformed parameter name: {param_name}; expecting <node_name>:<param_name>"
         )
 
-    def get_ros_version(self, request, response):
+    def get_ros_version(self, _request, response):
         response.version = 2
         response.distro = str(os.environ["ROS_DISTRO"])
         return response

rosapi/src/rosapi/glob_helper.py

@@ -1,11 +1,13 @@
-#!/usr/bin/env python
-
 import fnmatch
-from collections import namedtuple
+from typing import NamedTuple
 
 from rcl_interfaces.msg import ParameterType
 
-Globs = namedtuple("Globs", ["topics", "services", "params"])
+
+class Globs(NamedTuple):
+    topics: list
+    services: list
+    params: list
 
 
 def get_globs(node):
@@ -32,8 +34,7 @@ def filter_globs(globs, full_list):
     # If the globs are empty (weren't defined in the params), return the full list
     if globs is not None and len(globs) > 0:
         return list(filter(lambda x: any_match(x, globs), full_list))
-    else:
-        return full_list
+    return full_list
 
 
 def any_match(query, globs):