From 8e57e8075dc4adcacf9b8dd49abc8c0b6e50f9e0 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 1 Jan 2023 18:14:55 -0500 Subject: [PATCH] avoid using hook.src in R language this wasn't meant to be read -- hook.prefix works fine for local too --- pre_commit/languages/r.py | 18 ++++----------- tests/languages/r_test.py | 48 ++++++++++++++------------------------- 2 files changed, 22 insertions(+), 44 deletions(-) diff --git a/pre_commit/languages/r.py b/pre_commit/languages/r.py index d281102b..c050d451 100644 --- a/pre_commit/languages/r.py +++ b/pre_commit/languages/r.py @@ -44,19 +44,11 @@ def _get_env_dir(prefix: Prefix, version: str) -> str: return prefix.path(helpers.environment_dir(ENVIRONMENT_DIR, version)) -def _prefix_if_non_local_file_entry( - entry: Sequence[str], - prefix: Prefix, - src: str, -) -> Sequence[str]: +def _prefix_if_file_entry(entry: list[str], prefix: Prefix) -> Sequence[str]: if entry[1] == '-e': return entry[1:] else: - if src == 'local': - path = entry[1] - else: - path = prefix.path(entry[1]) - return (path,) + return (prefix.path(entry[1]),) def _rscript_exec() -> str: @@ -67,7 +59,7 @@ def _rscript_exec() -> str: return os.path.join(r_home, 'bin', win_exe('Rscript')) -def _entry_validate(entry: Sequence[str]) -> None: +def _entry_validate(entry: list[str]) -> None: """ Allowed entries: # Rscript -e expr @@ -91,8 +83,8 @@ def _cmd_from_hook(hook: Hook) -> tuple[str, ...]: _entry_validate(entry) return ( - *entry[:1], *RSCRIPT_OPTS, - *_prefix_if_non_local_file_entry(entry, hook.prefix, hook.src), + entry[0], *RSCRIPT_OPTS, + *_prefix_if_file_entry(entry, hook.prefix), *hook.args, ) diff --git a/tests/languages/r_test.py b/tests/languages/r_test.py index c52d5acd..c653a3cc 100644 --- a/tests/languages/r_test.py +++ b/tests/languages/r_test.py @@ -16,27 +16,18 @@ def _test_r_parsing( tempdir_factory, store, hook_id, - expected_hook_expr={}, - expected_args={}, - config={}, - expect_path_prefix=True, + expected_hook_expr=(), + expected_args=(), + config=None, ): - repo_path = 'r_hooks_repo' - path = make_repo(tempdir_factory, repo_path) - config = config or make_config_from_repo(path) + repo = make_repo(tempdir_factory, 'r_hooks_repo') + config = make_config_from_repo(repo) hook = _get_hook_no_install(config, store, hook_id) ret = r._cmd_from_hook(hook) - expected_cmd = 'Rscript' - expected_opts = ( - '--no-save', '--no-restore', '--no-site-file', '--no-environ', - ) - expected_path = os.path.join( - hook.prefix.prefix_dir if expect_path_prefix else '', - f'{hook_id}.R', - ) + expected_path = os.path.join(hook.prefix.prefix_dir, f'{hook_id}.R') expected = ( - expected_cmd, - *expected_opts, + 'Rscript', + '--no-save', '--no-restore', '--no-site-file', '--no-environ', *(expected_hook_expr or (expected_path,)), *expected_args, ) @@ -84,9 +75,7 @@ def test_r_parsing_expr_no_opts_no_args2(tempdir_factory, store): def test_r_parsing_expr_opts_no_args2(tempdir_factory, store): with pytest.raises(ValueError) as execinfo: r._entry_validate( - [ - 'Rscript', '--vanilla', '-e', '1+1', '-e', 'letters', - ], + ['Rscript', '--vanilla', '-e', '1+1', '-e', 'letters'], ) msg = execinfo.value.args assert msg == ( @@ -112,24 +101,21 @@ def test_r_parsing_expr_non_Rscirpt(tempdir_factory, store): def test_r_parsing_file_local(tempdir_factory, store): - path = 'path/to/script.R' - hook_id = 'local-r' config = { 'repo': 'local', 'hooks': [{ - 'id': hook_id, + 'id': 'local-r', 'name': 'local-r', - 'entry': f'Rscript {path}', + 'entry': 'Rscript path/to/script.R', 'language': 'r', }], } - _test_r_parsing( - tempdir_factory, - store, - hook_id=hook_id, - expected_hook_expr=(path,), - config=config, - expect_path_prefix=False, + hook = _get_hook_no_install(config, store, 'local-r') + ret = r._cmd_from_hook(hook) + assert ret == ( + 'Rscript', + '--no-save', '--no-restore', '--no-site-file', '--no-environ', + hook.prefix.path('path/to/script.R'), )