Merge pull request #3118 from gilles-peskine-arm/pylint-up-to-2.4-development
Pass Pylint up to 2.4
diff --git a/.pylintrc b/.pylintrc
index 037717e..ad25a7c 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -40,7 +40,27 @@
max-module-lines=2000
[MESSAGES CONTROL]
-disable=
+# * locally-disabled, locally-enabled: If we disable or enable a message
+# locally, it's by design. There's no need to clutter the Pylint output
+# with this information.
+# * logging-format-interpolation: Pylint warns about things like
+# ``log.info('...'.format(...))``. It insists on ``log.info('...', ...)``.
+# This is of minor utility (mainly a performance gain when there are
+# many messages that use formatting and are below the log level).
+# Some versions of Pylint (including 1.8, which is the version on
+# Ubuntu 18.04) only recognize old-style format strings using '%',
+# and complain about something like ``log.info('{}', foo)`` with
+# logging-too-many-args (Pylint supports new-style formatting if
+# declared globally with logging_format_style under [LOGGING] but
+# this requires Pylint >=2.2).
+# * no-else-return: Allow the perfectly reasonable idiom
+# if condition1:
+# return value1
+# else:
+# return value2
+# * unnecessary-pass: If we take the trouble of adding a line with "pass",
+# it's because we think the code is clearer that way.
+disable=locally-disabled,locally-enabled,logging-format-interpolation,no-else-return,unnecessary-pass
[REPORTS]
# Don't diplay statistics. Just the facts.
diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index e19f2c0..c2aca50 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -29,7 +29,7 @@
import xml.etree.ElementTree as ET
-class AbiChecker(object):
+class AbiChecker:
"""API and ABI checker."""
def __init__(self, old_version, new_version, configuration):
diff --git a/scripts/config.py b/scripts/config.py
index b7a9a08..20521a5 100755
--- a/scripts/config.py
+++ b/scripts/config.py
@@ -283,9 +283,13 @@
def __init__(self, filename=None):
"""Read the Mbed TLS configuration file."""
if filename is None:
- for filename in self.default_path:
- if os.path.lexists(filename):
+ for candidate in self.default_path:
+ if os.path.lexists(candidate):
+ filename = candidate
break
+ else:
+ raise Exception('Mbed TLS configuration file not found',
+ self.default_path)
super().__init__()
self.filename = filename
self.current_section = 'header'
@@ -448,7 +452,7 @@
value = config[args.symbol]
if value:
sys.stdout.write(value + '\n')
- return args.symbol not in config
+ return 0 if args.symbol in config else 1
elif args.command == 'set':
if not args.force and args.symbol not in config.settings:
sys.stderr.write("A #define for the symbol {} "
@@ -461,6 +465,7 @@
else:
config.adapt(args.adapter)
config.write(args.write)
+ return 0
# Import modules only used by main only if main is defined and called.
# pylint: disable=wrong-import-position
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index c3d0f6f..5b70caa 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -1773,15 +1773,6 @@
unset gdb_disable_aslr
}
-support_check_python_files () {
- # Find the installed version of Pylint. Installed as a distro package this can
- # be pylint3 and as a PEP egg, pylint.
- if type pylint >/dev/null 2>/dev/null || type pylint3 >/dev/null 2>/dev/null; then
- true;
- else
- false;
- fi
-}
component_check_python_files () {
msg "Lint: Python scripts"
record_status tests/scripts/check-python-files.sh
diff --git a/tests/scripts/check-files.py b/tests/scripts/check-files.py
index 6e35f52..65edecc 100755
--- a/tests/scripts/check-files.py
+++ b/tests/scripts/check-files.py
@@ -17,7 +17,7 @@
import sys
-class FileIssueTracker(object):
+class FileIssueTracker:
"""Base class for file-wide issue tracking.
To implement a checker that processes a file as a whole, inherit from
@@ -37,20 +37,31 @@
self.files_with_issues = {}
def should_check_file(self, filepath):
+ """Whether the given file name should be checked.
+
+ Files whose name ends with a string listed in ``self.files_exemptions``
+ will not be checked.
+ """
for files_exemption in self.files_exemptions:
if filepath.endswith(files_exemption):
return False
return True
def check_file_for_issue(self, filepath):
+ """Check the specified file for the issue that this class is for.
+
+ Subclasses must implement this method.
+ """
raise NotImplementedError
def record_issue(self, filepath, line_number):
+ """Record that an issue was found at the specified location."""
if filepath not in self.files_with_issues.keys():
self.files_with_issues[filepath] = []
self.files_with_issues[filepath].append(line_number)
def output_file_issues(self, logger):
+ """Log all the locations where the issue was found."""
if self.files_with_issues.values():
logger.info(self.heading)
for filename, lines in sorted(self.files_with_issues.items()):
@@ -70,6 +81,10 @@
"""
def issue_with_line(self, line, filepath):
+ """Check the specified line for the issue that this class is for.
+
+ Subclasses must implement this method.
+ """
raise NotImplementedError
def check_file_line(self, filepath, line, line_number):
@@ -77,6 +92,10 @@
self.record_issue(filepath, line_number)
def check_file_for_issue(self, filepath):
+ """Check the lines of the specified file.
+
+ Subclasses must implement the ``issue_with_line`` method.
+ """
with open(filepath, "rb") as f:
for i, line in enumerate(iter(f.readline, b"")):
self.check_file_line(filepath, line, i + 1)
@@ -170,7 +189,7 @@
return False
-class IntegrityChecker(object):
+class IntegrityChecker:
"""Sanity-check files under the current directory."""
def __init__(self, log_file):
diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh
index 6b864d2..cd18518 100755
--- a/tests/scripts/check-python-files.sh
+++ b/tests/scripts/check-python-files.sh
@@ -9,15 +9,10 @@
# Run 'pylint' on Python files for programming errors and helps enforcing
# PEP8 coding standards.
-# Find the installed version of Pylint. Installed as a distro package this can
-# be pylint3 and as a PEP egg, pylint. We prefer pylint over pylint3
-if type pylint >/dev/null 2>/dev/null; then
- PYLINT=pylint
-elif type pylint3 >/dev/null 2>/dev/null; then
- PYLINT=pylint3
+if type python3 >/dev/null 2>/dev/null; then
+ PYTHON=python3
else
- echo 'Pylint was not found.'
- exit 1
+ PYTHON=python
fi
-$PYLINT -j 2 scripts/*.py tests/scripts/*.py
+$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py
diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py
index 4abaa68..35a9987 100755
--- a/tests/scripts/check-test-cases.py
+++ b/tests/scripts/check-test-cases.py
@@ -77,6 +77,7 @@
seen[description] = line_number
def check_test_suite(results, data_file_name):
+ """Check the test cases in the given unit test data file."""
in_paragraph = False
descriptions = {}
with open(data_file_name, 'rb') as data_file:
@@ -94,6 +95,7 @@
in_paragraph = True
def check_ssl_opt_sh(results, file_name):
+ """Check the test cases in ssl-opt.sh or a file with a similar format."""
descriptions = {}
with open(file_name, 'rb') as file_contents:
for line_number, line in enumerate(file_contents, 1):
diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py
index 1fff099..21f816e 100755
--- a/tests/scripts/generate_test_code.py
+++ b/tests/scripts/generate_test_code.py
@@ -208,7 +208,7 @@
pass
-class FileWrapper(io.FileIO, object):
+class FileWrapper(io.FileIO):
"""
This class extends built-in io.FileIO class with attribute line_no,
that indicates line number for the line that is read.
@@ -402,8 +402,7 @@
:param inp_str: Input string with macros delimited by ':'.
:return: list of dependencies
"""
- dependencies = [dep for dep in map(validate_dependency,
- inp_str.split(':'))]
+ dependencies = list(map(validate_dependency, inp_str.split(':')))
return dependencies
diff --git a/tests/scripts/mbedtls_test.py b/tests/scripts/mbedtls_test.py
index 8f24435..709bb1a 100755
--- a/tests/scripts/mbedtls_test.py
+++ b/tests/scripts/mbedtls_test.py
@@ -1,3 +1,5 @@
+#!/usr/bin/env python3
+
# Greentea host test script for Mbed TLS on-target test suite testing.
#
# Copyright (C) 2018, Arm Limited, All Rights Reserved
@@ -46,7 +48,7 @@
pass
-class TestDataParser(object):
+class TestDataParser:
"""
Parses test name, dependencies, test function name and test parameters
from the data file.
@@ -260,7 +262,7 @@
data_bytes += bytearray(dependencies)
data_bytes += bytearray([function_id, len(parameters)])
for typ, param in parameters:
- if typ == 'int' or typ == 'exp':
+ if typ in ('int', 'exp'):
i = int(param, 0)
data_bytes += b'I' if typ == 'int' else b'E'
self.align_32bit(data_bytes)
diff --git a/tests/scripts/test_config_script.py b/tests/scripts/test_config_script.py
index 40ed9fd..c8fdea5 100755
--- a/tests/scripts/test_config_script.py
+++ b/tests/scripts/test_config_script.py
@@ -92,6 +92,7 @@
return re.split(r'[ ,]+', options.presets)
else:
help_text = subprocess.run([options.script, '--help'],
+ check=False, # config.pl --help returns 255
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT).stdout
return guess_presets_from_help(help_text.decode('ascii'))
diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py
index 6d7113e..c8e8c5c 100755
--- a/tests/scripts/test_generate_test_code.py
+++ b/tests/scripts/test_generate_test_code.py
@@ -294,7 +294,7 @@
self.assertEqual(code, expected)
-class StringIOWrapper(StringIO, object):
+class StringIOWrapper(StringIO):
"""
file like class to mock file object in tests.
"""
@@ -1127,9 +1127,8 @@
dhm_selftest:
"""
stream = StringIOWrapper('test_suite_ut.function', data)
- tests = [(name, test_function, dependencies, args)
- for name, test_function, dependencies, args in
- parse_test_data(stream)]
+ # List of (name, function_name, dependencies, args)
+ tests = list(parse_test_data(stream))
test1, test2, test3, test4 = tests
self.assertEqual(test1[0], 'Diffie-Hellman full exchange #1')
self.assertEqual(test1[1], 'dhm_do_dhm')
@@ -1170,9 +1169,8 @@
"""
stream = StringIOWrapper('test_suite_ut.function', data)
- tests = [(name, function_name, dependencies, args)
- for name, function_name, dependencies, args in
- parse_test_data(stream)]
+ # List of (name, function_name, dependencies, args)
+ tests = list(parse_test_data(stream))
test1, test2 = tests
self.assertEqual(test1[0], 'Diffie-Hellman full exchange #1')
self.assertEqual(test1[1], 'dhm_do_dhm')
diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py
index c02555e..2c9f058 100755
--- a/tests/scripts/test_psa_constant_names.py
+++ b/tests/scripts/test_psa_constant_names.py
@@ -453,7 +453,7 @@
tests.run_all(inputs)
tests.report(sys.stdout)
if tests.errors:
- exit(1)
+ sys.exit(1)
if __name__ == '__main__':
main()