Skip to content

Commit

Permalink
Merge #20451: lint: run mypy over contrib/devtools
Browse files Browse the repository at this point in the history
1ef2138 lint: run mypy over contrib/devtools (fanquake)

Pull request description:

  wumpus mentioned on IRC that we don't currently run `mypy` over the `contrib/devtools` directory, and that it would likely be worthwhile given #20434. This just adds that dir to the linter, as well as some missing annotations to fix existing errors. Note that now we require Python 3.6 we can make use of variable annotations.

  master (patched to check contrib devtools):
  ```bash
  test/lint/lint-python.sh
  contrib/devtools/symbol-check.py:154: error: Incompatible types in assignment (expression has type "List[str]", variable has type "str")
  contrib/devtools/circular-dependencies.py:35: error: Need type annotation for 'deps' (hint: "deps: Dict[<type>, <type>] = ...")
  contrib/devtools/circular-dependencies.py:67: error: Need type annotation for 'closure' (hint: "closure: Dict[<type>, <type>] = ...")
  Found 4 errors in 3 files (checked 187 source files)
  ```

  I haven't quite gone as far as to add annotations like
  ```python
  CHECKS: Dict[str, List[Tuple[str, Callable[[Any], bool]]]] = {...
  ```
  to `symbol-check.py`.

ACKs for top commit:
  laanwj:
    ACK 1ef2138

Tree-SHA512: a58c2ece588c640289dc1d35dad5b1b8732788272daa0965d6bf44ee8a7f7c8e8585f94d233ac41c84b9ffcfc97841a00fe2c9acba41f58fd164f01de4b6512b
  • Loading branch information
laanwj committed Dec 28, 2020
2 parents ddbf7a6 + 1ef2138 commit 7f9ae87
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
7 changes: 4 additions & 3 deletions contrib/devtools/circular-dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import sys
import re
from typing import Dict, List, Set

MAPPING = {
'core_read.cpp': 'core_io.cpp',
Expand Down Expand Up @@ -32,7 +33,7 @@ def module_name(path):
return None

files = dict()
deps = dict()
deps: Dict[str, Set[str]] = dict()

RE = re.compile("^#include <(.*)>")

Expand All @@ -59,12 +60,12 @@ def module_name(path):
deps[module].add(included_module)

# Loop to find the shortest (remaining) circular dependency
have_cycle = False
have_cycle: bool = False
while True:
shortest_cycle = None
for module in sorted(deps.keys()):
# Build the transitive closure of dependencies of module
closure = dict()
closure: Dict[str, List[str]] = dict()
for dep in deps[module]:
closure[dep] = []
while True:
Expand Down
14 changes: 7 additions & 7 deletions contrib/devtools/symbol-check.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def check_version(max_versions, version, arch) -> bool:
def check_imported_symbols(filename) -> bool:
elf = pixie.load(filename)
cppfilt = CPPFilt()
ok = True
ok: bool = True

for symbol in elf.dyn_symbols:
if not symbol.is_import:
Expand All @@ -172,7 +172,7 @@ def check_imported_symbols(filename) -> bool:
def check_exported_symbols(filename) -> bool:
elf = pixie.load(filename)
cppfilt = CPPFilt()
ok = True
ok: bool = True
for symbol in elf.dyn_symbols:
if not symbol.is_export:
continue
Expand All @@ -184,7 +184,7 @@ def check_exported_symbols(filename) -> bool:
return ok

def check_ELF_libraries(filename) -> bool:
ok = True
ok: bool = True
elf = pixie.load(filename)
for library_name in elf.query_dyn_tags(pixie.DT_NEEDED):
assert(isinstance(library_name, bytes))
Expand All @@ -207,7 +207,7 @@ def macho_read_libraries(filename) -> List[str]:
return libraries

def check_MACHO_libraries(filename) -> bool:
ok = True
ok: bool = True
for dylib in macho_read_libraries(filename):
if dylib not in MACHO_ALLOWED_LIBRARIES:
print('{} is not in ALLOWED_LIBRARIES!'.format(dylib))
Expand All @@ -227,7 +227,7 @@ def pe_read_libraries(filename) -> List[str]:
return libraries

def check_PE_libraries(filename) -> bool:
ok = True
ok: bool = True
for dylib in pe_read_libraries(filename):
if dylib not in PE_ALLOWED_LIBRARIES:
print('{} is not in ALLOWED_LIBRARIES!'.format(dylib))
Expand Down Expand Up @@ -260,7 +260,7 @@ def identify_executable(executable) -> Optional[str]:
return None

if __name__ == '__main__':
retval = 0
retval: int = 0
for filename in sys.argv[1:]:
try:
etype = identify_executable(filename)
Expand All @@ -269,7 +269,7 @@ def identify_executable(executable) -> Optional[str]:
retval = 1
continue

failed = []
failed: List[str] = []
for (name, func) in CHECKS[etype]:
if not func(filename):
failed.append(name)
Expand Down
2 changes: 1 addition & 1 deletion test/lint/lint-python.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ if ! PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; e
EXIT_CODE=1
fi

if ! mypy --ignore-missing-imports $(git ls-files "test/functional/*.py"); then
if ! mypy --ignore-missing-imports $(git ls-files "test/functional/*.py" "contrib/devtools/*.py"); then
EXIT_CODE=1
fi

Expand Down

0 comments on commit 7f9ae87

Please sign in to comment.