Skip to content

Commit

Permalink
Set up type_checked tag for internal type hint migration (#7886)
Browse files Browse the repository at this point in the history
### Problem
Per #6742, we will be _incrementally_ adding type hints to Pants.

We need some way to mark to our CI that the target should be checked against, to make sure that any gains we make in type hints are actually enforced.

### Solution
Introduce a new tag `type_checked`, which indicates to our CI that `./pants mypy` should be ran against that target.

Also introduce a helper script `build-support/bin/mypy.py` that runs `./pants mypy` over _all_ eligible targets in Pants. To run over a select few targets, users should instead simply run `./pants mypy path/to:target`.

#### Alternative solution considered: centralized whitelist file
It was decided that a decentralized system will work better in this particular case, for some of these reasons:

1) There's a convention for keeping it decentralized: the `integration` tag.
2) `BUILD` files are more accessible to modify, e.g. being closer to the source.
3) We want to type check the entire codebase eventually. One centralized file would risk becoming too large and difficult to maintain.
  • Loading branch information
Eric-Arellano committed Jun 17, 2019
1 parent 1048ebc commit c488456
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
13 changes: 13 additions & 0 deletions build-support/bin/BUILD
Expand Up @@ -5,6 +5,7 @@ python_library(
name = 'common',
sources = 'common.py',
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
Expand All @@ -14,6 +15,7 @@ python_binary(
':common',
],
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
Expand All @@ -23,6 +25,7 @@ python_binary(
':common',
],
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
Expand All @@ -32,6 +35,7 @@ python_binary(
':common',
],
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
Expand All @@ -41,6 +45,14 @@ python_binary(
':common',
],
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
name = 'mypy',
sources = 'mypy.py',
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)

python_binary(
Expand All @@ -50,4 +62,5 @@ python_binary(
':common',
],
compatibility = ['CPython>=3.6'],
tags = {'type_checked'},
)
13 changes: 13 additions & 0 deletions build-support/bin/mypy.py
@@ -0,0 +1,13 @@
#!/usr/bin/env python3
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import subprocess


def main() -> None:
subprocess.run(["./pants", "--tag=+type_checked", "mypy", "::"], check=True)


if __name__ == '__main__':
main()
4 changes: 2 additions & 2 deletions build-support/githooks/pre-commit
Expand Up @@ -62,11 +62,11 @@ if git rev-parse --verify "${MERGE_BASE}" &>/dev/null; then
echo "* Checking lint"
./pants --exclude-target-regexp='testprojects/.*' --changed-parent="${MERGE_BASE}" lint || exit 1

echo "* Checking types (for build-support)"
echo "* Checking types for targets marked \`type_checked\`"
# NB: This task requires Python 3, so we must temporarily override any intepreter constraints
# set by ci.py, specifically it setting constraints to Python 2, because those constraints may
# cause a select-interpreter failure.
PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']" ./pants mypy 'build-support::' || exit 1
PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']" ./build-support/bin/mypy.py || exit 1

if git diff "${MERGE_BASE}" --name-only | grep '\.rs$' > /dev/null; then
echo "* Checking formatting of rust files"
Expand Down

0 comments on commit c488456

Please sign in to comment.