From fe0a8ef07c72558397deacdacc46885cc0afc955 Mon Sep 17 00:00:00 2001 From: SJ Date: Thu, 17 Nov 2022 13:49:13 -0500 Subject: [PATCH 1/3] See issue #17557: Created an NpxToolBase in npx_tool.py (up for debate) with version option --- .../backend/javascript/lint/prettier/rules.py | 2 +- .../javascript/lint/prettier/subsystem.py | 4 ++-- .../backend/javascript/subsystems/npx_tool.py | 20 +++++++++++++++++++ .../backend/python/typecheck/pyright/rules.py | 2 +- .../python/typecheck/pyright/subsystem.py | 4 ++-- 5 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 src/python/pants/backend/javascript/subsystems/npx_tool.py diff --git a/src/python/pants/backend/javascript/lint/prettier/rules.py b/src/python/pants/backend/javascript/lint/prettier/rules.py index 2cb6829e5ea..764f2d8e22a 100644 --- a/src/python/pants/backend/javascript/lint/prettier/rules.py +++ b/src/python/pants/backend/javascript/lint/prettier/rules.py @@ -61,7 +61,7 @@ async def prettier_fmt(request: PrettierFmtRequest.Batch, prettier: Prettier) -> result = await Get( ProcessResult, NpxProcess( - npm_package=prettier.default_version, + npm_package=prettier.version, args=( "--write", *request.files, diff --git a/src/python/pants/backend/javascript/lint/prettier/subsystem.py b/src/python/pants/backend/javascript/lint/prettier/subsystem.py index aa2c06ff3a9..a9f3e252ca8 100644 --- a/src/python/pants/backend/javascript/lint/prettier/subsystem.py +++ b/src/python/pants/backend/javascript/lint/prettier/subsystem.py @@ -6,13 +6,13 @@ import os from typing import Iterable +from pants.backend.javascript.subsystems.npx_tool import NpxToolBase from pants.core.util_rules.config_files import ConfigFilesRequest from pants.option.option_types import ArgsListOption, SkipOption -from pants.option.subsystem import Subsystem from pants.util.strutil import softwrap -class Prettier(Subsystem): +class Prettier(NpxToolBase): options_scope = "prettier" name = "Prettier" help = softwrap( diff --git a/src/python/pants/backend/javascript/subsystems/npx_tool.py b/src/python/pants/backend/javascript/subsystems/npx_tool.py new file mode 100644 index 00000000000..a24ba5b1600 --- /dev/null +++ b/src/python/pants/backend/javascript/subsystems/npx_tool.py @@ -0,0 +1,20 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +from typing import ClassVar + +from pants.option.option_types import StrOption +from pants.option.subsystem import Subsystem + + +class NpxToolBase(Subsystem): + # Subclasses must set. + default_version: ClassVar[str] + + version = StrOption( + advanced=True, + default=lambda cls: cls.default_version, + help="Version string for the tool in the form package@version (e.g. prettier@2.6.2)", + ) diff --git a/src/python/pants/backend/python/typecheck/pyright/rules.py b/src/python/pants/backend/python/typecheck/pyright/rules.py index c1648332292..b4cebc6f50f 100644 --- a/src/python/pants/backend/python/typecheck/pyright/rules.py +++ b/src/python/pants/backend/python/typecheck/pyright/rules.py @@ -138,7 +138,7 @@ async def pyright_typecheck_partition( process = await Get( Process, NpxProcess( - npm_package=pyright.default_version, + npm_package=pyright.version, args=( f"--venv-path={complete_pex_env.pex_root}", # Used with `venv` in config *pyright.args, # User-added arguments diff --git a/src/python/pants/backend/python/typecheck/pyright/subsystem.py b/src/python/pants/backend/python/typecheck/pyright/subsystem.py index 7d55d252a17..ad11cd3b273 100644 --- a/src/python/pants/backend/python/typecheck/pyright/subsystem.py +++ b/src/python/pants/backend/python/typecheck/pyright/subsystem.py @@ -3,13 +3,13 @@ from __future__ import annotations +from pants.backend.javascript.subsystems.npx_tool import NpxToolBase from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.option.option_types import ArgsListOption, SkipOption, StrListOption -from pants.option.subsystem import Subsystem from pants.util.strutil import softwrap -class Pyright(Subsystem): +class Pyright(NpxToolBase): options_scope = "pyright" name = "Pyright" help = softwrap( From b0abef87c33dffcd4400eb7404d02f0cc9626c5f Mon Sep 17 00:00:00 2001 From: SJ Date: Thu, 17 Nov 2022 15:35:43 -0500 Subject: [PATCH 2/3] See issue #17557: Adding a sanity test --- .../backend/javascript/subsystems/nodejs.py | 6 ++- .../javascript/subsystems/npx_tool_test.py | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 src/python/pants/backend/javascript/subsystems/npx_tool_test.py diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index df7e8a1797c..ddb7d9ce1fe 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -11,6 +11,7 @@ ExternalToolRequest, TemplatedExternalTool, ) +from pants.core.util_rules.external_tool import rules as external_tool_rules from pants.engine.fs import EMPTY_DIGEST, Digest, MergeDigests from pants.engine.platform import Platform from pants.engine.process import Process @@ -109,4 +110,7 @@ async def setup_npx_process( def rules() -> Iterable[Rule | UnionRule]: - return collect_rules() + return ( + *collect_rules(), + *external_tool_rules(), + ) diff --git a/src/python/pants/backend/javascript/subsystems/npx_tool_test.py b/src/python/pants/backend/javascript/subsystems/npx_tool_test.py new file mode 100644 index 00000000000..a3c29debb27 --- /dev/null +++ b/src/python/pants/backend/javascript/subsystems/npx_tool_test.py @@ -0,0 +1,42 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import pytest + +from pants.backend.javascript.subsystems import nodejs +from pants.backend.javascript.subsystems.npx_tool import NpxToolBase +from pants.engine.rules import SubsystemRule +from pants.testutil.rule_runner import QueryRule, RuleRunner + + +class CowsayTool(NpxToolBase): + options_scope = "cowsay" + name = "Cowsay" + # Intentionally older version. + default_version = "cowsay@1.4.0" + help = "The Cowsay utility for printing cowsay messages" + + +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *nodejs.rules(), + SubsystemRule(CowsayTool), + QueryRule(CowsayTool, []), + ], + ) + + +def test_version_option_overrides_default(rule_runner: RuleRunner): + rule_runner.set_options( + [ + "--backend-packages=['pants.backend.experimental.javascript']", + "--cowsay-version=cowsay@1.5.0", + ] + ) + tool = rule_runner.request(CowsayTool, []) + assert tool.default_version == "cowsay@1.4.0" + assert tool.version == "cowsay@1.5.0" From afa42872f8fa4939a5b5edcbe6b5eb76af7b8f0d Mon Sep 17 00:00:00 2001 From: SJ Date: Thu, 17 Nov 2022 15:48:37 -0500 Subject: [PATCH 3/3] See issue #17557: Replacing SubsystemRule with classmethod and mypy ignore --- .../pants/backend/javascript/subsystems/npx_tool_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/npx_tool_test.py b/src/python/pants/backend/javascript/subsystems/npx_tool_test.py index a3c29debb27..b4aa4f4ebae 100644 --- a/src/python/pants/backend/javascript/subsystems/npx_tool_test.py +++ b/src/python/pants/backend/javascript/subsystems/npx_tool_test.py @@ -7,7 +7,6 @@ from pants.backend.javascript.subsystems import nodejs from pants.backend.javascript.subsystems.npx_tool import NpxToolBase -from pants.engine.rules import SubsystemRule from pants.testutil.rule_runner import QueryRule, RuleRunner @@ -24,7 +23,7 @@ def rule_runner() -> RuleRunner: return RuleRunner( rules=[ *nodejs.rules(), - SubsystemRule(CowsayTool), + *CowsayTool.rules(), # type: ignore[call-arg] QueryRule(CowsayTool, []), ], )