From a22597a2d675c16312b1df275966c71b8a679964 Mon Sep 17 00:00:00 2001 From: Tomas Sebestik Date: Mon, 1 Feb 2021 09:18:16 +0100 Subject: [PATCH] Add mypy check to pre-commit-config --- .mypy.ini | 26 +++ .pre-commit-config.yaml | 6 + components/espcoredump/espcoredump.py | 14 +- tools/build_apps.py | 3 +- tools/ci/check_type_comments.py | 94 +++++++++ tools/ci/ci_get_mr_info.py | 12 +- tools/ci/executable-list.txt | 1 + tools/ci/mypy_ignore_list.txt | 216 ++++++++++++++++++++ tools/mkdfu.py | 12 +- tools/unit-test-app/tools/UnitTestParser.py | 2 +- 10 files changed, 371 insertions(+), 15 deletions(-) create mode 100644 .mypy.ini create mode 100755 tools/ci/check_type_comments.py create mode 100644 tools/ci/mypy_ignore_list.txt diff --git a/.mypy.ini b/.mypy.ini new file mode 100644 index 0000000000..e4f451ca26 --- /dev/null +++ b/.mypy.ini @@ -0,0 +1,26 @@ +[mypy] + +# Specifies the Python version used to parse and check the target program +python_version = 3.9 + +# Disallows defining functions without type annotations or with incomplete type annotations +# True => enforce type annotation in all function definitions +disallow_untyped_defs = True + +# Shows a warning when returning a value with type Any from a function declared with a non- Any return type +warn_return_any = True + +# Shows errors for missing return statements on some execution paths +warn_no_return = True + +# Suppress error messages about imports that cannot be resolved +# True => ignore all import errors +ignore_missing_imports = True + +# Disallows defining functions with incomplete type annotations +disallow_incomplete_defs = False + +# Directs what to do with imports when the imported module is found as a .py file and not part of the files, +# modules and packages provided on the command line. +# SKIP -> mypy checks only single file, not included imports +follow_imports = skip diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 943f979d8c..4b9f493d1f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -91,3 +91,9 @@ repos: pass_filenames: false additional_dependencies: - PyYAML == 5.3.1 + - id: mypy-check + name: Check type annotations in python files + entry: tools/ci/check_type_comments.py + additional_dependencies: ['mypy==0.800', 'mypy-extensions==0.4.3'] + language: python + types: [python] diff --git a/components/espcoredump/espcoredump.py b/components/espcoredump/espcoredump.py index 0019265040..5b09e7761b 100755 --- a/components/espcoredump/espcoredump.py +++ b/components/espcoredump/espcoredump.py @@ -16,6 +16,12 @@ from corefile.gdb import EspGDB from corefile.loader import ESPCoreDumpFileLoader, ESPCoreDumpFlashLoader from pygdbmi.gdbcontroller import DEFAULT_GDB_TIMEOUT_SEC +try: + from typing import Optional, Tuple +except ImportError: + # Only used for type annotations + pass + IDF_PATH = os.getenv('IDF_PATH') if not IDF_PATH: sys.stderr.write('IDF_PATH is not found! Set proper IDF_PATH in environment.\n') @@ -34,7 +40,7 @@ else: CLOSE_FDS = True -def load_aux_elf(elf_path): # type: (str) -> (ElfFile, str) +def load_aux_elf(elf_path): # type: (str) -> Tuple[ElfFile, str] """ Loads auxiliary ELF file and composes GDB command to read its symbols. """ @@ -48,7 +54,7 @@ def load_aux_elf(elf_path): # type: (str) -> (ElfFile, str) return elf, sym_cmd -def core_prepare(): +def core_prepare(): # type: () -> Tuple[Optional[str], ESPCoreDumpFlashLoader] loader = None core_filename = None if not args.core: @@ -72,7 +78,7 @@ def core_prepare(): return core_filename, loader -def dbg_corefile(): +def dbg_corefile(): # type: () -> None """ Command to load core dump from file or flash and run GDB debug session with it """ @@ -91,7 +97,7 @@ def dbg_corefile(): print('Done!') -def info_corefile(): +def info_corefile(): # type: () -> None """ Command to load core dump from file or flash and print it's data in user friendly form """ diff --git a/tools/build_apps.py b/tools/build_apps.py index 561d1390a5..e613bb5b78 100755 --- a/tools/build_apps.py +++ b/tools/build_apps.py @@ -1,4 +1,5 @@ #!/usr/bin/env python + # coding=utf-8 # # ESP-IDF helper script to build multiple applications. Consumes the input of find_apps.py. @@ -12,7 +13,7 @@ from find_build_apps import BUILD_SYSTEMS, BuildError, BuildItem, setup_logging from find_build_apps.common import SIZE_JSON_FN, rmdir -def main(): +def main(): # type: () -> None parser = argparse.ArgumentParser(description='ESP-IDF app builder') parser.add_argument( '-v', diff --git a/tools/ci/check_type_comments.py b/tools/ci/check_type_comments.py new file mode 100755 index 0000000000..3e415ad95d --- /dev/null +++ b/tools/ci/check_type_comments.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python +# +# Copyright 2021 Espressif Systems (Shanghai) CO LTD +# +# 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 re +import subprocess +from sys import exit + +try: + from typing import List +except ImportError: + # Only used for type annotations + pass + +IGNORE_LIST_MYPY = 'tools/ci/mypy_ignore_list.txt' +COMMENT_RE = re.compile(r'#\s+type:\s+\(', re.MULTILINE) # REGEX: "# type: (" + + +def type_comments_in_file(file_name): # type: (str) -> bool + """ Check if type annotations (type comments) are present in the file """ + with open(file_name, 'r') as f: + return bool(COMMENT_RE.search(f.read())) + + +def types_valid(file_name): # type: (str) -> bool + """ Run Mypy check on the given file, return TRUE if Mypy check passes """ + mypy_exit_code = subprocess.call(('mypy %s' % file_name), shell=True) + return not bool(mypy_exit_code) + + +def check_files(files): # type: (List[str]) -> List[str] + """ + Check files for type annotatins: + - new python file -> run Mypy check + - existed file updated by type annotations -> run Mypy check, remove from ignore list + - existed file updated, but no type annotations added -> skip Mypy check on file + """ + type_issues = [] + + with open(IGNORE_LIST_MYPY, 'r') as f: + ignore_list = [item.strip() for item in f.readlines()] + updated_list = ignore_list.copy() + + for file_name in files: + if file_name in ignore_list: + if type_comments_in_file(file_name): + if types_valid(file_name): + updated_list.remove(file_name) + print('\33[93m\n File %s removed automatically from ignore list - run commit again! \n\33[0m' % file_name) + else: + type_issues.append(file_name) + else: + if not types_valid(file_name): + type_issues.append(file_name) + + ignore_list.sort() + updated_list.sort() + + if updated_list != ignore_list: + with open(IGNORE_LIST_MYPY, 'w') as f: + for item in updated_list: + f.write('%s\n' % item) + return type_issues + + +def main(): # type: () -> None + parser = argparse.ArgumentParser() + parser.add_argument('filenames', nargs='*', help='Filenames to check.') + args = parser.parse_args() + + type_issues = check_files(args.filenames) + + if type_issues: + print('mypy check failed for:') + for file_name in type_issues: + print('\t', file_name) + exit(1) + + +if __name__ == '__main__': + main() diff --git a/tools/ci/ci_get_mr_info.py b/tools/ci/ci_get_mr_info.py index b1550e9dbf..5deb8ca2de 100644 --- a/tools/ci/ci_get_mr_info.py +++ b/tools/ci/ci_get_mr_info.py @@ -24,8 +24,14 @@ import subprocess from gitlab_api import Gitlab +try: + from typing import Any, Union +except ImportError: + # Only used for type annotations + pass -def _get_mr_obj(source_branch): + +def _get_mr_obj(source_branch): # type: (str) -> Union[Gitlab, None] if not source_branch: return None gl = Gitlab(os.getenv('CI_PROJECT_ID', 'espressif/esp-idf')) @@ -46,7 +52,7 @@ def get_mr_iid(source_branch): # type: (str) -> str return str(mr.iid) -def get_mr_changed_files(source_branch): +def get_mr_changed_files(source_branch): # type: (str) -> Any mr = _get_mr_obj(source_branch) if not mr: return '' @@ -55,7 +61,7 @@ def get_mr_changed_files(source_branch): 'origin/{}...origin/{}'.format(mr.target_branch, source_branch)]).decode('utf8') -def get_mr_commits(source_branch): +def get_mr_commits(source_branch): # type: (str) -> str mr = _get_mr_obj(source_branch) if not mr: return '' diff --git a/tools/ci/executable-list.txt b/tools/ci/executable-list.txt index 49dbd081f5..ba39889e81 100644 --- a/tools/ci/executable-list.txt +++ b/tools/ci/executable-list.txt @@ -48,6 +48,7 @@ tools/ci/check_readme_links.py tools/ci/check_rom_apis.sh tools/ci/check_rules_yml.py tools/ci/check_tools_files_patterns.py +tools/ci/check_type_comments.py tools/ci/check_ut_cmake_make.sh tools/ci/checkout_project_ref.py tools/ci/deploy_docs.py diff --git a/tools/ci/mypy_ignore_list.txt b/tools/ci/mypy_ignore_list.txt new file mode 100644 index 0000000000..0b23762de9 --- /dev/null +++ b/tools/ci/mypy_ignore_list.txt @@ -0,0 +1,216 @@ +components/app_update/otatool.py +components/efuse/efuse_table_gen.py +components/efuse/test_efuse_host/efuse_tests.py +components/esp32s2/test/gen_digital_signature_tests.py +components/esp_local_ctrl/python/esp_local_ctrl_pb2.py +components/espcoredump/corefile/elf.py +components/espcoredump/corefile/gdb.py +components/espcoredump/corefile/loader.py +components/espcoredump/corefile/xtensa.py +components/espcoredump/test/test_espcoredump.py +components/lwip/weekend_test/net_suite_test.py +components/mbedtls/esp_crt_bundle/gen_crt_bundle.py +components/mbedtls/esp_crt_bundle/test_gen_crt_bundle/test_gen_crt_bundle.py +components/mbedtls/test/test_aes.c +components/mbedtls/test/test_aes_gcm.c +components/mqtt/weekend_test/mqtt_publish_test.py +components/nvs_flash/nvs_partition_generator/nvs_partition_gen.py +components/partition_table/gen_empty_partition.py +components/partition_table/gen_esp32part.py +components/partition_table/parttool.py +components/partition_table/test_gen_esp32part_host/gen_esp32part_tests.py +components/protocomm/python/constants_pb2.py +components/protocomm/python/sec0_pb2.py +components/protocomm/python/sec1_pb2.py +components/protocomm/python/session_pb2.py +components/spiffs/spiffsgen.py +components/ulp/esp32ulp_mapgen.py +components/wifi_provisioning/python/wifi_config_pb2.py +components/wifi_provisioning/python/wifi_constants_pb2.py +components/wifi_provisioning/python/wifi_scan_pb2.py +components/xtensa/trax/traceparse.py +docs/build_docs.py +docs/conf_common.py +docs/en/conf.py +docs/extensions/html_redirects.py +docs/extensions/list_filter.py +docs/extensions/toctree_filter.py +docs/generate_chart.py +docs/get_github_rev.py +docs/idf_extensions/esp_err_definitions.py +docs/idf_extensions/exclude_docs.py +docs/idf_extensions/format_idf_target.py +docs/idf_extensions/gen_defines.py +docs/idf_extensions/gen_idf_tools_links.py +docs/idf_extensions/gen_toolchain_links.py +docs/idf_extensions/gen_version_specific_includes.py +docs/idf_extensions/include_build_file.py +docs/idf_extensions/kconfig_reference.py +docs/idf_extensions/latex_builder.py +docs/idf_extensions/link_roles.py +docs/idf_extensions/run_doxygen.py +docs/idf_extensions/util.py +docs/sanitize_version.py +docs/test/en/conf.py +docs/test/test_docs.py +docs/test/test_sphinx_idf_extensions.py +docs/zh_CN/conf.py +examples/bluetooth/nimble/blecent/blecent_test.py +examples/bluetooth/nimble/blehr/blehr_test.py +examples/bluetooth/nimble/bleprph/bleprph_test.py +examples/get-started/blink/example_test.py +examples/get-started/hello_world/example_test.py +examples/peripherals/gpio/generic_gpio/example_test.py +examples/peripherals/sdio/sdio_test.py +examples/protocols/esp_local_ctrl/scripts/esp_local_ctrl.py +examples/protocols/esp_local_ctrl/scripts/proto.py +examples/protocols/http_server/advanced_tests/http_server_advanced_test.py +examples/protocols/http_server/advanced_tests/scripts/test.py +examples/protocols/http_server/persistent_sockets/http_server_persistence_test.py +examples/protocols/http_server/simple/http_server_simple_test.py +examples/protocols/http_server/ws_echo_server/ws_server_example_test.py +examples/protocols/mdns/mdns_example_test.py +examples/protocols/modbus/serial/example_test.py +examples/protocols/modbus/tcp/example_test.py +examples/protocols/mqtt/ssl/mqtt_ssl_example_test.py +examples/protocols/mqtt/ssl_ds/configure_ds.py +examples/protocols/pppos_client/example_test.py +examples/protocols/sockets/tcp_client/example_test.py +examples/protocols/sockets/udp_client/example_test.py +examples/protocols/websocket/example_test.py +examples/provisioning/legacy/ble_prov/ble_prov_test.py +examples/provisioning/legacy/custom_config/components/custom_provisioning/python/custom_config_pb2.py +examples/provisioning/legacy/softap_prov/softap_prov_test.py +examples/provisioning/wifi_prov_mgr/wifi_prov_mgr_test.py +examples/storage/parttool/parttool_example.py +examples/system/ota/advanced_https_ota/example_test.py +examples/system/ota/native_ota_example/example_test.py +examples/system/ota/otatool/get_running_partition.py +examples/system/ota/otatool/otatool_example.py +examples/system/ota/simple_ota_example/example_test.py +tools/ble/lib_ble_client.py +tools/ble/lib_gap.py +tools/ble/lib_gatt.py +tools/check_python_dependencies.py +tools/check_term.py +tools/ci/apply_bot_filter.py +tools/ci/check_artifacts_expire_time.py +tools/ci/check_build_warnings.py +tools/ci/check_callgraph.py +tools/ci/check_codeowners.py +tools/ci/check_deprecated_kconfigs.py +tools/ci/check_executables.py +tools/ci/check_kconfigs.py +tools/ci/check_public_headers.py +tools/ci/check_readme_links.py +tools/ci/checkout_project_ref.py +tools/ci/ci_fetch_submodule.py +tools/ci/deploy_docs.py +tools/ci/envsubst.py +tools/ci/idf_ci_utils.py +tools/ci/normalize_clangtidy_path.py +tools/ci/python_packages/gitlab_api.py +tools/ci/python_packages/idf_http_server_test/adder.py +tools/ci/python_packages/idf_http_server_test/client.py +tools/ci/python_packages/idf_http_server_test/test.py +tools/ci/python_packages/idf_iperf_test_util/LineChart.py +tools/ci/python_packages/idf_iperf_test_util/PowerControl.py +tools/ci/python_packages/tiny_test_fw/App.py +tools/ci/python_packages/tiny_test_fw/DUT.py +tools/ci/python_packages/tiny_test_fw/Env.py +tools/ci/python_packages/tiny_test_fw/EnvConfig.py +tools/ci/python_packages/tiny_test_fw/TinyFW.py +tools/ci/python_packages/tiny_test_fw/Utility/CIAssignTest.py +tools/ci/python_packages/tiny_test_fw/Utility/CaseConfig.py +tools/ci/python_packages/tiny_test_fw/Utility/GitlabCIJob.py +tools/ci/python_packages/tiny_test_fw/Utility/SearchCases.py +tools/ci/python_packages/tiny_test_fw/Utility/TestCase.py +tools/ci/python_packages/tiny_test_fw/bin/Runner.py +tools/ci/python_packages/ttfw_idf/CIScanTests.py +tools/ci/python_packages/ttfw_idf/DebugUtils.py +tools/ci/python_packages/ttfw_idf/IDFApp.py +tools/ci/python_packages/ttfw_idf/IDFDUT.py +tools/ci/python_packages/wifi_tools.py +tools/ci/test_autocomplete.py +tools/ci/test_check_kconfigs.py +tools/cmake/convert_to_cmake.py +tools/esp_app_trace/espytrace/apptrace.py +tools/esp_app_trace/espytrace/sysview.py +tools/esp_app_trace/logtrace_proc.py +tools/esp_app_trace/sysviewtrace_proc.py +tools/esp_prov/esp_prov.py +tools/esp_prov/prov/custom_prov.py +tools/esp_prov/prov/wifi_prov.py +tools/esp_prov/prov/wifi_scan.py +tools/esp_prov/security/security.py +tools/esp_prov/security/security0.py +tools/esp_prov/security/security1.py +tools/esp_prov/transport/ble_cli.py +tools/esp_prov/transport/transport.py +tools/esp_prov/transport/transport_ble.py +tools/esp_prov/transport/transport_console.py +tools/esp_prov/transport/transport_http.py +tools/esp_prov/utils/convenience.py +tools/find_apps.py +tools/find_build_apps/cmake.py +tools/find_build_apps/common.py +tools/find_build_apps/make.py +tools/gdb_panic_server.py +tools/gen_esp_err_to_name.py +tools/idf.py +tools/idf_monitor.py +tools/idf_py_actions/core_ext.py +tools/idf_py_actions/create_ext.py +tools/idf_py_actions/debug_ext.py +tools/idf_py_actions/dfu_ext.py +tools/idf_py_actions/errors.py +tools/idf_py_actions/serial_ext.py +tools/idf_py_actions/tools.py +tools/idf_py_actions/uf2_ext.py +tools/idf_size.py +tools/idf_tools.py +tools/kconfig_new/confgen.py +tools/kconfig_new/confserver.py +tools/kconfig_new/esp-windows-curses/setup.py +tools/kconfig_new/gen_kconfig_doc.py +tools/kconfig_new/prepare_kconfig_files.py +tools/kconfig_new/test/confgen/test_confgen.py +tools/kconfig_new/test/confserver/test_confserver.py +tools/kconfig_new/test/gen_kconfig_doc/test_kconfig_out.py +tools/kconfig_new/test/gen_kconfig_doc/test_target_visibility.py +tools/ldgen/fragments.py +tools/ldgen/generation.py +tools/ldgen/ldgen.py +tools/ldgen/ldgen_common.py +tools/ldgen/sdkconfig.py +tools/ldgen/test/test_fragments.py +tools/ldgen/test/test_generation.py +tools/mass_mfg/mfg_gen.py +tools/mkuf2.py +tools/test_apps/protocols/mqtt/publish_connect_test/app_test.py +tools/test_apps/protocols/openssl/app_test.py +tools/test_apps/protocols/pppos/app_test.py +tools/test_apps/system/gdb_loadable_elf/app_test.py +tools/test_apps/system/memprot/app_test.py +tools/test_apps/system/monitor_ide_integration/app_test.py +tools/test_apps/system/panic/app_test.py +tools/test_apps/system/panic/panic_tests.py +tools/test_apps/system/panic/test_panic_util/test_panic_util.py +tools/test_apps/system/startup/app_test.py +tools/test_idf_monitor/run_test_idf_monitor.py +tools/test_idf_py/extra_path/some_ext.py +tools/test_idf_py/idf_ext.py +tools/test_idf_py/test_idf_extensions/test_ext/test_extension.py +tools/test_idf_py/test_idf_py.py +tools/test_idf_size/test_idf_size.py +tools/test_idf_tools/test_idf_tools.py +tools/test_mkdfu/test_mkdfu.py +tools/test_mkuf2/test_mkuf2.py +tools/unit-test-app/idf_ext.py +tools/unit-test-app/tools/CreateSectionTable.py +tools/unit-test-app/tools/UnitTestParser.py +tools/unit-test-app/unit_test.py +tools/windows/eclipse_make.py +tools/windows/tool_setup/system_check/system_check_download.py +tools/windows/tool_setup/system_check/system_check_subprocess.py +tools/windows/tool_setup/system_check/system_check_virtualenv.py diff --git a/tools/mkdfu.py b/tools/mkdfu.py index e521354fe1..4429d17b74 100755 --- a/tools/mkdfu.py +++ b/tools/mkdfu.py @@ -38,7 +38,7 @@ except ImportError: pass try: - from itertools import izip as zip + from itertools import izip as zip # type: ignore except ImportError: # Python 3 pass @@ -125,7 +125,7 @@ def pad_bytes(b, multiple, padding=b'\x00'): # type: (bytes, int, bytes) -> byt class EspDfuWriter(object): - def __init__(self, dest_file, pid): # type: (typing.BinaryIO) -> None + def __init__(self, dest_file, pid): # type: (typing.BinaryIO, int) -> None self.dest = dest_file self.pid = pid self.entries = [] # type: typing.List[bytes] @@ -187,7 +187,7 @@ class EspDfuWriter(object): self.entries.insert(0, entry) -def action_write(args): +def action_write(args): # type: (typing.Mapping[str, typing.Any]) -> None writer = EspDfuWriter(args['output_file'], args['pid']) for addr, f in args['files']: print('Adding {} at {:#x}'.format(f, addr)) @@ -196,7 +196,7 @@ def action_write(args): print('"{}" has been written. You may proceed with DFU flashing.'.format(args['output_file'].name)) -def main(): +def main(): # type: () -> None parser = argparse.ArgumentParser() # Provision to add "info" command @@ -218,7 +218,7 @@ def main(): args = parser.parse_args() - def check_file(file_name): + def check_file(file_name): # type: (str) -> str if not os.path.isfile(file_name): raise RuntimeError('{} is not a regular file!'.format(file_name)) return file_name @@ -230,7 +230,7 @@ def main(): if args.json: json_dir = os.path.dirname(os.path.abspath(args.json)) - def process_json_file(path): + def process_json_file(path): # type: (str) -> str ''' The input path is relative to json_dir. This function makes it relative to the current working directory. diff --git a/tools/unit-test-app/tools/UnitTestParser.py b/tools/unit-test-app/tools/UnitTestParser.py index 3fff495b18..7cf86440be 100644 --- a/tools/unit-test-app/tools/UnitTestParser.py +++ b/tools/unit-test-app/tools/UnitTestParser.py @@ -13,7 +13,7 @@ import yaml try: from yaml import CLoader as Loader except ImportError: - from yaml import Loader as Loader + from yaml import Loader as Loader # type: ignore TEST_CASE_PATTERN = { 'initial condition': 'UTINIT1',