typescript: add support for unowned dependency behavior (#21288)

This is a follow-up of #20293.

As per
#20293 (comment),
Pants should be able to gracefully handle invalid imports.

Instead of having the warnings shown (by default), an error will be
raised:

```
$ pants --backend-packages="+['pants.backend.experimental.typescript']" \
  --nodejs-infer-unowned-dependency-behavior=error \
  dependencies testprojects/src/ts/frontend/config/app.ts
```

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()
             ),
         )

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,22 +33,18 @@
 )
 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,
     HydratedSources,
     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]:

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

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';

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).