Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move linters configurations in pyproject.toml #1699

Merged
merged 4 commits into from
Dec 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions docs/CONTRIBUTORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 --line-length 80 tuf/api
$ isort --line-length 80 --profile black -p tuf tuf/api
$ black <filename>
$ isort <filename>

or via source code editor plugin
[`black <https://black.readthedocs.io/en/stable/editor_integration.html>`__,
Expand Down
87 changes: 87 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,90 @@
# 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

# 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:
# 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="^(?:(?P<exempt>setUp|tearDown|setUpModule|tearDownModule)|(?P<camel_case>_?[A-Z][a-zA-Z0-9]*)|(?P<snake_case>_?[a-z][a-z0-9_]*))$"
method-rgx="(?x)^(?:(?P<exempt>_[a-z0-9_]+__|runTest|setUp|tearDown|setUpTestCase|tearDownTestCase|setupSelf|tearDownClass|setUpClass|(test|assert)_*[A-Z0-9][a-zA-Z0-9_]*|next)|(?P<camel_case>_{0,2}[A-Z][a-zA-Z0-9_]*)|(?P<snake_case>_{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_]*$"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are some regexes in triple-quotes and others not? """rgx""" vs "rgx"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good catch. I copied the regex content from tuf/api/pylintrc and made the mistake of not reading it.
Fixing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in tuf/api/pylintrc there are no quotes at all in these lines. (I actually compared the lines in both files while reviewing and the only difference are the quotes.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaa, I think I wanted to split them multiline but decided against it.
No matter what was the reason it's fixed now.

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"
]
Comment on lines +79 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these make sense here so you can just run mypy... but I wonder if maintenance would be easier if we gave these on the command line in tox.ini like all other tools -- then we could define a lint_dirs variable in there once and use it for all tools (mypy would need a exceptions.py added until we move the file to api/ but that's fine)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally a good idea! But let's hold off this future work until we've removed legacy code, shall we? Maybe we won't need a lint_dirs then.


[[tool.mypy.overrides]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why double brackets?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was suggested by the mypy pyproject.toml configuration file documentation here.
I tried running it without one of the bracket pairs and got the error:

pyproject.toml: tool.mypy.overrides sections must be an array. Please make sure you are using double brackets like so: [[tool.mypy.overrides]]

module = [
"securesystemslib.*",
"urllib3.*"
]
ignore_missing_imports = "True"
20 changes: 0 additions & 20 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 3 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@ 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 --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
black --check --diff tuf/api tuf/ngclient
isort --check --diff tuf/api tuf/ngclient
pylint -j 0 tuf/api tuf/ngclient --rcfile=pyproject.toml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also format and lint the scripts in the newly added examples directory. (see #1685)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add a separate pr for that as I noticed there are a couple of mypy warnings that must be addressed as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, we already have a ticket for that in #1697

Copy link
Collaborator Author

@MVrachev MVrachev Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After we agree that this pr is good, I will submit a pr for #1697.


# NOTE: Contrary to what the pylint docs suggest, ignoring full paths does
# work, unfortunately each subdirectory has to be ignored explicitly.
Expand Down
2 changes: 1 addition & 1 deletion tuf/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 0 additions & 46 deletions tuf/api/pylintrc

This file was deleted.