From 8ba3cc4394b716aac1ef015f2940abaf90dd872b Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 25 Nov 2021 14:13:21 +0200 Subject: [PATCH 1/4] Move mypy and pylint configs in pyproject.toml This aims to add a single source of truth for pylint and mypy configurations. Signed-off-by: Martin Vrachev --- pyproject.toml | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ setup.cfg | 20 ------------- tox.ini | 2 +- tuf/api/pylintrc | 46 ------------------------------ 4 files changed, 75 insertions(+), 67 deletions(-) delete mode 100644 tuf/api/pylintrc diff --git a/pyproject.toml b/pyproject.toml index 2f21011953..245e2cdf5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,77 @@ [build-system] requires = ["setuptools>=40.8.0", "wheel"] build-backend = "setuptools.build_meta" + +# Pylint section + +# Minimal pylint configuration file for Secure Systems Lab Python Style Guide: +# https://github.com/secure-systems-lab/code-style-guidelines +# +# Based on Google Python Style Guide pylintrc and pylint defaults: +# https://google.github.io/styleguide/pylintrc +# http://pylint.pycqa.org/en/latest/technical_reference/features.html + +[tool.pylint.message_control] +# Disable the message, report, category or checker with the given id(s). +# NOTE: To keep this config as short as possible we only disable checks that +# are currently in conflict with our code. If new code displeases the linter +# (for good reasons) consider updating this config file, or disable checks with. +disable=[ + "fixme", + "too-few-public-methods", + "too-many-arguments", + "format", + "duplicate-code" +] + +[tool.pylint.basic] +good-names = ["i","j","k","v","e","f","fn","fp","_type","_"] +# Regexes for allowed names are copied from the Google pylintrc +# NOTE: Pylint captures regex name groups such as 'snake_case' or 'camel_case'. +# If there are multiple groups it enfoces the prevalent naming style inside +# each modules. Names in the exempt capturing group are ignored. +function-rgx="^(?:(?PsetUp|tearDown|setUpModule|tearDownModule)|(?P_?[A-Z][a-zA-Z0-9]*)|(?P_?[a-z][a-z0-9_]*))$" +method-rgx="(?x)^(?:(?P_[a-z0-9_]+__|runTest|setUp|tearDown|setUpTestCase|tearDownTestCase|setupSelf|tearDownClass|setUpClass|(test|assert)_*[A-Z0-9][a-zA-Z0-9_]*|next)|(?P_{0,2}[A-Z][a-zA-Z0-9_]*)|(?P_{0,2}[a-z][a-z0-9_]*))$" +argument-rgx="^[a-z][a-z0-9_]*$" +attr-rgx="^_{0,2}[a-z][a-z0-9_]*$" +class-attribute-rgx="^(_?[A-Z][A-Z0-9_]*|__[a-z0-9_]+__|_?[a-z][a-z0-9_]*)$" +class-rgx="^_?[A-Z][a-zA-Z0-9]*$" +const-rgx="^(_?[A-Z][A-Z0-9_]*|__[a-z0-9_]+__|_?[a-z][a-z0-9_]*)$" +inlinevar-rgx="^[a-z][a-z0-9_]*$" +module-rgx="^(_?[a-z][a-z0-9_]*|__init__)$" +no-docstring-rgx="(__.*__|main|test.*|.*test|.*Test)$" +variable-rgx="^[a-z][a-z0-9_]*$" +docstring-min-length=10 + +[tool.pylint.logging] +logging-format-style="old" + +[tool.pylint.miscellaneous] +notes="TODO" + +[tool.pylint.STRING] +check-quote-consistency="yes" + +# mypy section +# Read more here: https://mypy.readthedocs.io/en/stable/config_file.html#using-a-pyproject-toml-file +[tool.mypy] +warn_unused_configs = "True" +warn_redundant_casts = "True" +warn_unused_ignores = "True" +warn_unreachable = "True" +strict_equality = "True" +disallow_untyped_defs = "True" +disallow_untyped_calls = "True" +show_error_codes = "True" +files = [ + "tuf/api/", + "tuf/ngclient", + "tuf/exceptions.py" +] + +[[tool.mypy.overrides]] +module = [ + "securesystemslib.*", + "urllib3.*" +] +ignore_missing_imports = "True" diff --git a/setup.cfg b/setup.cfg index d86316a13e..73a975b948 100644 --- a/setup.cfg +++ b/setup.cfg @@ -52,23 +52,3 @@ tuf = py.typed ignore = .fossa.yml .readthedocs.yaml - -[mypy] -warn_unused_configs = True -warn_redundant_casts = True -warn_unused_ignores = True -warn_unreachable = True -strict_equality = True -disallow_untyped_defs = True -disallow_untyped_calls = True -show_error_codes = True -files = - tuf/api/, - tuf/ngclient, - tuf/exceptions.py - -[mypy-securesystemslib.*] -ignore_missing_imports = True - -[mypy-urllib3.*] -ignore_missing_imports = True diff --git a/tox.ini b/tox.ini index b8359b7772..767b1b4a86 100644 --- a/tox.ini +++ b/tox.ini @@ -43,7 +43,7 @@ commands = # TODO: configure black and isort args in pyproject.toml (see #1161) black --check --diff --line-length 80 tuf/api tuf/ngclient isort --check --diff --line-length 80 --profile black -p tuf tuf/api tuf/ngclient - pylint -j 0 tuf/api tuf/ngclient --rcfile=tuf/api/pylintrc + pylint -j 0 tuf/api tuf/ngclient --rcfile=pyproject.toml # NOTE: Contrary to what the pylint docs suggest, ignoring full paths does # work, unfortunately each subdirectory has to be ignored explicitly. diff --git a/tuf/api/pylintrc b/tuf/api/pylintrc deleted file mode 100644 index d9b1da754a..0000000000 --- a/tuf/api/pylintrc +++ /dev/null @@ -1,46 +0,0 @@ -# Minimal pylint configuration file for Secure Systems Lab Python Style Guide: -# https://github.com/secure-systems-lab/code-style-guidelines -# -# Based on Google Python Style Guide pylintrc and pylint defaults: -# https://google.github.io/styleguide/pylintrc -# http://pylint.pycqa.org/en/latest/technical_reference/features.html - -[MESSAGES CONTROL] -# Disable the message, report, category or checker with the given id(s). -# NOTE: To keep this config as short as possible we only disable checks that -# are currently in conflict with our code. If new code displeases the linter -# (for good reasons) consider updating this config file, or disable checks with -# 'pylint: disable=XYZ' comments. -disable=fixme, - too-few-public-methods, - too-many-arguments, - format, - duplicate-code, - -[BASIC] -good-names=i,j,k,v,e,f,fn,fp,_type,_ -# Regexes for allowed names are copied from the Google pylintrc -# NOTE: Pylint captures regex name groups such as 'snake_case' or 'camel_case'. -# If there are multiple groups it enfoces the prevalent naming style inside -# each modules. Names in the exempt capturing group are ignored. -function-rgx=^(?:(?PsetUp|tearDown|setUpModule|tearDownModule)|(?P_?[A-Z][a-zA-Z0-9]*)|(?P_?[a-z][a-z0-9_]*))$ -method-rgx=(?x)^(?:(?P_[a-z0-9_]+__|runTest|setUp|tearDown|setUpTestCase|tearDownTestCase|setupSelf|tearDownClass|setUpClass|(test|assert)_*[A-Z0-9][a-zA-Z0-9_]*|next)|(?P_{0,2}[A-Z][a-zA-Z0-9_]*)|(?P_{0,2}[a-z][a-z0-9_]*))$ -argument-rgx=^[a-z][a-z0-9_]*$ -attr-rgx=^_{0,2}[a-z][a-z0-9_]*$ -class-attribute-rgx=^(_?[A-Z][A-Z0-9_]*|__[a-z0-9_]+__|_?[a-z][a-z0-9_]*)$ -class-rgx=^_?[A-Z][a-zA-Z0-9]*$ -const-rgx=^(_?[A-Z][A-Z0-9_]*|__[a-z0-9_]+__|_?[a-z][a-z0-9_]*)$ -inlinevar-rgx=^[a-z][a-z0-9_]*$ -module-rgx=^(_?[a-z][a-z0-9_]*|__init__)$ -no-docstring-rgx=(__.*__|main|test.*|.*test|.*Test)$ -variable-rgx=^[a-z][a-z0-9_]*$ -docstring-min-length=10 - -[LOGGING] -logging-format-style=old - -[MISCELLANEOUS] -notes=TODO - -[STRING] -check-quote-consistency=yes From 4597761adbc38b711eac159d57ed7f58cbbb72d8 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Sun, 28 Nov 2021 00:16:04 +0200 Subject: [PATCH 2/4] Move black configuration in pyproject.toml We are using 4 linters: black, isort, pylint and mypy. It's good if we use one file as a source for truth for all linter configurations. As a first step move black options in pyproject.toml. I tried multiple ways to use the include option, so we can just call black --config=pyproject.toml, but I was not successful. Then I found this comment https://github.com/psf/black/issues/861#issuecomment-680411125 explaining that the path argument is mandatory. As a result, I will move all configuration options for black inside pyproject.toml without the actual directories that need to be linted. Signed-off-by: Martin Vrachev --- docs/CONTRIBUTORS.rst | 2 +- pyproject.toml | 6 ++++++ tox.ini | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/CONTRIBUTORS.rst b/docs/CONTRIBUTORS.rst index 302c8c205b..bf68594158 100644 --- a/docs/CONTRIBUTORS.rst +++ b/docs/CONTRIBUTORS.rst @@ -114,7 +114,7 @@ Auto-formatting can be done on the command line: :: $ # TODO: configure black and isort args in pyproject.toml (see #1161) - $ black --line-length 80 tuf/api + $ black $ isort --line-length 80 --profile black -p tuf tuf/api or via source code editor plugin diff --git a/pyproject.toml b/pyproject.toml index 245e2cdf5f..be5e723804 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,13 @@ +# Build-system section [build-system] requires = ["setuptools>=40.8.0", "wheel"] build-backend = "setuptools.build_meta" +# Black section +# Read more here: https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file +[tool.black] +line-length=80 + # Pylint section # Minimal pylint configuration file for Secure Systems Lab Python Style Guide: diff --git a/tox.ini b/tox.ini index 767b1b4a86..b06150ea57 100644 --- a/tox.ini +++ b/tox.ini @@ -41,7 +41,7 @@ changedir = {toxinidir} commands = # Use different configs for new (tuf/api/*) and legacy code # TODO: configure black and isort args in pyproject.toml (see #1161) - black --check --diff --line-length 80 tuf/api tuf/ngclient + black --check --diff tuf/api tuf/ngclient isort --check --diff --line-length 80 --profile black -p tuf tuf/api tuf/ngclient pylint -j 0 tuf/api tuf/ngclient --rcfile=pyproject.toml From ed8a06bcb3541235e2c47e01249fc801382b06cf Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 30 Nov 2021 16:40:44 +0200 Subject: [PATCH 3/4] Move part of isort options in pyproject.toml We are using 4 linters: black, isort, pylint and mypy. It's good if we use one file as a source for truth for all linter configurations. I tried multiple ways to use the src_path option, so we can just call isort without pointing out the target folders, but I was not successful. I tried running isort with "isort --settings-path=pyproject.toml" I got the error: "Error: arguments passed in without any paths or content." Additionally, I saw one project with source configuration https://github.com/Pylons/pyramid/blob/master/pyproject.toml, but they had to give explicit folders too https://github.com/Pylons/pyramid/blob/8061fce297cc7117d3e6e2b39e47512c7db2904f/tox.ini#L26 and https://github.com/Pylons/pyramid/blob/8061fce297cc7117d3e6e2b39e47512c7db2904f/tox.ini#L66 It was a similar situation with "check" and "diff". In the documentation it's said that for both check and diff are not supported in configuration files. See: - https://pycqa.github.io/isort/docs/configuration/options.html#check - https://pycqa.github.io/isort/docs/configuration/options.html#show-diff Additionally, in two issues it was confirmed that in integration tests we should use --check and --diff the way we did until now. As a result, I moved part of the configuration options for isort inside pyproject.toml without the actual directories that need to be linted and "check" and "diff" options. Signed-off-by: Martin Vrachev --- docs/CONTRIBUTORS.rst | 3 +-- pyproject.toml | 7 +++++++ tox.ini | 3 +-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/CONTRIBUTORS.rst b/docs/CONTRIBUTORS.rst index bf68594158..54ef8a8d40 100644 --- a/docs/CONTRIBUTORS.rst +++ b/docs/CONTRIBUTORS.rst @@ -113,9 +113,8 @@ CI/CD will check that new TUF code is formatted with `black Auto-formatting can be done on the command line: :: - $ # TODO: configure black and isort args in pyproject.toml (see #1161) $ black - $ isort --line-length 80 --profile black -p tuf tuf/api + $ isort or via source code editor plugin [`black `__, diff --git a/pyproject.toml b/pyproject.toml index be5e723804..339f2416dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,6 +8,13 @@ build-backend = "setuptools.build_meta" [tool.black] line-length=80 +# Isort section +# Read more here: https://pycqa.github.io/isort/docs/configuration/config_files.html +[tool.isort] +profile="black" +line_length=80 +known_first_party = ["tuf"] + # Pylint section # Minimal pylint configuration file for Secure Systems Lab Python Style Guide: diff --git a/tox.ini b/tox.ini index b06150ea57..a6e0d765d8 100644 --- a/tox.ini +++ b/tox.ini @@ -40,9 +40,8 @@ commands = changedir = {toxinidir} commands = # Use different configs for new (tuf/api/*) and legacy code - # TODO: configure black and isort args in pyproject.toml (see #1161) black --check --diff tuf/api tuf/ngclient - isort --check --diff --line-length 80 --profile black -p tuf tuf/api tuf/ngclient + isort --check --diff tuf/api tuf/ngclient pylint -j 0 tuf/api tuf/ngclient --rcfile=pyproject.toml # NOTE: Contrary to what the pylint docs suggest, ignoring full paths does From 5c8a86665fb8f6f3e7848893eb7c9f10978a9b86 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 1 Dec 2021 16:11:31 +0200 Subject: [PATCH 4/4] Fix small pylint error Signed-off-by: Martin Vrachev --- tuf/api/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 4ab4360fe5..80ef390131 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -539,7 +539,7 @@ def __init__( ): if not all( isinstance(at, str) for at in [keyid, keytype, scheme] - ) or not isinstance(keyval, Dict): + ) or not isinstance(keyval, dict): raise TypeError("Unexpected Key attributes types!") self.keyid = keyid self.keytype = keytype