diff --git a/src/python/pants/backend/javascript/dependency_inference/rules.py b/src/python/pants/backend/javascript/dependency_inference/rules.py index ee484f60f82..4a220c69957 100644 --- a/src/python/pants/backend/javascript/dependency_inference/rules.py +++ b/src/python/pants/backend/javascript/dependency_inference/rules.py @@ -157,8 +157,7 @@ async def _prepare_inference_metadata(address: Address, file_path: str) -> Infer ) -def _add_extensions(file_imports: frozenset[str]) -> PathGlobs: - file_extensions = (*JS_FILE_EXTENSIONS, *JSX_FILE_EXTENSIONS) +def _add_extensions(file_imports: frozenset[str], file_extensions: tuple[str, ...]) -> PathGlobs: extensions = file_extensions + tuple(f"/index{ext}" for ext in file_extensions) return PathGlobs( string @@ -174,8 +173,14 @@ def _add_extensions(file_imports: frozenset[str]) -> PathGlobs: async def _determine_import_from_candidates( candidates: ParsedJavascriptDependencyCandidate, package_candidate_map: NodePackageCandidateMap, + file_extensions: tuple[str, ...], ) -> Addresses: - paths = await path_globs_to_paths(_add_extensions(candidates.file_imports)) + paths = await path_globs_to_paths( + _add_extensions( + candidates.file_imports, + file_extensions, + ) + ) local_owners = await Get(Owners, OwnersRequest(paths.files)) owning_targets = await Get(Targets, Addresses(local_owners)) @@ -250,7 +255,11 @@ async def infer_js_source_dependencies( zip( import_strings.imports, await concurrently( - _determine_import_from_candidates(candidates, candidate_pkgs) + _determine_import_from_candidates( + candidates, + candidate_pkgs, + file_extensions=JS_FILE_EXTENSIONS + JSX_FILE_EXTENSIONS, + ) for string, candidates in import_strings.imports.items() ), ) diff --git a/src/python/pants/backend/typescript/dependency_inference/rules.py b/src/python/pants/backend/typescript/dependency_inference/rules.py index 2fa24890030..5e5aaa9ef53 100644 --- a/src/python/pants/backend/typescript/dependency_inference/rules.py +++ b/src/python/pants/backend/typescript/dependency_inference/rules.py @@ -10,8 +10,11 @@ from pants.backend.javascript import package_json from pants.backend.javascript.dependency_inference.rules import ( InferNodePackageDependenciesRequest, - NodePackageCandidateMap, RequestNodePackagesCandidateMap, + _determine_import_from_candidates, + _handle_unowned_imports, + _is_node_builtin_module, + map_candidate_node_packages, ) from pants.backend.javascript.package_json import ( OwningNodePackageRequest, @@ -21,6 +24,7 @@ subpath_imports_for_source, ) from pants.backend.javascript.subsystems.nodejs_infer import NodeJSInfer +from pants.backend.javascript.target_types import JS_FILE_EXTENSIONS from pants.backend.typescript import tsconfig from pants.backend.typescript.target_types import ( TS_FILE_EXTENSIONS, @@ -29,11 +33,9 @@ ) from pants.backend.typescript.tsconfig import ParentTSConfigRequest, TSConfig, find_parent_ts_config from pants.build_graph.address import Address -from pants.engine.addresses import Addresses -from pants.engine.internals.graph import Owners, OwnersRequest -from pants.engine.internals.native_dep_inference import NativeParsedJavascriptDependencies from pants.engine.internals.native_engine import InferenceMetadata, NativeDependenciesRequest -from pants.engine.internals.selectors import Get, MultiGet, concurrently +from pants.engine.internals.selectors import Get, concurrently +from pants.engine.intrinsics import parse_javascript_deps from pants.engine.rules import Rule, collect_rules, implicitly, rule from pants.engine.target import ( FieldSet, @@ -41,10 +43,8 @@ HydrateSourcesRequest, InferDependenciesRequest, InferredDependencies, - Targets, ) from pants.engine.unions import UnionRule -from pants.util.ordered_set import FrozenOrderedSet @dataclass(frozen=True) @@ -100,21 +100,6 @@ async def _prepare_inference_metadata(address: Address, file_path: str) -> Infer ) -@rule -async def get_file_imports_targets(import_path: TypeScriptFileImportPath) -> Targets: - """Get address to build targets representing a file import discovered with dependency inference. - - For now, we only support .ts files; in the future, may need to iterate through all possible file - extensions until find one matching, if any. - """ - package, filename = os.path.dirname(import_path.path), os.path.basename(import_path.path) - address = Address(package, relative_file_path=f"{filename}{TS_FILE_EXTENSIONS[0]}") - _ = await Get(Targets, Addresses([address])) - owners = await Get(Owners, OwnersRequest((str(address),))) - owning_targets = await Get(Targets, Addresses(owners)) - return owning_targets - - @rule async def infer_typescript_source_dependencies( request: InferTypeScriptDependenciesRequest, @@ -129,36 +114,36 @@ async def infer_typescript_source_dependencies( ) metadata = await _prepare_inference_metadata(request.field_set.address, source.file_path) - import_strings = await Get( - NativeParsedJavascriptDependencies, - NativeDependenciesRequest(sources.snapshot.digest, metadata), - ) - - owning_targets_collection = await MultiGet( - Get(Targets, TypeScriptFileImportPath, TypeScriptFileImportPath(path=path)) - for path in import_strings.file_imports - ) - owning_targets = [tgt for targets in owning_targets_collection for tgt in targets] - - non_path_string_bases = FrozenOrderedSet( - non_path_string.partition(os.path.sep)[0] - for non_path_string in import_strings.package_imports - ) - - candidate_pkgs = await Get( - NodePackageCandidateMap, RequestNodePackagesCandidateMap(request.field_set.address) + import_strings, candidate_pkgs = await concurrently( + parse_javascript_deps(NativeDependenciesRequest(sources.snapshot.digest, metadata)), + map_candidate_node_packages( + RequestNodePackagesCandidateMap(request.field_set.address), **implicitly() + ), ) - pkg_addresses = ( - candidate_pkgs[pkg_name] for pkg_name in non_path_string_bases if pkg_name in candidate_pkgs - ) - - return InferredDependencies( - itertools.chain( - pkg_addresses, - (tgt.address for tgt in owning_targets if tgt.has_field(TypeScriptSourceField)), + imports = dict( + zip( + import_strings.imports, + await concurrently( + _determine_import_from_candidates( + candidates, + candidate_pkgs, + file_extensions=TS_FILE_EXTENSIONS + JS_FILE_EXTENSIONS, + ) + for string, candidates in import_strings.imports.items() + ), ) ) + _handle_unowned_imports( + request.field_set.address, + nodejs_infer.unowned_dependency_behavior, + frozenset( + string + for string, addresses in imports.items() + if not addresses and not _is_node_builtin_module(string) + ), + ) + return InferredDependencies(itertools.chain.from_iterable(imports.values())) def rules() -> Iterable[Rule | UnionRule]: diff --git a/src/python/pants/backend/typescript/dependency_inference/rules_test.py b/src/python/pants/backend/typescript/dependency_inference/rules_test.py index b835007feaa..45100d90cba 100644 --- a/src/python/pants/backend/typescript/dependency_inference/rules_test.py +++ b/src/python/pants/backend/typescript/dependency_inference/rules_test.py @@ -26,10 +26,11 @@ TypeScriptSourceTarget, ) from pants.build_graph.address import Address +from pants.core.util_rules.unowned_dependency_behavior import UnownedDependencyError from pants.engine.internals.graph import Owners, OwnersRequest from pants.engine.rules import QueryRule from pants.engine.target import InferredDependencies -from pants.testutil.rule_runner import RuleRunner +from pants.testutil.rule_runner import RuleRunner, engine_error @pytest.fixture @@ -60,7 +61,7 @@ def rule_runner() -> RuleRunner: def test_infers_typescript_file_imports_dependencies(rule_runner: RuleRunner) -> None: rule_runner.write_files( { - "src/ts/BUILD": "typescript_sources()", + "src/ts/BUILD": "typescript_sources()\njavascript_sources(name='js')", "src/ts/index.ts": dedent( """\ import { x } from "./localModuleA"; @@ -73,6 +74,9 @@ def test_infers_typescript_file_imports_dependencies(rule_runner: RuleRunner) -> // You can import a file and not include any variables import "./localModuleC"; + + // You can import a JS module in a TypeScript module + import { x } from './localModuleJs'; """ ), "src/ts/localModuleA.ts": "", @@ -83,6 +87,7 @@ def test_infers_typescript_file_imports_dependencies(rule_runner: RuleRunner) -> "src/ts/localModuleF.ts": "", "src/ts/localModuleG.ts": "", "src/ts/localModuleH.ts": "", + "src/ts/localModuleJs.js": "", } ) @@ -101,6 +106,7 @@ def test_infers_typescript_file_imports_dependencies(rule_runner: RuleRunner) -> Address("src/ts", relative_file_path="localModuleF.ts"), Address("src/ts", relative_file_path="localModuleG.ts"), Address("src/ts", relative_file_path="localModuleH.ts"), + Address("src/ts", relative_file_path="localModuleJs.js", target_name="js"), } @@ -140,3 +146,45 @@ def test_infers_typescript_file_imports_dependencies_parent_dirs(rule_runner: Ru Address("src/ts", relative_file_path="localModuleB.ts"), Address("src/ts/subdir1", relative_file_path="localModuleC.ts"), } + + +def test_unmatched_ts_dependencies_error_unowned_behaviour(rule_runner: RuleRunner) -> None: + rule_runner.set_options(["--nodejs-infer-unowned-dependency-behavior=error"]) + rule_runner.write_files( + { + "root/project/src/modA.ts": "", + "root/project/src/index.ts": dedent( + """\ + import { foo } from "./bar"; + import { something } from "./modA"; + """ + ), + "root/project/src/BUILD": "typescript_sources()", + } + ) + + root_index_tgt = rule_runner.get_target( + Address("root/project/src", relative_file_path="index.ts") + ) + + with engine_error(UnownedDependencyError, contains="./bar"): + rule_runner.request( + InferredDependencies, + [ + InferTypeScriptDependenciesRequest( + TypeScriptSourceInferenceFieldSet.create(root_index_tgt) + ) + ], + ) + + # having unowned dependencies should not lead to errors + rule_runner.set_options(["--nodejs-infer-unowned-dependency-behavior=warning"]) + addresses = rule_runner.request( + InferredDependencies, + [ + InferTypeScriptDependenciesRequest( + TypeScriptSourceInferenceFieldSet.create(root_index_tgt) + ) + ], + ).include + assert list(addresses)[0].spec == Address("root/project/src/modA.ts").spec_path diff --git a/testprojects/src/ts/frontend/config/app.ts b/testprojects/src/ts/frontend/config/app.ts index fc71d236186..cc510112a80 100644 --- a/testprojects/src/ts/frontend/config/app.ts +++ b/testprojects/src/ts/frontend/config/app.ts @@ -11,3 +11,5 @@ import * as Sentry from '@sentry/react'; // local file import import { dispatcher } from './dispatcher'; import { receiver } from './receiver'; + +import { generator } from './generator'; diff --git a/testprojects/src/ts/frontend/config/generator.js b/testprojects/src/ts/frontend/config/generator.js new file mode 100644 index 00000000000..bcf0e9651c4 --- /dev/null +++ b/testprojects/src/ts/frontend/config/generator.js @@ -0,0 +1,2 @@ +// Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). +// Licensed under the Apache License, Version 2.0 (see LICENSE).