Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/pre_commit_terraform/__main__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""A runpy-style CLI entry-point module."""

from sys import argv, exit as exit_with_return_code
from sys import argv
from sys import exit as exit_with_return_code

from ._cli import invoke_cli_app

Expand Down
2 changes: 1 addition & 1 deletion src/pre_commit_terraform/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def invoke_cli_app(cli_args: list[str]) -> ReturnCodeType:
parsed_cli_args = root_cli_parser.parse_args(cli_args)
invoke_cli_app = cast_to(
# FIXME: attempt typing per https://stackoverflow.com/a/75666611/595220
CLIAppEntryPointCallableType,
'CLIAppEntryPointCallableType',
parsed_cli_args.invoke_cli_app,
)

Expand Down
2 changes: 1 addition & 1 deletion src/pre_commit_terraform/_cli_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def attach_subcommand_parsers_to(root_cli_parser: ArgumentParser, /) -> None:
)
for subcommand_module in SUBCOMMAND_MODULES:
subcommand_parser = subcommand_parsers.add_parser(
subcommand_module.CLI_SUBCOMMAND_NAME
subcommand_module.CLI_SUBCOMMAND_NAME,
)
subcommand_parser.set_defaults(
invoke_cli_app=subcommand_module.invoke_cli_app,
Expand Down
22 changes: 13 additions & 9 deletions src/pre_commit_terraform/terraform_docs_replace.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType:
'https://github.com/antonbabenko/pre-commit-terraform/issues/248'
'#issuecomment-1290829226',
category=UserWarning,
stacklevel=2,
Comment thread
MaxymVlasov marked this conversation as resolved.
Outdated
Comment thread
MaxymVlasov marked this conversation as resolved.
Outdated
)

dirs: list[str] = []
for filename in cast_to(list[str], parsed_cli_args.filenames):
for filename in cast_to('list[str]', parsed_cli_args.filenames):
if os.path.realpath(filename) not in dirs and (
filename.endswith('.tf') or filename.endswith('.tfvars')
filename.endswith(('.tf', '.tfvars'))
):
dirs.append(os.path.dirname(filename))

Expand All @@ -68,14 +69,17 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType:
try:
procArgs = []
procArgs.append('terraform-docs')
if cast_to(bool, parsed_cli_args.sort):
if cast_to('bool', parsed_cli_args.sort):
procArgs.append('--sort-by-required')
procArgs.append('md')
procArgs.append('./{dir}'.format(dir=dir))
procArgs.append('>')
procArgs.append(
'./{dir}/{dest}'.format(
dir=dir, dest=cast_to(bool, parsed_cli_args.dest)
procArgs.extend(
(
'md',
f'./{dir}',
'>',
'./{dir}/{dest}'.format(
dir=dir,
dest=cast_to('bool', parsed_cli_args.dest),
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect type casting

There's a bug in this line. You're casting parsed_cli_args.dest to 'bool' when it should be a string (default is 'README.md' as defined in line 23).

-                        dest=cast_to('bool', parsed_cli_args.dest),
+                        dest=cast_to('str', parsed_cli_args.dest),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dest=cast_to('bool', parsed_cli_args.dest),
dest=cast_to('str', parsed_cli_args.dest),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have this rectified please? I mean accepting coderabbit's code suggestion?

This comment was marked as off-topic.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was marked as off-topic.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have this rectified please? I mean accepting coderabbit's code suggestion?

FWIW, this is not really related to the linting violations that are being fixed in this PR. It might be better to do this separately.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your organization has reached its limit of developer seats under the Pro Plan. In order to use the Chat feature, you need to buy additional seats

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this is not really related to the linting violations that are being fixed in this PR. It might be better to do this separately.

Let's do it one way or another. Coderabbit is crying and I also don't think the confusing expression is worth of keeping in code once we've found it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your organization has reached its limit of developer seats under the Pro Plan. In order to use the Chat feature, you need to buy additional seats

),
),
)
subprocess.check_call(' '.join(procArgs), shell=True)
Expand Down
8 changes: 4 additions & 4 deletions tests/pytest/_cli_test.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
"""Tests for the high-level CLI entry point."""

from argparse import ArgumentParser, Namespace
import pytest

import pytest
from pre_commit_terraform import _cli_parsing as _cli_parsing_mod
from pre_commit_terraform._cli import invoke_cli_app
from pre_commit_terraform._errors import (
PreCommitTerraformExit,
PreCommitTerraformBaseError,
PreCommitTerraformExit,
PreCommitTerraformRuntimeError,
)
from pre_commit_terraform._structs import ReturnCode
Expand Down Expand Up @@ -67,7 +67,7 @@ def invoke_cli_app(self, parsed_cli_args: Namespace) -> ReturnCodeType:
[CustomCmdStub()],
)

assert ReturnCode.ERROR == invoke_cli_app(['sentinel'])
assert invoke_cli_app(['sentinel']) == ReturnCode.ERROR

captured_outputs = capsys.readouterr()
assert captured_outputs.err == f'{expected_stderr !s}\n'
Expand All @@ -89,7 +89,7 @@ def populate_argument_parser(
return None

def invoke_cli_app(self, parsed_cli_args: Namespace) -> ReturnCodeType:
raise PreCommitTerraformExit('sentinel')
raise PreCommitTerraformExit(self.CLI_SUBCOMMAND_NAME)

monkeypatch.setattr(
_cli_parsing_mod,
Expand Down
8 changes: 5 additions & 3 deletions tests/pytest/terraform_docs_replace_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
from argparse import ArgumentParser, Namespace
from subprocess import CalledProcessError

import pytest
import pytest_mock

import pytest
from pre_commit_terraform._structs import ReturnCode
from pre_commit_terraform.terraform_docs_replace import (
invoke_cli_app,
populate_argument_parser,
)
from pre_commit_terraform.terraform_docs_replace import (
subprocess as replace_docs_subprocess_mod,
)

Expand Down Expand Up @@ -82,7 +84,7 @@ def test_control_flow_positive(
check_call_mock,
)

assert ReturnCode.OK == invoke_cli_app(parsed_cli_args)
assert invoke_cli_app(parsed_cli_args) == ReturnCode.OK

executed_commands = [
cmd for ((cmd,), _shell) in check_call_mock.call_args_list
Expand Down Expand Up @@ -117,6 +119,6 @@ def test_control_flow_negative(
check_call_mock,
)

assert ReturnCode.ERROR == invoke_cli_app(parsed_cli_args)
assert invoke_cli_app(parsed_cli_args) == ReturnCode.ERROR

check_call_mock.assert_called_once_with(expected_cmd, shell=True)
Loading