diff --git a/.mypy.ini b/.mypy.ini new file mode 100644 index 0000000000..6b831ddb77 --- /dev/null +++ b/.mypy.ini @@ -0,0 +1,4 @@ +[mypy] +mypy_path = scripts +namespace_packages = True +warn_unused_configs = True diff --git a/.pylintrc b/.pylintrc index ad25a7ca17..5f3d2b235f 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,3 +1,6 @@ +[MASTER] +init-hook='import sys; sys.path.append("scripts")' + [BASIC] # We're ok with short funtion argument names. # [invalid-name] diff --git a/.travis.yml b/.travis.yml index 76cb1c5372..9b729ec071 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ jobs: language: python # Needed to get pip for Python 3 python: 3.5 # version from Ubuntu 16.04 install: - - pip install pylint==2.4.4 + - pip install mypy==0.780 pylint==2.4.4 script: - tests/scripts/all.sh -k 'check_*' - tests/scripts/all.sh -k test_default_out_of_box diff --git a/scripts/mbedtls_dev/c_build_helper.py b/scripts/mbedtls_dev/c_build_helper.py new file mode 100644 index 0000000000..5c587a16bb --- /dev/null +++ b/scripts/mbedtls_dev/c_build_helper.py @@ -0,0 +1,138 @@ +"""Generate and run C code. +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# 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 os +import platform +import subprocess +import sys +import tempfile + +def remove_file_if_exists(filename): + """Remove the specified file, ignoring errors.""" + if not filename: + return + try: + os.remove(filename) + except OSError: + pass + +def create_c_file(file_label): + """Create a temporary C file. + + * ``file_label``: a string that will be included in the file name. + + Return ```(c_file, c_name, exe_name)``` where ``c_file`` is a Python + stream open for writing to the file, ``c_name`` is the name of the file + and ``exe_name`` is the name of the executable that will be produced + by compiling the file. + """ + c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(file_label), + suffix='.c') + exe_suffix = '.exe' if platform.system() == 'Windows' else '' + exe_name = c_name[:-2] + exe_suffix + remove_file_if_exists(exe_name) + c_file = os.fdopen(c_fd, 'w', encoding='ascii') + return c_file, c_name, exe_name + +def generate_c_printf_expressions(c_file, cast_to, printf_format, expressions): + """Generate C instructions to print the value of ``expressions``. + + Write the code with ``c_file``'s ``write`` method. + + Each expression is cast to the type ``cast_to`` and printed with the + printf format ``printf_format``. + """ + for expr in expressions: + c_file.write(' printf("{}\\n", ({}) {});\n' + .format(printf_format, cast_to, expr)) + +def generate_c_file(c_file, + caller, header, + main_generator): + """Generate a temporary C source file. + + * ``c_file`` is an open stream on the C source file. + * ``caller``: an informational string written in a comment at the top + of the file. + * ``header``: extra code to insert before any function in the generated + C file. + * ``main_generator``: a function called with ``c_file`` as its sole argument + to generate the body of the ``main()`` function. + """ + c_file.write('/* Generated by {} */' + .format(caller)) + c_file.write(''' +#include +''') + c_file.write(header) + c_file.write(''' +int main(void) +{ +''') + main_generator(c_file) + c_file.write(''' return 0; +} +''') + +def get_c_expression_values( + cast_to, printf_format, + expressions, + caller=__name__, file_label='', + header='', include_path=None, + keep_c=False, +): # pylint: disable=too-many-arguments + """Generate and run a program to print out numerical values for expressions. + + * ``cast_to``: a C type. + * ``printf_format``: a printf format suitable for the type ``cast_to``. + * ``header``: extra code to insert before any function in the generated + C file. + * ``expressions``: a list of C language expressions that have the type + ``cast_to``. + * ``include_path``: a list of directories containing header files. + * ``keep_c``: if true, keep the temporary C file (presumably for debugging + purposes). + + Return the list of values of the ``expressions``. + """ + if include_path is None: + include_path = [] + c_name = None + exe_name = None + try: + c_file, c_name, exe_name = create_c_file(file_label) + generate_c_file( + c_file, caller, header, + lambda c_file: generate_c_printf_expressions(c_file, + cast_to, printf_format, + expressions) + ) + c_file.close() + cc = os.getenv('CC', 'cc') + subprocess.check_call([cc] + + ['-I' + dir for dir in include_path] + + ['-o', exe_name, c_name]) + if keep_c: + sys.stderr.write('List of {} tests kept at {}\n' + .format(caller, c_name)) + else: + os.remove(c_name) + output = subprocess.check_output([exe_name]) + return output.decode('ascii').strip().split('\n') + finally: + remove_file_if_exists(exe_name) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 518c423d9c..449803a543 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -14,11 +14,13 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -# -# Purpose: -# -# Run 'pylint' on Python files for programming errors and helps enforcing -# PEP8 coding standards. + +# Purpose: check Python files for potential programming errors or maintenance +# hurdles. Run pylint to detect some potential mistakes and enforce PEP8 +# coding standards. If available, run mypy to perform static type checking. + +# We'll keep going on errors and report the status at the end. +ret=0 if type python3 >/dev/null 2>/dev/null; then PYTHON=python3 @@ -26,4 +28,56 @@ else PYTHON=python fi -$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py +check_version () { + $PYTHON - "$2" <&2 "pylint reported errors" + ret=1 +} + +# Check types if mypy is available +if can_mypy; then + echo + echo 'Running mypy ...' + $PYTHON -m mypy scripts/*.py tests/scripts/*.py || + ret=1 +fi + +exit $ret diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index 13fee9d762..8857e00213 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -29,6 +29,10 @@ import codecs import re import subprocess import sys +try: + from typing import FrozenSet, Optional, Pattern # pylint: disable=unused-import +except ImportError: + pass class FileIssueTracker: @@ -48,8 +52,8 @@ class FileIssueTracker: ``heading``: human-readable description of the issue """ - suffix_exemptions = frozenset() - path_exemptions = None + suffix_exemptions = frozenset() #type: FrozenSet[str] + path_exemptions = None #type: Optional[Pattern[str]] # heading must be defined in derived classes. # pylint: disable=no-member @@ -161,13 +165,64 @@ class PermissionIssueTracker(FileIssueTracker): heading = "Incorrect permissions:" + # .py files can be either full scripts or modules, so they may or may + # not be executable. + suffix_exemptions = frozenset({".py"}) + def check_file_for_issue(self, filepath): is_executable = os.access(filepath, os.X_OK) - should_be_executable = filepath.endswith((".sh", ".pl", ".py")) + should_be_executable = filepath.endswith((".sh", ".pl")) if is_executable != should_be_executable: self.files_with_issues[filepath] = None +class ShebangIssueTracker(FileIssueTracker): + """Track files with a bad, missing or extraneous shebang line. + + Executable scripts must start with a valid shebang (#!) line. + """ + + heading = "Invalid shebang line:" + + # Allow either /bin/sh, /bin/bash, or /usr/bin/env. + # Allow at most one argument (this is a Linux limitation). + # For sh and bash, the argument if present must be options. + # For env, the argument must be the base name of the interpeter. + _shebang_re = re.compile(rb'^#! ?(?:/bin/(bash|sh)(?: -[^\n ]*)?' + rb'|/usr/bin/env ([^\n /]+))$') + _extensions = { + b'bash': 'sh', + b'perl': 'pl', + b'python3': 'py', + b'sh': 'sh', + } + + def is_valid_shebang(self, first_line, filepath): + m = re.match(self._shebang_re, first_line) + if not m: + return False + interpreter = m.group(1) or m.group(2) + if interpreter not in self._extensions: + return False + if not filepath.endswith('.' + self._extensions[interpreter]): + return False + return True + + def check_file_for_issue(self, filepath): + is_executable = os.access(filepath, os.X_OK) + with open(filepath, "rb") as f: + first_line = f.readline() + if first_line.startswith(b'#!'): + if not is_executable: + # Shebang on a non-executable file + self.files_with_issues[filepath] = None + elif not self.is_valid_shebang(first_line, filepath): + self.files_with_issues[filepath] = [1] + elif is_executable: + # Executable without a shebang + self.files_with_issues[filepath] = None + + class EndOfFileNewlineIssueTracker(FileIssueTracker): """Track files that end with an incomplete line (no newline character at the end of the last line).""" @@ -288,6 +343,7 @@ class IntegrityChecker: self.setup_logger(log_file) self.issues_to_check = [ PermissionIssueTracker(), + ShebangIssueTracker(), EndOfFileNewlineIssueTracker(), Utf8BomIssueTracker(), UnixLineEndingIssueTracker(), diff --git a/tests/scripts/mbedtls_test.py b/tests/scripts/mbedtls_test.py index a5d094064c..64f12bbb31 100755 --- a/tests/scripts/mbedtls_test.py +++ b/tests/scripts/mbedtls_test.py @@ -38,7 +38,7 @@ import re import os import binascii -from mbed_host_tests import BaseHostTest, event_callback # pylint: disable=import-error +from mbed_host_tests import BaseHostTest, event_callback # type: ignore # pylint: disable=import-error class TestDataParserError(Exception): diff --git a/tests/scripts/scripts_path.py b/tests/scripts/scripts_path.py new file mode 100644 index 0000000000..10bf6f8524 --- /dev/null +++ b/tests/scripts/scripts_path.py @@ -0,0 +1,28 @@ +"""Add our Python library directory to the module search path. + +Usage: + + import scripts_path # pylint: disable=unused-import +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# 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 os +import sys + +sys.path.append(os.path.join(os.path.dirname(__file__), + os.path.pardir, os.path.pardir, + 'scripts')) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 000c2a7010..9bf66f1cc6 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -20,21 +20,10 @@ Unit tests for generate_test_code.py """ -# pylint: disable=wrong-import-order -try: - # Python 2 - from StringIO import StringIO -except ImportError: - # Python 3 - from io import StringIO +from io import StringIO from unittest import TestCase, main as unittest_main -try: - # Python 2 - from mock import patch -except ImportError: - # Python 3 - from unittest.mock import patch -# pylint: enable=wrong-import-order +from unittest.mock import patch + from generate_test_code import gen_dependencies, gen_dependencies_one_line from generate_test_code import gen_function_wrapper, gen_dispatch from generate_test_code import parse_until_pattern, GeneratorInputError @@ -317,25 +306,16 @@ class StringIOWrapper(StringIO): :return: Line read from file. """ parent = super(StringIOWrapper, self) - if getattr(parent, 'next', None): - # Python 2 - line = parent.next() - else: - # Python 3 - line = parent.__next__() + line = parent.__next__() return line - # Python 3 - __next__ = next - - def readline(self, length=0): + def readline(self, _length=0): """ Wrap the base class readline. :param length: :return: """ - # pylint: disable=unused-argument line = super(StringIOWrapper, self).readline() if line is not None: self.line_no += 1 @@ -549,38 +529,6 @@ class ParseFunctionCode(TestCase): Test suite for testing parse_function_code() """ - def assert_raises_regex(self, exp, regex, func, *args): - """ - Python 2 & 3 portable wrapper of assertRaisesRegex(p)? function. - - :param exp: Exception type expected to be raised by cb. - :param regex: Expected exception message - :param func: callable object under test - :param args: variable positional arguments - """ - parent = super(ParseFunctionCode, self) - - # Pylint does not appreciate that the super method called - # conditionally can be available in other Python version - # then that of Pylint. - # Workaround is to call the method via getattr. - # Pylint ignores that the method got via getattr is - # conditionally executed. Method has to be a callable. - # Hence, using a dummy callable for getattr default. - dummy = lambda *x: None - # First Python 3 assertRaisesRegex is checked, since Python 2 - # assertRaisesRegexp is also available in Python 3 but is - # marked deprecated. - for name in ('assertRaisesRegex', 'assertRaisesRegexp'): - method = getattr(parent, name, dummy) - if method is not dummy: - method(exp, regex, func, *args) - break - else: - raise AttributeError(" 'ParseFunctionCode' object has no attribute" - " 'assertRaisesRegex' or 'assertRaisesRegexp'" - ) - def test_no_function(self): """ Test no test function found. @@ -593,8 +541,8 @@ function ''' stream = StringIOWrapper('test_suite_ut.function', data) err_msg = 'file: test_suite_ut.function - Test functions not found!' - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) def test_no_end_case_comment(self): """ @@ -609,8 +557,8 @@ void test_func() stream = StringIOWrapper('test_suite_ut.function', data) err_msg = r'file: test_suite_ut.function - '\ 'end case pattern .*? not found!' - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) @patch("generate_test_code.parse_function_arguments") def test_function_called(self, @@ -727,8 +675,8 @@ exit: data = 'int entropy_threshold( char * a, data_t * h, int result )' err_msg = 'file: test_suite_ut.function - Test functions not found!' stream = StringIOWrapper('test_suite_ut.function', data) - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) @patch("generate_test_code.gen_dispatch") @patch("generate_test_code.gen_dependencies") diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index bead60cd8f..4ec98d3ad4 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -26,11 +26,12 @@ import argparse from collections import namedtuple import itertools import os -import platform import re import subprocess import sys -import tempfile + +import scripts_path # pylint: disable=unused-import +from mbedtls_dev import c_build_helper class ReadFileLineException(Exception): def __init__(self, filename, line_number): @@ -308,63 +309,23 @@ def gather_inputs(headers, test_suites, inputs_class=Inputs): inputs.gather_arguments() return inputs -def remove_file_if_exists(filename): - """Remove the specified file, ignoring errors.""" - if not filename: - return - try: - os.remove(filename) - except OSError: - pass - def run_c(type_word, expressions, include_path=None, keep_c=False): - """Generate and run a program to print out numerical values for expressions.""" - if include_path is None: - include_path = [] + """Generate and run a program to print out numerical values of C expressions.""" if type_word == 'status': cast_to = 'long' printf_format = '%ld' else: cast_to = 'unsigned long' printf_format = '0x%08lx' - c_name = None - exe_name = None - try: - c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(type_word), - suffix='.c', - dir='programs/psa') - exe_suffix = '.exe' if platform.system() == 'Windows' else '' - exe_name = c_name[:-2] + exe_suffix - remove_file_if_exists(exe_name) - c_file = os.fdopen(c_fd, 'w', encoding='ascii') - c_file.write('/* Generated by test_psa_constant_names.py for {} values */' - .format(type_word)) - c_file.write(''' -#include -#include -int main(void) -{ -''') - for expr in expressions: - c_file.write(' printf("{}\\n", ({}) {});\n' - .format(printf_format, cast_to, expr)) - c_file.write(''' return 0; -} -''') - c_file.close() - cc = os.getenv('CC', 'cc') - subprocess.check_call([cc] + - ['-I' + dir for dir in include_path] + - ['-o', exe_name, c_name]) - if keep_c: - sys.stderr.write('List of {} tests kept at {}\n' - .format(type_word, c_name)) - else: - os.remove(c_name) - output = subprocess.check_output([exe_name]) - return output.decode('ascii').strip().split('\n') - finally: - remove_file_if_exists(exe_name) + return c_build_helper.get_c_expression_values( + cast_to, printf_format, + expressions, + caller='test_psa_constant_names.py for {} values'.format(type_word), + file_label=type_word, + header='#include ', + include_path=include_path, + keep_c=keep_c + ) NORMALIZE_STRIP_RE = re.compile(r'\s+') def normalize(expr):