Merge pull request #3953 from gilles-peskine-arm/python-mypy-mkdir
Python upscale: pass mypy and create library directory
diff --git a/.mypy.ini b/.mypy.ini
new file mode 100644
index 0000000..6b831dd
--- /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 ad25a7c..5f3d2b2 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 76cb1c5..9b729ec 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,7 +17,7 @@
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 0000000..5c587a1
--- /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 <stdio.h>
+''')
+ 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 518c423..449803a5 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 @@
PYTHON=python
fi
-$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py
+check_version () {
+ $PYTHON - "$2" <<EOF
+import packaging.version
+import sys
+import $1 as package
+actual = package.__version__
+wanted = sys.argv[1]
+if packaging.version.parse(actual) < packaging.version.parse(wanted):
+ sys.stderr.write("$1: version %s is too old (want %s)\n" % (actual, wanted))
+ exit(1)
+EOF
+}
+
+can_pylint () {
+ # Pylint 1.5.2 from Ubuntu 16.04 is too old:
+ # E: 34, 0: Unable to import 'mbedtls_dev' (import-error)
+ # Pylint 1.8.3 from Ubuntu 18.04 passed on the first commit containing this line.
+ check_version pylint 1.8.3
+}
+
+can_mypy () {
+ # mypy 0.770 is too old:
+ # tests/scripts/test_psa_constant_names.py:34: error: Cannot find implementation or library stub for module named 'mbedtls_dev'
+ # mypy 0.780 from pip passed on the first commit containing this line.
+ check_version mypy.version 0.780
+}
+
+# With just a --can-xxx option, check whether the tool for xxx is available
+# with an acceptable version, and exit without running any checks. The exit
+# status is true if the tool is available and acceptable and false otherwise.
+if [ "$1" = "--can-pylint" ]; then
+ can_pylint
+ exit
+elif [ "$1" = "--can-mypy" ]; then
+ can_mypy
+ exit
+fi
+
+echo 'Running pylint ...'
+$PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || {
+ echo >&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 13fee9d..8857e00 100755
--- a/tests/scripts/check_files.py
+++ b/tests/scripts/check_files.py
@@ -29,6 +29,10 @@
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 @@
``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 @@
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 @@
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 a5d0940..64f12bb 100755
--- a/tests/scripts/mbedtls_test.py
+++ b/tests/scripts/mbedtls_test.py
@@ -38,7 +38,7 @@
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 0000000..10bf6f8
--- /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 000c2a7..9bf66f1 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 @@
: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 @@
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 @@
'''
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 @@
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 @@
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 bead60c..4ec98d3 100755
--- a/tests/scripts/test_psa_constant_names.py
+++ b/tests/scripts/test_psa_constant_names.py
@@ -26,11 +26,12 @@
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 @@
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 <stdio.h>
-#include <psa/crypto.h>
-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 <psa/crypto.h>',
+ include_path=include_path,
+ keep_c=keep_c
+ )
NORMALIZE_STRIP_RE = re.compile(r'\s+')
def normalize(expr):