[internal] Refactor GenerateJvmLockfileFromTool to work with `scala…

…c_plugins.py` (#14230)

When we have a tool lockfile, we must have a rule that goes from a subclass of `GenerateToolLockfileSentinel -> GenerateJvmLockfile`. To accommodate that `[tool].artifacts` can be either coordinate strings or addresses to `jvm_artifact` targets, we had an intermediate `GenerateJvmLockfileFromTool` type to do that normalization.

But, `scalac_plugins.py` does not use a normal `JvmToolBase`, so the original factoring did not work. We were duplicating the setup and violating DRY.

Now, when you have a tool, we have one single way to set up the `GenerateJvmLockfile` for that tool.

--

Note that I tried to inline `GatherJvmCoordinatesRequest` in the `GenerateJvmLockfileFromTool` rule, but we can't because we use it for a non-lockfile usage:

https://github.com/pantsbuild/pants/blob/c184d0af42a6670bdd43da12f10d7c59c100144e/src/python/pants/backend/codegen/protobuf/scala/rules.py#L239-L254

[ci skip-rust]

src/python/pants/backend/codegen/avro/java/rules.py

@@ -34,15 +34,14 @@
     HydrateSourcesRequest,
 )
 from pants.engine.unions import UnionRule
-from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
+from pants.jvm.resolve import jvm_tool
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool, ValidatedJvmToolLockfileRequest
 from pants.source.source_root import SourceRoot, SourceRootRequest
 from pants.util.logging import LogLevel
 
@@ -217,16 +216,16 @@ def make_avro_process(
 
 
 @rule
-async def generate_avro_tools_lockfile_request(
+def generate_avro_tools_lockfile_request(
     _: AvroToolLockfileSentinel, tool: AvroSubsystem
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(tool))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(tool)
 
 
 def rules():
     return (
         *collect_rules(),
-        *lockfile.rules(),
+        *jvm_tool.rules(),
         UnionRule(GenerateSourcesRequest, GenerateJavaFromAvroRequest),
         UnionRule(GenerateToolLockfileSentinel, AvroToolLockfileSentinel),
     )

src/python/pants/backend/codegen/protobuf/scala/rules.py

@@ -45,15 +45,18 @@
 from pants.engine.unions import UnionRule
 from pants.jvm.compile import ClasspathEntry
 from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
 from pants.jvm.resolve.common import ArtifactRequirements, Coordinate
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import GatherJvmCoordinatesRequest, ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import (
+    GatherJvmCoordinatesRequest,
+    GenerateJvmLockfileFromTool,
+    ValidatedJvmToolLockfileRequest,
+)
 from pants.source.source_root import SourceRoot, SourceRootRequest
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import FrozenOrderedSet
@@ -242,7 +245,7 @@ async def materialize_jvm_plugin(request: MaterializeJvmPluginRequest) -> Materi
         ArtifactRequirements,
         GatherJvmCoordinatesRequest(
             artifact_inputs=FrozenOrderedSet([request.plugin.artifact]),
-            option_name="--scalapb-plugin-artifacts",
+            option_name="[scalapb].jvm_plugins",
         ),
     )
     classpath = await Get(
@@ -375,10 +378,10 @@ async def setup_scalapb_shim_classfiles(
 
 
 @rule
-async def generate_scalapbc_lockfile_request(
+def generate_scalapbc_lockfile_request(
     _: ScalapbcToolLockfileSentinel, tool: ScalaPBSubsystem
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(tool))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(tool)
 
 
 def rules():

src/python/pants/backend/codegen/thrift/scrooge/rules.py

@@ -16,14 +16,13 @@
 from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest, WrappedTarget
 from pants.engine.unions import UnionRule
 from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool, ValidatedJvmToolLockfileRequest
 from pants.source.source_root import SourceRootsRequest, SourceRootsResult
 from pants.util.logging import LogLevel
 
@@ -141,10 +140,10 @@ async def generate_scrooge_thrift_sources(
 
 
 @rule
-async def generate_scrooge_lockfile_request(
+def generate_scrooge_lockfile_request(
     _: ScroogeToolLockfileSentinel, scrooge: ScroogeSubsystem
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(scrooge))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(scrooge)
 
 
 def rules():

src/python/pants/backend/experimental/codegen/avro/java/register.py

@@ -1,15 +1,13 @@
 # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
+
 from pants.backend.codegen.avro.java.rules import rules as avro_java_rules
 from pants.backend.codegen.avro.rules import rules as avro_rules
 from pants.backend.codegen.avro.target_types import AvroSourcesGeneratorTarget, AvroSourceTarget
 from pants.core.util_rules import config_files, source_files, stripped_source_files
-from pants.core.util_rules.external_tool import rules as external_tool_rules
-from pants.jvm import classpath
-from pants.jvm.compile import rules as jvm_compile_rules
-from pants.jvm.jdk_rules import rules as jdk_rules
-from pants.jvm.resolve.coursier_fetch import rules as coursier_fetch_rules
-from pants.jvm.resolve.coursier_setup import rules as coursier_setup_rules
+from pants.jvm import classpath, compile, jdk_rules
+from pants.jvm.goals import lockfile
+from pants.jvm.resolve import coursier_fetch, jvm_tool
 from pants.jvm.util_rules import rules as util_rules
 
 
@@ -24,12 +22,12 @@ def rules():
         # Re-export rules necessary to avoid rule graph errors.
         *config_files.rules(),
         *classpath.rules(),
-        *coursier_fetch_rules(),
-        *coursier_setup_rules(),
-        *external_tool_rules(),
+        *coursier_fetch.rules(),
+        *jvm_tool.rules(),
         *source_files.rules(),
         *util_rules(),
-        *jdk_rules(),
+        *jdk_rules.rules(),
         *stripped_source_files.rules(),
-        *jvm_compile_rules(),
+        *compile.rules(),
+        *lockfile.rules(),
     ]

src/python/pants/backend/experimental/java/lint/google_java_format/register.py

@@ -3,8 +3,8 @@
 
 from pants.backend.java.lint.google_java_format import rules as fmt_rules
 from pants.backend.java.lint.google_java_format import skip_field
-from pants.jvm import jdk_rules
-from pants.jvm import util_rules as jvm_util_rules
+from pants.jvm import jdk_rules, util_rules
+from pants.jvm.goals import lockfile
 from pants.jvm.resolve import coursier_fetch, jvm_tool
 
 
@@ -13,7 +13,8 @@ def rules():
         *fmt_rules.rules(),
         *skip_field.rules(),
         *jdk_rules.rules(),
+        *lockfile.rules(),
         *jvm_tool.rules(),
         *coursier_fetch.rules(),
-        *jvm_util_rules.rules(),
+        *util_rules.rules(),
     ]

src/python/pants/backend/java/lint/google_java_format/rules.py

@@ -17,15 +17,14 @@
 from pants.engine.rules import collect_rules, rule
 from pants.engine.target import FieldSet, Target
 from pants.engine.unions import UnionRule
-from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
+from pants.jvm.resolve import jvm_tool
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool, ValidatedJvmToolLockfileRequest
 from pants.util.logging import LogLevel
 from pants.util.strutil import pluralize
 
@@ -168,16 +167,16 @@ async def google_java_format_lint(
 
 
 @rule
-async def generate_google_java_format_lockfile_request(
+def generate_google_java_format_lockfile_request(
     _: GoogleJavaFormatToolLockfileSentinel, tool: GoogleJavaFormatSubsystem
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(tool))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(tool)
 
 
 def rules():
     return [
         *collect_rules(),
-        *lockfile.rules(),
+        *jvm_tool.rules(),
         UnionRule(FmtRequest, GoogleJavaFormatRequest),
         UnionRule(LintRequest, GoogleJavaFormatRequest),
         UnionRule(GenerateToolLockfileSentinel, GoogleJavaFormatToolLockfileSentinel),

src/python/pants/backend/scala/compile/scalac_plugins.py

@@ -18,14 +18,12 @@
 from pants.engine.target import WrappedTarget
 from pants.engine.unions import UnionRule
 from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile
-from pants.jvm.resolve.common import ArtifactRequirements
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import GatherJvmCoordinatesRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool
 from pants.jvm.resolve.jvm_tool import rules as jvm_tool_rules
 from pants.util.ordered_set import FrozenOrderedSet
 from pants.util.strutil import bullet_list
@@ -72,20 +70,14 @@ class GlobalScalacPluginsToolLockfileSentinel(GenerateToolLockfileSentinel):
 
 
 @rule
-async def generate_global_scalac_plugins_lockfile_request(
+def generate_global_scalac_plugins_lockfile_request(
     _: GlobalScalacPluginsToolLockfileSentinel,
     loaded_global_plugins: _LoadedGlobalScalacPlugins,
     scalac_plugins: Scalac,
-) -> GenerateJvmLockfile:
-    artifacts = await Get(
-        ArtifactRequirements,
-        GatherJvmCoordinatesRequest(
-            FrozenOrderedSet(loaded_global_plugins.artifact_address_inputs),
-            f"[{scalac_plugins.options_scope}].plugins_global",
-        ),
-    )
-    return GenerateJvmLockfile(
-        artifacts=artifacts,
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool(
+        FrozenOrderedSet(loaded_global_plugins.artifact_address_inputs),
+        artifact_option_name=f"[{scalac_plugins.options_scope}].plugins_global",
         resolve_name="scalac-plugins",
         lockfile_dest=scalac_plugins.plugins_global_lockfile,
     )

src/python/pants/backend/scala/lint/scalafmt/rules.py

@@ -30,14 +30,13 @@
 from pants.engine.target import FieldSet, Target
 from pants.engine.unions import UnionRule
 from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool, ValidatedJvmToolLockfileRequest
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.strutil import pluralize
@@ -325,10 +324,10 @@ async def scalafmt_lint(field_sets: ScalafmtRequest, tool: ScalafmtSubsystem) ->
 
 
 @rule
-async def generate_scalafmt_lockfile_request(
+def generate_scalafmt_lockfile_request(
     _: ScalafmtToolLockfileSentinel, tool: ScalafmtSubsystem
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(tool))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(tool)
 
 
 def rules():

src/python/pants/backend/scala/test/scalatest.py

@@ -22,14 +22,13 @@
 from pants.engine.unions import UnionRule
 from pants.jvm.classpath import Classpath
 from pants.jvm.goals import lockfile
-from pants.jvm.goals.lockfile import GenerateJvmLockfile, GenerateJvmLockfileFromTool
 from pants.jvm.jdk_rules import JdkSetup
 from pants.jvm.resolve.coursier_fetch import (
     CoursierResolvedLockfile,
     MaterializedClasspath,
     MaterializedClasspathRequest,
 )
-from pants.jvm.resolve.jvm_tool import ValidatedJvmToolLockfileRequest
+from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool, ValidatedJvmToolLockfileRequest
 from pants.jvm.subsystems import JvmSubsystem
 from pants.util.logging import LogLevel
 
@@ -169,10 +168,10 @@ async def setup_scalatest_debug_request(field_set: ScalatestTestFieldSet) -> Tes
 
 
 @rule
-async def generate_scalatest_lockfile_request(
+def generate_scalatest_lockfile_request(
     _: ScalatestToolLockfileSentinel, scalatest: Scalatest
-) -> GenerateJvmLockfile:
-    return await Get(GenerateJvmLockfile, GenerateJvmLockfileFromTool(scalatest))
+) -> GenerateJvmLockfileFromTool:
+    return GenerateJvmLockfileFromTool.create(scalatest)
 
 
 def rules():

src/python/pants/jvm/goals/lockfile.py

@@ -20,56 +20,21 @@
 from pants.engine.rules import Get, collect_rules, rule
 from pants.engine.target import AllTargets
 from pants.engine.unions import UnionRule
-from pants.jvm.resolve import coursier_fetch, jvm_tool
+from pants.jvm.resolve import coursier_fetch
 from pants.jvm.resolve.common import ArtifactRequirement, ArtifactRequirements
 from pants.jvm.resolve.coursier_fetch import CoursierResolvedLockfile
-from pants.jvm.resolve.jvm_tool import GatherJvmCoordinatesRequest, JvmToolBase
 from pants.jvm.resolve.key import CoursierResolveKey
 from pants.jvm.resolve.lockfile_metadata import JVMLockfileMetadata
 from pants.jvm.subsystems import JvmSubsystem
 from pants.jvm.target_types import JvmArtifactCompatibleResolvesField
 from pants.util.logging import LogLevel
-from pants.util.meta import frozen_after_init
-from pants.util.ordered_set import FrozenOrderedSet
 
 
 @dataclass(frozen=True)
 class GenerateJvmLockfile(GenerateLockfile):
     artifacts: ArtifactRequirements
 
 
-@frozen_after_init
-@dataclass(unsafe_hash=True)
-class GenerateJvmLockfileFromTool:
-    artifact_inputs: FrozenOrderedSet[str]
-    options_scope: str
-    lockfile_dest: str
-
-    def __init__(self, tool: JvmToolBase) -> None:
-        # Note that `JvmToolBase` is not hashable, so we extract the relevant information eagerly.
-        self.artifact_inputs = FrozenOrderedSet(tool.artifact_inputs)
-        self.options_scope = tool.options_scope
-        self.lockfile_dest = tool.lockfile
-
-
-@rule
-async def setup_lockfile_request_from_tool(
-    request: GenerateJvmLockfileFromTool,
-) -> GenerateJvmLockfile:
-    artifacts = await Get(
-        ArtifactRequirements,
-        GatherJvmCoordinatesRequest(
-            request.artifact_inputs,
-            f"[{request.options_scope}].artifacts",
-        ),
-    )
-    return GenerateJvmLockfile(
-        artifacts=artifacts,
-        resolve_name=request.options_scope,
-        lockfile_dest=request.lockfile_dest,
-    )
-
-
 @rule
 def wrap_jvm_lockfile_request(request: GenerateJvmLockfile) -> WrappedGenerateLockfile:
     return WrappedGenerateLockfile(request)
@@ -164,7 +129,6 @@ def rules():
     return (
         *collect_rules(),
         *coursier_fetch.rules(),
-        *jvm_tool.rules(),
         UnionRule(GenerateLockfile, GenerateJvmLockfile),
         UnionRule(KnownUserResolveNamesRequest, KnownJVMUserResolveNamesRequest),
         UnionRule(RequestedUserResolveNames, RequestedJVMserResolveNames),