Skip to content

feat: support :integer option type in CLI spec #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
toshi0806 merged 4 commits into from
Jul 22, 2026
Merged

feat: support :integer option type in CLI spec #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d741a39
feat: support :integer option type in CLI spec
toshi0806 Jul 22, 2026
2b08f44
test: cover rejection of non-integer values for :integer options
toshi0806 Jul 22, 2026
5629776
chore: retrigger CI (no runs started for previous push)
toshi0806 Jul 22, 2026
6adf83e
style: alias ToolKit.CLI.Parser in spec test
toshi0806 Jul 22, 2026
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
7 changes: 4 additions & 3 deletions lib/tool_kit/cli/spec.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ defmodule ToolKit.CLI.Spec do
defstruct [:tool_name, :tool_summary, :option_catalog, :global_option_names, :commands]

@type option_def :: %{
type: :boolean | :string,
type: :boolean | :string | :integer,
alias: atom() | nil,
values: [String.t()] | nil,

@github-actions github-actions Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] :integer 型のオプションに対して values フィールドが nil 以外（例: ["1", "2", "4"] のような候補リスト）を持つケースを将来的に許容するかどうかが不明確です。現在の render_values/1 が :string 向けに設計されている場合、:integer で values が非 nil のときの描画が意図通りになるか確認が必要です。型定義上 values: [String.t()] | nil のままであり、:integer との組み合わせの意味論が曖昧です。将来の拡張を見越してコメントや型定義で制約を明示することを検討してください。

@toshi0806 toshi0806 Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

据え置きます。values は enum 検証(パース済み値が文字列リストに含まれるか)のためのフィールドで、意味論上 :string 専用です。:integer オプションで values を設定すると validate_opts が必ず不一致になるため事実上使えず、使う予定もありません(現行の利用者 rm/tm/em に該当なし)。必要になった時点で型定義の制約強化を検討します。

doc: String.t()
Expand Down Expand Up @@ -78,7 +78,7 @@ defmodule ToolKit.CLI.Spec do
end

@doc "OptionParser の strict リスト(全オプションの和集合)"
@spec strict_switches(t()) :: [{atom(), :boolean | :string}]
@spec strict_switches(t()) :: [{atom(), :boolean | :string | :integer}]
def strict_switches(%__MODULE__{option_catalog: catalog}) do
Enum.map(catalog, fn {name, %{type: type}} -> {name, type} end)
end
Expand Down Expand Up @@ -206,7 +206,8 @@ defmodule ToolKit.CLI.Spec do
end

defp render_option_line(option) do
value = if option.type == :string, do: " #{render_values(option)}", else: ""
# 値を取る型(string / integer)には VALUE プレースホルダを表示する
value = if option.type == :boolean, do: "", else: " #{render_values(option)}"

if single_char_name?(option.name) do
" -#{option.name}#{value} #{option.doc}"
Expand Down
32 changes: 32 additions & 0 deletions test/tool_kit/cli/spec_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule ToolKit.CLI.SpecTest do
use ExUnit.Case, async: true

alias ToolKit.CLI.Parser
alias ToolKit.CLI.Spec
alias ToolKit.Test.RegistryManagerSpecFixture, as: Fixture

Expand Down Expand Up @@ -45,6 +46,37 @@ defmodule ToolKit.CLI.SpecTest do
end
end

test "integer options derive strict switches and render a VALUE placeholder" do
spec = %Spec{
tool_name: "demo",
tool_summary: "demo tool",
option_catalog: %{
help: %{type: :boolean, alias: :h, values: nil, doc: "help"},
jobs: %{type: :integer, alias: nil, values: nil, doc: "並列数"}
},
global_option_names: [:help],
commands: [
%{
name: "run",
aliases: [],
usage: ["run"],
summary: "run",
options: [:jobs],
examples: ["run --jobs 4"]
}
]
}

assert {:jobs, :integer} in Spec.strict_switches(spec)
assert Spec.render_command_help(spec, "run") =~ "--jobs VALUE"
assert Spec.validate_opts(spec, "run", jobs: 4) == :ok

@github-actions github-actions Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [MEDIUM] validate_opts/3 に対して jobs: 4（整数値）のみ検証していますが、:integer 型のオプションに文字列や浮動小数点数が渡された場合（例: jobs: "four" や jobs: 3.14）のバリデーション失敗ケースがテストされていません。OptionParser が :integer を正しく拒否するかどうかは実装依存であり、将来の変更で誤った値が通過するリスクがあります。正常系に加えて異常系のテストケースも追加することを推奨します。

@toshi0806 toshi0806 Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

修正済み(2b08f44)。整数でない値の拒否は OptionParser(strict)の責務で、Parser がパース段階のエラーに変換します。Parser.parse(spec, ["run", "--jobs", "four"]) が {:error, ...} になる negative テストを追加しました。


# 整数でない値の拒否は OptionParser(strict)の責務で、
# Parser がパース段階のエラーに変換する
assert {:error, message} = Parser.parse(spec, ["run", "--jobs", "four"])

@github-actions github-actions Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] Spec.validate_opts(spec, "run", jobs: 4) で :ok を確認していますが、jobs: -1 や jobs: 0 など境界値・負値のケースが検証されていません。また、Parser.parse/2 で "four" を渡した際のエラーメッセージが "--jobs" を含むことのみを確認していますが、エラーの種類（型不一致）を示すメッセージ内容についてもアサーションを追加すると、将来のリグレッション検出に役立ちます。

@toshi0806 toshi0806 Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

据え置きます。負値・0 などの値域検証は spec エンジンの責務ではなくツール側のビジネスロジックの責務です(OptionParser は任意の整数を受理し、例えば max_concurrency の下限はツールが判断する)。エラーメッセージ文言のアサーション追加は、文言変更のたびに壊れる brittle なテストになるため、エラーであること + 対象オプション名の包含の検証に留めています。

assert message =~ "--jobs"
end

test "command option overrides replace values and doc", %{spec: spec} do
list_command = Spec.find_command(spec, "list")
sort = spec |> Spec.options_for(list_command) |> Enum.find(&(&1.name == :sort))
Expand Down
Loading