From 4ad57ef7d2730c39aeb3e69e6967b0bcbfa9146e Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sun, 4 Oct 2020 22:31:02 -0700 Subject: [PATCH 1/9] add libCST # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- 3rdparty/python/constraints.txt | 8 ++++---- 3rdparty/python/requirements.txt | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/3rdparty/python/constraints.txt b/3rdparty/python/constraints.txt index 497a23e6a6c..f2a11eb4355 100644 --- a/3rdparty/python/constraints.txt +++ b/3rdparty/python/constraints.txt @@ -1,4 +1,4 @@ -# Generated by build-support/bin/generate_lockfile.sh on Mon Sep 28 09:49:00 PDT 2020 +# Generated by build-support/bin/generate_lockfile.sh on Sun Oct 4 22:30:50 PDT 2020 ansicolors==1.1.8 attrs==20.2.0 beautifulsoup4==4.6.3 @@ -17,8 +17,8 @@ mypy==0.782 mypy-extensions==0.4.3 packaging==20.4 pathspec==0.8.0 -pex==2.1.16 -pip==19.0.3 +pex==2.1.17 +pip==18.1 pluggy==0.13.1 psutil==5.7.0 py==1.9.0 @@ -37,4 +37,4 @@ typed-ast==1.4.1 typing-extensions==3.7.4.2 urllib3==1.25.10 www-authenticate==0.9.2 -zipp==3.2.0 +zipp==3.3.0 diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index e0aa9d09854..14a84613ea7 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -2,6 +2,7 @@ ansicolors==1.1.8 beautifulsoup4>=4.6.0,<4.7 dataclasses==0.6 fasteners==0.15.0 +libcst==0.3.12 # The MyPy requirement should be maintained in lockstep with the requirement the Pants repo uses # for the mypy task since it configures custom MyPy plugins. That requirement can be found via: From fd082cbd3cff238ec6d863e528e1c3109b4115f8 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sun, 4 Oct 2020 23:53:05 -0700 Subject: [PATCH 2/9] use libCST to parse python 3 # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/import_parser.py | 138 ++++++++++++------ .../python/dependency_inference/rules.py | 38 ++++- .../pants/backend/python/util_rules/pex.py | 15 +- .../python/util_rules/pex_environment.py | 18 ++- 4 files changed, 159 insertions(+), 50 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index b00e24899cb..5158194ad59 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -1,12 +1,12 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import ast as ast3 import re -import sys from dataclasses import dataclass -from typing import Optional, Set, Tuple +from typing import Any, Optional, Set, Tuple, Type, TypeVar, Union +import libcst as cst +from pex.interpreter import PythonIdentity from typed_ast import ast27 from pants.util.memo import memoized_property @@ -35,37 +35,46 @@ def all_imports(self) -> FrozenOrderedSet[str]: return FrozenOrderedSet(sorted([*self.explicit_imports, *self.inferred_imports])) -def parse_file(*, filename: str, content: str) -> Optional[Tuple]: - try: - # NB: The Python 3 ast is generally backwards-compatible with earlier versions. The only - # breaking change is `async` `await` becoming reserved keywords in Python 3.7 (deprecated - # in 3.6). If the std-lib fails to parse, we could use typed-ast to try parsing with a - # target version of Python 3.5, but we don't because Python 3.5 is almost EOL and has very - # low usage. - # We will also fail to parse Python 3.8 syntax if Pants is run with Python 3.6 or 3.7. - # There is no known workaround for this, beyond users changing their `./pants` script to - # always use >= 3.8. - tree = ast3.parse(content, filename=filename) - visitor_cls = _Py3AstVisitor if sys.version_info[:2] < (3, 8) else _Py38AstVisitor - return tree, visitor_cls - except SyntaxError: - try: - py27_tree = ast27.parse(content, filename=filename) - return py27_tree, _Py27AstVisitor - except SyntaxError: - return None - - -def find_python_imports(*, filename: str, content: str, module_name: str) -> ParsedPythonImports: - parse_result = parse_file(filename=filename, content=content) +def parse_file( + identity: PythonIdentity, + *, + filename: str, + content: str, +) -> Tuple[Any, "_VisitorInterface"]: + major, minor, _patch = identity.version + if major == 2: + py27_tree = ast27.parse(content, filename=filename) + return py27_tree, _Py27AstVisitor + + if major != 3: + raise ValueError( + f"Unrecognized python version: {identity}. Currently only 2.7 " + "and 3.5-3.8 are supported." + ) + + # Parse all python 3 code with libCST. + config = cst.PartialParserConfig(python_version=f"{major}.{minor}") + cst_tree = cst.parse_module(content, config=config) + visitor_cls = _CSTVisitor + return cst_tree, visitor_cls + + +def find_python_imports( + identity: PythonIdentity, + *, + filename: str, + content: str, + module_name: str, +) -> ParsedPythonImports: # If there were syntax errors, gracefully early return. This is more user friendly than # propagating the exception. Dependency inference simply won't be used for that file, and # it'll be up to the tool actually being run (e.g. Pytest or Flake8) to error. - if parse_result is None: + try: + parse_result = parse_file(identity, filename=filename, content=content) + except (SyntaxError, cst.ParserSyntaxError): return ParsedPythonImports(FrozenOrderedSet(), FrozenOrderedSet()) tree, ast_visitor_cls = parse_result - ast_visitor = ast_visitor_cls(module_name) - ast_visitor.visit(tree) + ast_visitor = ast_visitor_cls.visit_tree(tree, module_name) return ParsedPythonImports( explicit_imports=FrozenOrderedSet(sorted(ast_visitor.explicit_imports)), inferred_imports=FrozenOrderedSet(sorted(ast_visitor.inferred_imports)), @@ -77,13 +86,31 @@ def find_python_imports(*, filename: str, content: str, module_name: str) -> Par _INFERRED_IMPORT_REGEX = re.compile(r"^([a-z_][a-z_\d]*\.){2,}[a-zA-Z_]\w*$") -class _BaseAstVisitor: +_Visitor = TypeVar("_Visitor") + + +class _VisitorInterface: + def __init__(self, module_name: str) -> None: + ... + + @classmethod + def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + ... + + +class _Py27AstVisitor(ast27.NodeVisitor, _VisitorInterface): def __init__(self, module_name: str) -> None: self._module_parts = module_name.split(".") self.explicit_imports: Set[str] = set() self.inferred_imports: Set[str] = set() - def maybe_add_inferred_import(self, s: str) -> None: + @classmethod + def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + visitor = cls(module_name) + visitor.visit(tree) + return visitor + + def _maybe_add_inferred_import(self, s: str) -> None: if _INFERRED_IMPORT_REGEX.match(s): self.inferred_imports.add(s) @@ -99,20 +126,47 @@ def visit_ImportFrom(self, node) -> None: for alias in node.names: self.explicit_imports.add(f"{abs_module}.{alias.name}") - -class _Py27AstVisitor(ast27.NodeVisitor, _BaseAstVisitor): def visit_Str(self, node) -> None: val = ensure_text(node.s) - self.maybe_add_inferred_import(val) + self._maybe_add_inferred_import(val) -class _Py3AstVisitor(ast3.NodeVisitor, _BaseAstVisitor): - def visit_Str(self, node) -> None: - self.maybe_add_inferred_import(node.s) +class _CSTVisitor(cst.CSTVisitor, _VisitorInterface): + def __init__(self, module_name: str) -> None: + self._module_parts = module_name.split(".") + self.explicit_imports: Set[str] = set() + self.inferred_imports: Set[str] = set() + + @classmethod + def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + visitor = cls(module_name) + tree.visit(visitor) + return visitor + + def _maybe_add_inferred_import(self, s: str) -> None: + if _INFERRED_IMPORT_REGEX.match(s): + self.inferred_imports.add(s) + + def _flatten_attribute_or_name(self, node: Optional[Union[cst.Attribute, cst.Name]]) -> str: + if node is None: + return "" + if isinstance(node, cst.Name): + return node.value + inner = self._flatten_attribute_or_name(node.value) + return f"{inner}.{node.attr.value}" + def visit_Import(self, node) -> None: + for alias in node.names: + self.explicit_imports.add(self._flatten_attribute_or_name(alias.name)) + + def visit_ImportFrom(self, node) -> None: + rel_module = self._flatten_attribute_or_name(node.module) + abs_module = ".".join( + self._module_parts[0 : -len(node.relative)] + + ([] if rel_module is None else [rel_module]) + ) + for alias in node.names: + self.explicit_imports.add(f"{abs_module}.{alias.name.value}") -class _Py38AstVisitor(ast3.NodeVisitor, _BaseAstVisitor): - # Python 3.8 deprecated the Str node in favor of Constant. - def visit_Constant(self, node) -> None: - if isinstance(node.value, str): - self.maybe_add_inferred_import(node.value) + def visit_SimpleString(self, node) -> None: + self._maybe_add_inferred_import(node.evaluated_value) diff --git a/src/python/pants/backend/python/dependency_inference/rules.py b/src/python/pants/backend/python/dependency_inference/rules.py index 264e26f264a..4d58ccedd80 100644 --- a/src/python/pants/backend/python/dependency_inference/rules.py +++ b/src/python/pants/backend/python/dependency_inference/rules.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import itertools +from dataclasses import dataclass from pathlib import PurePath from typing import List, cast @@ -9,11 +10,18 @@ from pants.backend.python.dependency_inference.import_parser import find_python_imports from pants.backend.python.dependency_inference.module_mapper import PythonModule, PythonModuleOwners from pants.backend.python.dependency_inference.python_stdlib.combined import combined_stdlib -from pants.backend.python.target_types import PythonSources, PythonTestsSources +from pants.backend.python.target_types import ( + PythonInterpreterCompatibility, + PythonSources, + PythonTestsSources, +) from pants.backend.python.util_rules import ancestor_files from pants.backend.python.util_rules.ancestor_files import AncestorFiles, AncestorFilesRequest +from pants.backend.python.util_rules.pex import PexInterpreterConstraints +from pants.backend.python.util_rules.pex_environment import PythonExecutable from pants.core.util_rules.source_files import SourceFilesRequest from pants.core.util_rules.stripped_source_files import StrippedSourceFiles +from pants.engine.addresses import Address from pants.engine.fs import Digest, DigestContents from pants.engine.internals.graph import Owners, OwnersRequest from pants.engine.rules import Get, MultiGet, collect_rules, rule @@ -22,10 +30,12 @@ HydrateSourcesRequest, InferDependenciesRequest, InferredDependencies, + WrappedTarget, ) from pants.engine.unions import UnionRule from pants.option.global_options import OwnersNotFoundBehavior from pants.option.subsystem import Subsystem +from pants.python.python_setup import PythonSetup class PythonInference(Subsystem): @@ -99,9 +109,28 @@ class InferPythonDependencies(InferDependenciesRequest): infer_from = PythonSources +@dataclass(frozen=True) +class InterpreterForPythonTargetRequest: + address: Address + + +@rule +async def interpreter_for_target( + request: InterpreterForPythonTargetRequest, + python_setup: PythonSetup, +) -> PythonExecutable: + wrapped_tgt = await Get(WrappedTarget, Address, request.address) + compatibility = wrapped_tgt.target.get(PythonInterpreterCompatibility) + pex_constraints = PexInterpreterConstraints.create_from_compatibility_fields( + [compatibility], python_setup + ) + return await Get(PythonExecutable, PexInterpreterConstraints, pex_constraints) + + @rule(desc="Inferring Python dependencies.") async def infer_python_dependencies( - request: InferPythonDependencies, python_inference: PythonInference + request: InferPythonDependencies, + python_inference: PythonInference, ) -> InferredDependencies: if not python_inference.imports: return InferredDependencies([], sibling_dependencies_inferrable=False) @@ -113,9 +142,14 @@ async def infer_python_dependencies( ) digest_contents = await Get(DigestContents, Digest, stripped_sources.snapshot.digest) + interpreter_binary = await Get( + PythonExecutable, InterpreterForPythonTargetRequest(request.sources_field.address) + ) + owners_requests: List[Get[PythonModuleOwners, PythonModule]] = [] for file_content, module in zip(digest_contents, modules): file_imports_obj = find_python_imports( + interpreter_binary.identity, filename=file_content.path, content=file_content.content.decode(), module_name=module.module, diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index cfcc1103ee9..a0daddc35db 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -20,6 +20,7 @@ Union, ) +from pex.interpreter import PythonIdentity from pkg_resources import Requirement from typing_extensions import Protocol @@ -47,6 +48,7 @@ ) from pants.engine.platform import Platform, PlatformConstraint from pants.engine.process import ( + BinaryPath, MultiPlatformProcess, Process, ProcessResult, @@ -420,9 +422,14 @@ async def find_interpreter(interpreter_constraints: PexInterpreterConstraints) - dedent( """\ import hashlib + import json import os import sys + from pex.interpreter import PythonIdentity + + print(PythonIdentity.get().encode()) + python = os.path.realpath(sys.executable) print(python) @@ -443,8 +450,12 @@ async def find_interpreter(interpreter_constraints: PexInterpreterConstraints) - result = await Get( ProcessResult, UncacheableProcess(process=process, scope=ProcessScope.PER_SESSION) ) - path, fingerprint = result.stdout.decode().strip().splitlines() - return PythonExecutable(path=path, fingerprint=fingerprint) + encoded_identity, path, fingerprint = result.stdout.decode().strip().splitlines() + identity = PythonIdentity.decode(encoded_identity) + return PythonExecutable( + identity=identity, + binary=BinaryPath(path=path, fingerprint=fingerprint), + ) @rule(level=LogLevel.DEBUG) diff --git a/src/python/pants/backend/python/util_rules/pex_environment.py b/src/python/pants/backend/python/util_rules/pex_environment.py index 4750815996a..305f5f23927 100644 --- a/src/python/pants/backend/python/util_rules/pex_environment.py +++ b/src/python/pants/backend/python/util_rules/pex_environment.py @@ -6,6 +6,8 @@ from textwrap import dedent from typing import Mapping, Optional, Tuple, cast +from pex.interpreter import PythonIdentity + from pants.core.util_rules import subprocess_environment from pants.core.util_rules.subprocess_environment import SubprocessEnvironmentVars from pants.engine import process @@ -90,8 +92,11 @@ def verbosity(self) -> int: return level -class PythonExecutable(BinaryPath, EngineAwareReturnType): +@dataclass(frozen=True) +class PythonExecutable(EngineAwareReturnType): """The BinaryPath of a Python executable.""" + identity: PythonIdentity + binary: BinaryPath def message(self) -> str: return f"Selected {self.path} to run PEXes with." @@ -108,7 +113,7 @@ def create_argv( self, pex_path: str, *args: str, python: Optional[PythonExecutable] = None ) -> Tuple[str, ...]: python = python or self.bootstrap_python - argv = [python.path] if python else [] + argv = [python.binary.path] if python else [] argv.extend((pex_path, *args)) return tuple(argv) @@ -202,8 +207,13 @@ def first_python_binary() -> Optional[PythonExecutable]: for binary_paths in all_python_binary_paths: if binary_paths.first_path: return PythonExecutable( - path=binary_paths.first_path.path, - fingerprint=binary_paths.first_path.fingerprint, + # TODO: Avoid touching the filesystem again here and figure out how to extract + # it from a BinaryPathRequest! + identity=PythonIdentity.get(binary_paths.first_path.path), + binary=BinaryPath( + path=binary_paths.first_path.path, + fingerprint=binary_paths.first_path.fingerprint, + ), ) return None From e08e4384c0cc8562d3d3d0e9cb75f0510c4ac772 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 5 Oct 2020 00:17:23 -0700 Subject: [PATCH 3/9] update lockfile! # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- 3rdparty/python/constraints.txt | 4 +++- .../backend/python/dependency_inference/import_parser.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/3rdparty/python/constraints.txt b/3rdparty/python/constraints.txt index f2a11eb4355..bbe5c4d03bd 100644 --- a/3rdparty/python/constraints.txt +++ b/3rdparty/python/constraints.txt @@ -1,4 +1,4 @@ -# Generated by build-support/bin/generate_lockfile.sh on Sun Oct 4 22:30:50 PDT 2020 +# Generated by build-support/bin/generate_lockfile.sh on Mon Oct 5 00:17:05 PDT 2020 ansicolors==1.1.8 attrs==20.2.0 beautifulsoup4==4.6.3 @@ -11,6 +11,7 @@ fasteners==0.15 idna==2.10 importlib-metadata==2.0.0 iniconfig==1.0.1 +libcst==0.3.12 monotonic==1.5 more-itertools==8.5.0 mypy==0.782 @@ -35,6 +36,7 @@ six==1.15.0 toml==0.10.1 typed-ast==1.4.1 typing-extensions==3.7.4.2 +typing-inspect==0.6.0 urllib3==1.25.10 www-authenticate==0.9.2 zipp==3.3.0 diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index 5158194ad59..663432e9ede 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -143,7 +143,9 @@ def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: tree.visit(visitor) return visitor - def _maybe_add_inferred_import(self, s: str) -> None: + def _maybe_add_inferred_import(self, s: Union[str, bytes]) -> None: + if isinstance(s, bytes): + return if _INFERRED_IMPORT_REGEX.match(s): self.inferred_imports.add(s) From 507a3b4c0612c36953a114f5e68fb778aabb215a Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 5 Oct 2020 18:50:40 -0700 Subject: [PATCH 4/9] use python 3.8 to parse all python 3 code! # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/import_parser.py | 87 +++++++++---------- .../python/dependency_inference/rules.py | 35 +------- .../pants/backend/python/util_rules/pex.py | 15 +--- .../python/util_rules/pex_environment.py | 18 +--- 4 files changed, 47 insertions(+), 108 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index 663432e9ede..de3825700b6 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -3,10 +3,9 @@ import re from dataclasses import dataclass -from typing import Any, Optional, Set, Tuple, Type, TypeVar, Union +from typing import Any, Optional, Set, Type, TypeVar, Union import libcst as cst -from pex.interpreter import PythonIdentity from typed_ast import ast27 from pants.util.memo import memoized_property @@ -35,49 +34,55 @@ def all_imports(self) -> FrozenOrderedSet[str]: return FrozenOrderedSet(sorted([*self.explicit_imports, *self.inferred_imports])) -def parse_file( - identity: PythonIdentity, - *, - filename: str, - content: str, -) -> Tuple[Any, "_VisitorInterface"]: - major, minor, _patch = identity.version - if major == 2: - py27_tree = ast27.parse(content, filename=filename) - return py27_tree, _Py27AstVisitor +_Visitor = TypeVar("_Visitor") - if major != 3: - raise ValueError( - f"Unrecognized python version: {identity}. Currently only 2.7 " - "and 3.5-3.8 are supported." - ) + +class VisitorInterface: + explicit_imports: Set[str] + inferred_imports: Set[str] + + def __init__(self, module_name: str) -> None: + ... + + @classmethod + def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + ... + + +def parse_file(*, filename: str, content: str, module_name: str) -> VisitorInterface: + """Parse the file for python imports, and return a visitor with the imports it found.""" + try: + py27_tree = ast27.parse(content, filename=filename) + completed_visitor = _Py27AstVisitor.visit_tree(py27_tree, module_name=module_name) + return completed_visitor + except SyntaxError: + # NB: When the python 2 ast visitor fails to parse python 3 syntax, it raises a + # SyntaxError. This may also occur when the file contains invalid python code. If we parse a + # python 3 file with a python 2 parser, that should not change the imports we calculate. + pass # Parse all python 3 code with libCST. - config = cst.PartialParserConfig(python_version=f"{major}.{minor}") + # NB: Since all python 3 code is forwards-compatible with the 3.8 parser, and the import syntax + # remains unchanged, we are safely able to use the 3.8 parser for parsing imports. + config = cst.PartialParserConfig(python_version="3.8") cst_tree = cst.parse_module(content, config=config) - visitor_cls = _CSTVisitor - return cst_tree, visitor_cls + # NB: If the file contains invalid python code, it will raise an exception we expect to be + # caught. + completed_visitor = _CSTVisitor.visit_tree(cst_tree, module_name=module_name) + return completed_visitor -def find_python_imports( - identity: PythonIdentity, - *, - filename: str, - content: str, - module_name: str, -) -> ParsedPythonImports: +def find_python_imports(*, filename: str, content: str, module_name: str) -> ParsedPythonImports: # If there were syntax errors, gracefully early return. This is more user friendly than # propagating the exception. Dependency inference simply won't be used for that file, and # it'll be up to the tool actually being run (e.g. Pytest or Flake8) to error. try: - parse_result = parse_file(identity, filename=filename, content=content) - except (SyntaxError, cst.ParserSyntaxError): + completed_visitor = parse_file(filename=filename, content=content, module_name=module_name) + except cst.ParserSyntaxError: return ParsedPythonImports(FrozenOrderedSet(), FrozenOrderedSet()) - tree, ast_visitor_cls = parse_result - ast_visitor = ast_visitor_cls.visit_tree(tree, module_name) return ParsedPythonImports( - explicit_imports=FrozenOrderedSet(sorted(ast_visitor.explicit_imports)), - inferred_imports=FrozenOrderedSet(sorted(ast_visitor.inferred_imports)), + explicit_imports=FrozenOrderedSet(sorted(completed_visitor.explicit_imports)), + inferred_imports=FrozenOrderedSet(sorted(completed_visitor.inferred_imports)), ) @@ -86,19 +91,7 @@ def find_python_imports( _INFERRED_IMPORT_REGEX = re.compile(r"^([a-z_][a-z_\d]*\.){2,}[a-zA-Z_]\w*$") -_Visitor = TypeVar("_Visitor") - - -class _VisitorInterface: - def __init__(self, module_name: str) -> None: - ... - - @classmethod - def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: - ... - - -class _Py27AstVisitor(ast27.NodeVisitor, _VisitorInterface): +class _Py27AstVisitor(ast27.NodeVisitor, VisitorInterface): def __init__(self, module_name: str) -> None: self._module_parts = module_name.split(".") self.explicit_imports: Set[str] = set() @@ -131,7 +124,7 @@ def visit_Str(self, node) -> None: self._maybe_add_inferred_import(val) -class _CSTVisitor(cst.CSTVisitor, _VisitorInterface): +class _CSTVisitor(cst.CSTVisitor, VisitorInterface): def __init__(self, module_name: str) -> None: self._module_parts = module_name.split(".") self.explicit_imports: Set[str] = set() diff --git a/src/python/pants/backend/python/dependency_inference/rules.py b/src/python/pants/backend/python/dependency_inference/rules.py index 4d58ccedd80..c2eac5a4c97 100644 --- a/src/python/pants/backend/python/dependency_inference/rules.py +++ b/src/python/pants/backend/python/dependency_inference/rules.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import itertools -from dataclasses import dataclass from pathlib import PurePath from typing import List, cast @@ -10,18 +9,11 @@ from pants.backend.python.dependency_inference.import_parser import find_python_imports from pants.backend.python.dependency_inference.module_mapper import PythonModule, PythonModuleOwners from pants.backend.python.dependency_inference.python_stdlib.combined import combined_stdlib -from pants.backend.python.target_types import ( - PythonInterpreterCompatibility, - PythonSources, - PythonTestsSources, -) +from pants.backend.python.target_types import PythonSources, PythonTestsSources from pants.backend.python.util_rules import ancestor_files from pants.backend.python.util_rules.ancestor_files import AncestorFiles, AncestorFilesRequest -from pants.backend.python.util_rules.pex import PexInterpreterConstraints -from pants.backend.python.util_rules.pex_environment import PythonExecutable from pants.core.util_rules.source_files import SourceFilesRequest from pants.core.util_rules.stripped_source_files import StrippedSourceFiles -from pants.engine.addresses import Address from pants.engine.fs import Digest, DigestContents from pants.engine.internals.graph import Owners, OwnersRequest from pants.engine.rules import Get, MultiGet, collect_rules, rule @@ -30,12 +22,10 @@ HydrateSourcesRequest, InferDependenciesRequest, InferredDependencies, - WrappedTarget, ) from pants.engine.unions import UnionRule from pants.option.global_options import OwnersNotFoundBehavior from pants.option.subsystem import Subsystem -from pants.python.python_setup import PythonSetup class PythonInference(Subsystem): @@ -109,24 +99,6 @@ class InferPythonDependencies(InferDependenciesRequest): infer_from = PythonSources -@dataclass(frozen=True) -class InterpreterForPythonTargetRequest: - address: Address - - -@rule -async def interpreter_for_target( - request: InterpreterForPythonTargetRequest, - python_setup: PythonSetup, -) -> PythonExecutable: - wrapped_tgt = await Get(WrappedTarget, Address, request.address) - compatibility = wrapped_tgt.target.get(PythonInterpreterCompatibility) - pex_constraints = PexInterpreterConstraints.create_from_compatibility_fields( - [compatibility], python_setup - ) - return await Get(PythonExecutable, PexInterpreterConstraints, pex_constraints) - - @rule(desc="Inferring Python dependencies.") async def infer_python_dependencies( request: InferPythonDependencies, @@ -142,14 +114,9 @@ async def infer_python_dependencies( ) digest_contents = await Get(DigestContents, Digest, stripped_sources.snapshot.digest) - interpreter_binary = await Get( - PythonExecutable, InterpreterForPythonTargetRequest(request.sources_field.address) - ) - owners_requests: List[Get[PythonModuleOwners, PythonModule]] = [] for file_content, module in zip(digest_contents, modules): file_imports_obj = find_python_imports( - interpreter_binary.identity, filename=file_content.path, content=file_content.content.decode(), module_name=module.module, diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index a0daddc35db..cfcc1103ee9 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -20,7 +20,6 @@ Union, ) -from pex.interpreter import PythonIdentity from pkg_resources import Requirement from typing_extensions import Protocol @@ -48,7 +47,6 @@ ) from pants.engine.platform import Platform, PlatformConstraint from pants.engine.process import ( - BinaryPath, MultiPlatformProcess, Process, ProcessResult, @@ -422,14 +420,9 @@ async def find_interpreter(interpreter_constraints: PexInterpreterConstraints) - dedent( """\ import hashlib - import json import os import sys - from pex.interpreter import PythonIdentity - - print(PythonIdentity.get().encode()) - python = os.path.realpath(sys.executable) print(python) @@ -450,12 +443,8 @@ async def find_interpreter(interpreter_constraints: PexInterpreterConstraints) - result = await Get( ProcessResult, UncacheableProcess(process=process, scope=ProcessScope.PER_SESSION) ) - encoded_identity, path, fingerprint = result.stdout.decode().strip().splitlines() - identity = PythonIdentity.decode(encoded_identity) - return PythonExecutable( - identity=identity, - binary=BinaryPath(path=path, fingerprint=fingerprint), - ) + path, fingerprint = result.stdout.decode().strip().splitlines() + return PythonExecutable(path=path, fingerprint=fingerprint) @rule(level=LogLevel.DEBUG) diff --git a/src/python/pants/backend/python/util_rules/pex_environment.py b/src/python/pants/backend/python/util_rules/pex_environment.py index 305f5f23927..4750815996a 100644 --- a/src/python/pants/backend/python/util_rules/pex_environment.py +++ b/src/python/pants/backend/python/util_rules/pex_environment.py @@ -6,8 +6,6 @@ from textwrap import dedent from typing import Mapping, Optional, Tuple, cast -from pex.interpreter import PythonIdentity - from pants.core.util_rules import subprocess_environment from pants.core.util_rules.subprocess_environment import SubprocessEnvironmentVars from pants.engine import process @@ -92,11 +90,8 @@ def verbosity(self) -> int: return level -@dataclass(frozen=True) -class PythonExecutable(EngineAwareReturnType): +class PythonExecutable(BinaryPath, EngineAwareReturnType): """The BinaryPath of a Python executable.""" - identity: PythonIdentity - binary: BinaryPath def message(self) -> str: return f"Selected {self.path} to run PEXes with." @@ -113,7 +108,7 @@ def create_argv( self, pex_path: str, *args: str, python: Optional[PythonExecutable] = None ) -> Tuple[str, ...]: python = python or self.bootstrap_python - argv = [python.binary.path] if python else [] + argv = [python.path] if python else [] argv.extend((pex_path, *args)) return tuple(argv) @@ -207,13 +202,8 @@ def first_python_binary() -> Optional[PythonExecutable]: for binary_paths in all_python_binary_paths: if binary_paths.first_path: return PythonExecutable( - # TODO: Avoid touching the filesystem again here and figure out how to extract - # it from a BinaryPathRequest! - identity=PythonIdentity.get(binary_paths.first_path.path), - binary=BinaryPath( - path=binary_paths.first_path.path, - fingerprint=binary_paths.first_path.fingerprint, - ), + path=binary_paths.first_path.path, + fingerprint=binary_paths.first_path.fingerprint, ) return None From e58a4b184244f6ec7067a41821336ccc11ccccf4 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 5 Oct 2020 18:54:02 -0700 Subject: [PATCH 5/9] remove test skip (!!!!!!!!) # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../backend/python/dependency_inference/import_parser_test.py | 4 ---- src/python/pants/backend/python/dependency_inference/rules.py | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser_test.py b/src/python/pants/backend/python/dependency_inference/import_parser_test.py index 25ab75802af..f1c91bcee40 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser_test.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser_test.py @@ -146,10 +146,6 @@ def test_works_with_python2() -> None: assert set(imports.inferred_imports) == {"dep.from.bytes", "dep.from.str"} -@pytest.mark.skipif( - sys.version_info[:2] < (3, 8), - reason="Cannot parse Python 3.8 unless Pants is run with Python 3.8.", -) def test_works_with_python38() -> None: imports = find_python_imports( filename="foo.py", diff --git a/src/python/pants/backend/python/dependency_inference/rules.py b/src/python/pants/backend/python/dependency_inference/rules.py index c2eac5a4c97..264e26f264a 100644 --- a/src/python/pants/backend/python/dependency_inference/rules.py +++ b/src/python/pants/backend/python/dependency_inference/rules.py @@ -101,8 +101,7 @@ class InferPythonDependencies(InferDependenciesRequest): @rule(desc="Inferring Python dependencies.") async def infer_python_dependencies( - request: InferPythonDependencies, - python_inference: PythonInference, + request: InferPythonDependencies, python_inference: PythonInference ) -> InferredDependencies: if not python_inference.imports: return InferredDependencies([], sibling_dependencies_inferrable=False) From 7ed7fa46e604d65b583288c0aebb2e3249b4e68e Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 7 Oct 2020 11:58:04 -0700 Subject: [PATCH 6/9] parse python 3 code first! and add 2 TODOs # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/import_parser.py | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index de3825700b6..81c7fae19e7 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -1,6 +1,7 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging import re from dataclasses import dataclass from typing import Any, Optional, Set, Type, TypeVar, Union @@ -13,8 +14,7 @@ from pants.util.strutil import ensure_text -class ImportParseError(ValueError): - pass +logger = logging.getLogger(__name__) @dataclass(frozen=True) @@ -49,36 +49,42 @@ def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: ... -def parse_file(*, filename: str, content: str, module_name: str) -> VisitorInterface: +def parse_file(*, filename: str, content: str, module_name: str) -> Optional[VisitorInterface]: """Parse the file for python imports, and return a visitor with the imports it found.""" + # Parse all python 3 code with libCST. We parse assuming python 3 goes first, because we assume + # most user code will be python 3. + # TODO(#10921): identify the appropriate interpreter version beforehand! + try: + # NB: Since all python 3 code is forwards-compatible with the 3.8 parser, and the import + # syntax remains unchanged, we are safely able to use the 3.8 parser for parsing imports. + # TODO(#10922): Support parsing python 3.9/3.10 with libCST! + config = cst.PartialParserConfig(python_version="3.8") + cst_tree = cst.parse_module(content, config=config) + completed_visitor = _CSTVisitor.visit_tree(cst_tree, module_name=module_name) + return completed_visitor + except cst.ParserSyntaxError as e: + # NB: When the python 3 ast visitor fails to parse python 2 syntax, it raises a + # ParserSyntaxError. This may also occur when the file contains invalid python code. If we + # successfully parse a python 2 file with a python 3 parser, that should not change the + # imports we calculate. + logger.debug(f"Failed to parse {filename} with python 3.8 libCST parser: {e}") + try: py27_tree = ast27.parse(content, filename=filename) completed_visitor = _Py27AstVisitor.visit_tree(py27_tree, module_name=module_name) return completed_visitor - except SyntaxError: - # NB: When the python 2 ast visitor fails to parse python 3 syntax, it raises a - # SyntaxError. This may also occur when the file contains invalid python code. If we parse a - # python 3 file with a python 2 parser, that should not change the imports we calculate. - pass - - # Parse all python 3 code with libCST. - # NB: Since all python 3 code is forwards-compatible with the 3.8 parser, and the import syntax - # remains unchanged, we are safely able to use the 3.8 parser for parsing imports. - config = cst.PartialParserConfig(python_version="3.8") - cst_tree = cst.parse_module(content, config=config) - # NB: If the file contains invalid python code, it will raise an exception we expect to be - # caught. - completed_visitor = _CSTVisitor.visit_tree(cst_tree, module_name=module_name) - return completed_visitor + except SyntaxError as e: + logger.debug(f"Failed to parse {filename} with python 2.7 typed-ast parser: {e}") + + return None def find_python_imports(*, filename: str, content: str, module_name: str) -> ParsedPythonImports: + completed_visitor = parse_file(filename=filename, content=content, module_name=module_name) # If there were syntax errors, gracefully early return. This is more user friendly than # propagating the exception. Dependency inference simply won't be used for that file, and # it'll be up to the tool actually being run (e.g. Pytest or Flake8) to error. - try: - completed_visitor = parse_file(filename=filename, content=content, module_name=module_name) - except cst.ParserSyntaxError: + if completed_visitor is None: return ParsedPythonImports(FrozenOrderedSet(), FrozenOrderedSet()) return ParsedPythonImports( explicit_imports=FrozenOrderedSet(sorted(completed_visitor.explicit_imports)), From f0a9a99c7ae466a41930894b569d4c5b8994aaa4 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 7 Oct 2020 12:04:02 -0700 Subject: [PATCH 7/9] make libCST requirement float a bit! # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- 3rdparty/python/constraints.txt | 2 +- 3rdparty/python/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/3rdparty/python/constraints.txt b/3rdparty/python/constraints.txt index bbe5c4d03bd..fbd0d3542ce 100644 --- a/3rdparty/python/constraints.txt +++ b/3rdparty/python/constraints.txt @@ -1,4 +1,4 @@ -# Generated by build-support/bin/generate_lockfile.sh on Mon Oct 5 00:17:05 PDT 2020 +# Generated by build-support/bin/generate_lockfile.sh on Wed Oct 7 12:00:29 PDT 2020 ansicolors==1.1.8 attrs==20.2.0 beautifulsoup4==4.6.3 diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 14a84613ea7..8252dc58ee0 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -2,7 +2,7 @@ ansicolors==1.1.8 beautifulsoup4>=4.6.0,<4.7 dataclasses==0.6 fasteners==0.15.0 -libcst==0.3.12 +libcst>=0.3.12,<0.4 # The MyPy requirement should be maintained in lockstep with the requirement the Pants repo uses # for the mypy task since it configures custom MyPy plugins. That requirement can be found via: From 7dab459ac192caea52c2ebd108accb58b9d3e887 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 7 Oct 2020 12:37:17 -0700 Subject: [PATCH 8/9] make typechecking work # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/import_parser.py | 74 ++++++++----------- .../import_parser_test.py | 3 - 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index 81c7fae19e7..d76e3b4a2af 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -4,16 +4,16 @@ import logging import re from dataclasses import dataclass -from typing import Any, Optional, Set, Type, TypeVar, Union +from typing import Optional, Set, Type, TypeVar, Union import libcst as cst from typed_ast import ast27 +from typing_extensions import Protocol from pants.util.memo import memoized_property from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import ensure_text - logger = logging.getLogger(__name__) @@ -34,20 +34,10 @@ def all_imports(self) -> FrozenOrderedSet[str]: return FrozenOrderedSet(sorted([*self.explicit_imports, *self.inferred_imports])) -_Visitor = TypeVar("_Visitor") - - -class VisitorInterface: +class VisitorInterface(Protocol): explicit_imports: Set[str] inferred_imports: Set[str] - def __init__(self, module_name: str) -> None: - ... - - @classmethod - def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: - ... - def parse_file(*, filename: str, content: str, module_name: str) -> Optional[VisitorInterface]: """Parse the file for python imports, and return a visitor with the imports it found.""" @@ -60,8 +50,8 @@ def parse_file(*, filename: str, content: str, module_name: str) -> Optional[Vis # TODO(#10922): Support parsing python 3.9/3.10 with libCST! config = cst.PartialParserConfig(python_version="3.8") cst_tree = cst.parse_module(content, config=config) - completed_visitor = _CSTVisitor.visit_tree(cst_tree, module_name=module_name) - return completed_visitor + completed_cst_visitor = _CSTVisitor.visit_tree(cst_tree, module_name=module_name) + return completed_cst_visitor except cst.ParserSyntaxError as e: # NB: When the python 3 ast visitor fails to parse python 2 syntax, it raises a # ParserSyntaxError. This may also occur when the file contains invalid python code. If we @@ -71,8 +61,8 @@ def parse_file(*, filename: str, content: str, module_name: str) -> Optional[Vis try: py27_tree = ast27.parse(content, filename=filename) - completed_visitor = _Py27AstVisitor.visit_tree(py27_tree, module_name=module_name) - return completed_visitor + completed_ast27_visitor = _Py27AstVisitor.visit_tree(py27_tree, module_name=module_name) + return completed_ast27_visitor except SyntaxError as e: logger.debug(f"Failed to parse {filename} with python 2.7 typed-ast parser: {e}") @@ -97,14 +87,14 @@ def find_python_imports(*, filename: str, content: str, module_name: str) -> Par _INFERRED_IMPORT_REGEX = re.compile(r"^([a-z_][a-z_\d]*\.){2,}[a-zA-Z_]\w*$") -class _Py27AstVisitor(ast27.NodeVisitor, VisitorInterface): +class _Py27AstVisitor(ast27.NodeVisitor): def __init__(self, module_name: str) -> None: self._module_parts = module_name.split(".") self.explicit_imports: Set[str] = set() self.inferred_imports: Set[str] = set() @classmethod - def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + def visit_tree(cls, tree: ast27.AST, module_name: str) -> "_Py27AstVisitor": visitor = cls(module_name) visitor.visit(tree) return visitor @@ -113,31 +103,30 @@ def _maybe_add_inferred_import(self, s: str) -> None: if _INFERRED_IMPORT_REGEX.match(s): self.inferred_imports.add(s) - def visit_Import(self, node) -> None: + def visit_Import(self, node: ast27.Import) -> None: for alias in node.names: self.explicit_imports.add(alias.name) - def visit_ImportFrom(self, node) -> None: - rel_module = node.module - abs_module = ".".join( - self._module_parts[0 : -node.level] + ([] if rel_module is None else [rel_module]) - ) + def visit_ImportFrom(self, node: ast27.ImportFrom) -> None: + rel_module = [] if node.module is None else [node.module] + relative_level = 0 if node.level is None else node.level + abs_module = ".".join(self._module_parts[0:-relative_level] + rel_module) for alias in node.names: self.explicit_imports.add(f"{abs_module}.{alias.name}") - def visit_Str(self, node) -> None: + def visit_Str(self, node: ast27.Str) -> None: val = ensure_text(node.s) self._maybe_add_inferred_import(val) -class _CSTVisitor(cst.CSTVisitor, VisitorInterface): +class _CSTVisitor(cst.CSTVisitor): def __init__(self, module_name: str) -> None: self._module_parts = module_name.split(".") self.explicit_imports: Set[str] = set() self.inferred_imports: Set[str] = set() @classmethod - def visit_tree(cls: Type[_Visitor], tree: Any, module_name: str) -> _Visitor: + def visit_tree(cls, tree: cst.Module, module_name: str) -> "_CSTVisitor": visitor = cls(module_name) tree.visit(visitor) return visitor @@ -148,26 +137,27 @@ def _maybe_add_inferred_import(self, s: Union[str, bytes]) -> None: if _INFERRED_IMPORT_REGEX.match(s): self.inferred_imports.add(s) - def _flatten_attribute_or_name(self, node: Optional[Union[cst.Attribute, cst.Name]]) -> str: - if node is None: - return "" + def _flatten_attribute_or_name(self, node: cst.BaseExpression) -> str: if isinstance(node, cst.Name): return node.value + if not isinstance(node, cst.Attribute): + raise TypeError(f"Unrecognized cst.BaseExpression subclass: {node}") inner = self._flatten_attribute_or_name(node.value) return f"{inner}.{node.attr.value}" - def visit_Import(self, node) -> None: + def visit_Import(self, node: cst.Import) -> None: for alias in node.names: self.explicit_imports.add(self._flatten_attribute_or_name(alias.name)) - def visit_ImportFrom(self, node) -> None: - rel_module = self._flatten_attribute_or_name(node.module) - abs_module = ".".join( - self._module_parts[0 : -len(node.relative)] - + ([] if rel_module is None else [rel_module]) - ) - for alias in node.names: - self.explicit_imports.add(f"{abs_module}.{alias.name.value}") - - def visit_SimpleString(self, node) -> None: + def visit_ImportFrom(self, node: cst.ImportFrom) -> None: + rel_module = [] if node.module is None else [self._flatten_attribute_or_name(node.module)] + relative_level = len(node.relative) + abs_module = ".".join(self._module_parts[0:-relative_level] + rel_module) + if isinstance(node.names, cst.ImportStar): + self.explicit_imports.add(f"{abs_module}.*") + else: + for alias in node.names: + self.explicit_imports.add(f"{abs_module}.{alias.name.value}") + + def visit_SimpleString(self, node: cst.SimpleString) -> None: self._maybe_add_inferred_import(node.evaluated_value) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser_test.py b/src/python/pants/backend/python/dependency_inference/import_parser_test.py index f1c91bcee40..617e6698fe0 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser_test.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser_test.py @@ -1,11 +1,8 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import sys from textwrap import dedent -import pytest - from pants.backend.python.dependency_inference.import_parser import find_python_imports From d7de2797f542480a6213b62cb0a071ac68799c62 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 7 Oct 2020 13:11:44 -0700 Subject: [PATCH 9/9] test debug log! # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/import_parser.py | 8 +++- .../import_parser_test.py | 42 ++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/import_parser.py b/src/python/pants/backend/python/dependency_inference/import_parser.py index d76e3b4a2af..dd303c149aa 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser.py @@ -4,7 +4,7 @@ import logging import re from dataclasses import dataclass -from typing import Optional, Set, Type, TypeVar, Union +from typing import Optional, Set, Union import libcst as cst from typed_ast import ast27 @@ -33,6 +33,10 @@ class ParsedPythonImports: def all_imports(self) -> FrozenOrderedSet[str]: return FrozenOrderedSet(sorted([*self.explicit_imports, *self.inferred_imports])) + @classmethod + def empty(cls) -> "ParsedPythonImports": + return cls(FrozenOrderedSet(), FrozenOrderedSet()) + class VisitorInterface(Protocol): explicit_imports: Set[str] @@ -75,7 +79,7 @@ def find_python_imports(*, filename: str, content: str, module_name: str) -> Par # propagating the exception. Dependency inference simply won't be used for that file, and # it'll be up to the tool actually being run (e.g. Pytest or Flake8) to error. if completed_visitor is None: - return ParsedPythonImports(FrozenOrderedSet(), FrozenOrderedSet()) + return ParsedPythonImports.empty() return ParsedPythonImports( explicit_imports=FrozenOrderedSet(sorted(completed_visitor.explicit_imports)), inferred_imports=FrozenOrderedSet(sorted(completed_visitor.inferred_imports)), diff --git a/src/python/pants/backend/python/dependency_inference/import_parser_test.py b/src/python/pants/backend/python/dependency_inference/import_parser_test.py index 617e6698fe0..87113176e6a 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser_test.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser_test.py @@ -1,9 +1,13 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging from textwrap import dedent -from pants.backend.python.dependency_inference.import_parser import find_python_imports +from pants.backend.python.dependency_inference.import_parser import ( + ParsedPythonImports, + find_python_imports, +) def test_normal_imports() -> None: @@ -162,3 +166,39 @@ def test_works_with_python38() -> None: ) assert set(imports.explicit_imports) == {"demo", "project.demo.Demo"} assert set(imports.inferred_imports) == {"dep.from.str"} + + +def test_debug_log_with_python39(caplog) -> None: + """We can't parse python 3.9 yet, so we want to be able to provide good debug log.""" + caplog.set_level(logging.DEBUG) + imports = find_python_imports( + filename="foo.py", + # See https://www.python.org/dev/peps/pep-0614/ for the newly relaxed decorator expressions. + content=dedent( + """\ + @buttons[0].clicked.connect + def spam(): + ... + """ + ), + module_name="project.app", + ) + assert imports == ParsedPythonImports.empty() + assert len(caplog.records) == 2 + messages = [rec.getMessage() for rec in caplog.records] + + cst_parse_error = dedent( + """\ + Failed to parse foo.py with python 3.8 libCST parser: Syntax Error @ 1:9. + Incomplete input. Encountered '[', but expected '(', or 'NEWLINE'. + + @buttons[0].clicked.connect + ^""" + ) + assert cst_parse_error == messages[0] + + ast27_parse_error = dedent( + """\ + Failed to parse foo.py with python 2.7 typed-ast parser: invalid syntax (foo.py, line 1)""" + ) + assert ast27_parse_error == messages[1]