Skip to content

Commit f9f322f

Browse files
committed
fix: Codexレビュー指摘を修正 / Fix Codex review findings
1 parent 9eeb997 commit f9f322f

13 files changed

Lines changed: 813 additions & 48 deletions

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,4 @@ jobs:
5252
- name: Enforce coverage thresholds
5353
run: ruby -Ilib:test test/coverage_runner.rb
5454
env:
55-
COVERAGE_BASE_REF: origin/${{ github.base_ref || github.event.repository.default_branch }}
55+
COVERAGE_BASE_REF: ${{ github.event_name == 'push' && github.event.created == false && github.event.deleted == false && github.event.before || format('origin/{0}', github.base_ref || github.event.repository.default_branch) }}

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,29 @@
88
- Required options accept negative exponent notation such as `-1e3` without mistaking it for another option.
99
- Constant discovery now includes classes and modules assigned with `Class.new` / `Module.new` during the target file load.
1010
- Repeated loads retain assigned constant aliases when the source file is unchanged.
11+
- Repeated loads after source edits retain aliases created by direct, guarded, multiple, and `const_set` assignments while their active definitions remain, without reviving aliases behind newly disabled conditions.
1112
- Constructor arity errors raised by `--new` are now wrapped in Rubycli's user-facing runner error.
13+
- Framework argument errors raised by constructors are also wrapped in the same user-facing runner error.
1214
- Positional type conversion now waits for JSON/eval coercion, matching keyword-option behavior and preserving `--new` JSON/eval inputs.
1315
- Runner tests now execute without terminating the Minitest process and assert converted command arguments instead of stubbed targets.
1416
- `--check --new` now inspects exposed instance/class commands without running constructors, while `--check` rejects pre-scripts instead of evaluating them.
17+
- `--check` now inspects explicitly selected commands even when their methods or defining procs come from required files.
1518
- Explicit nested constant names no longer fall back to inherited top-level constants, and malformed pre-scripts now produce contextual Rubycli errors.
1619
- Rest, optional-before-required, and trailing-required positional arguments now follow Ruby's argument binding rules for conversion, validation, and help output.
1720
- Documented scalar/list conversions now preserve numeric-looking strings, handle repeated booleans, return real `DateTime` values, accept JSON arrays, and reject scalar/array values where `JSON`/`Hash` shapes do not allow them.
1821
- Positional `Symbol` annotations preserve colon-prefixed symbol literals instead of embedding the colon in the symbol name.
1922
- Assignment-like positional values remain positional unless they match a keyword, while matching assignments use the same documented conversion as long options.
2023
- YARD positional tags are aligned by parameter name instead of comment order.
2124
- Eval-mode local variables and command-line strict/check/result-output flags no longer leak across separate programmatic runs.
25+
- Constructor and command eval arguments share one binding per Runner execution without enabling eval mode while the target file loads.
26+
- Required options accept a lone `-` value, bare rest placeholders render with an ellipsis, and quoted `String[]` elements remain strings.
2227
- Invalid Ruby syntax passed through strict eval mode now produces a user-facing Rubycli argument error instead of leaking a `SyntaxError` backtrace.
2328
- Circular arrays/hashes returned by commands now fall back to inspected output instead of raising a JSON nesting error.
2429

2530
### Testing
2631
- Added dependency-free overall line, branch, and changed-line coverage gates plus GitHub Actions checks spanning the supported Ruby range.
2732
- Changed-line coverage now treats new library files that were never loaded by the test suite as uncovered.
33+
- Push coverage compares against the previous commit and falls back to the default branch when a new ref has no previous commit or a ref is deleted.
2834

2935
## [0.1.7] - 2025-11-12
3036

README.ja.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,8 @@ Rubycli が角括弧を補います。
274274
受け付けます。スペース区切りの複数値(`--tags build test`)には対応しておらず、
275275
繰り返し注記のないオプションはスカラーのままです。`--strict` 実行時は各要素の型も
276276
検証されるため、`[String[]]` と書かれた注釈に対して `--tags [1,2]` を渡すとエラーになります。
277+
`--tags '["true","null"]'` のように引用された要素は、別のリテラルに見える内容でも文字列の
278+
まま保持されます。
277279

278280
### リテラル列挙(enum)
279281

@@ -417,8 +419,10 @@ rubycli -E scripts/report_runner.rb publish \
417419
--channels '[:email, :slack]'
418420
```
419421

420-
評価は隔離された binding(`Object.new.instance_eval { binding }`)内で行われますが、
421-
入力そのものは信頼できる呼び出し元に限定してください。プログラムからは
422+
評価は隔離された binding(`Object.new.instance_eval { binding }`)内で行われます。
423+
1回の Runner 実行では、`--new=VALUE` のコンストラクタ引数と選択したコマンドの引数を含む
424+
すべての eval 引数が同じ binding を共有し、実行終了時に破棄されます。入力そのものは
425+
信頼できる呼び出し元に限定してください。プログラムからは
422426
`Rubycli.with_eval_mode(true) { ... }` で切り替えられます。
423427

424428
`--eval-lax` / `-E``--eval-args` と同様に eval モードを有効にしつつ、Ruby として

README.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,9 @@ syntax (`--tags '["build","test"]'`) and comma-delimited strings
282282
(`--tags "build,test"`) are accepted. Space-separated multi-value flags
283283
(`--tags build test`) are not supported, and options without a repeated/array
284284
hint stay scalars. `--strict` verifies each element against the documented
285-
type, so `--tags [1,2]` fails when the docs say `[String[]]`.
285+
type, so `--tags [1,2]` fails when the docs say `[String[]]`. Quoted elements
286+
remain strings even when their contents look like other literals, such as
287+
`--tags '["true","null"]'`.
286288

287289
### Literal choices and enums
288290

@@ -440,9 +442,11 @@ rubycli -E scripts/report_runner.rb publish \
440442
```
441443

442444
Evaluation happens inside an isolated binding
443-
(`Object.new.instance_eval { binding }`). Treat this as unsafe input: do not
444-
enable it for untrusted callers. Programmatic equivalent:
445-
`Rubycli.with_eval_mode(true) { ... }`.
445+
(`Object.new.instance_eval { binding }`). All eval arguments in one Runner
446+
execution, including `--new=VALUE` constructor arguments and the selected
447+
command's arguments, share that binding; it is discarded after the execution.
448+
Treat this as unsafe input: do not enable it for untrusted callers.
449+
Programmatic equivalent: `Rubycli.with_eval_mode(true) { ... }`.
446450

447451
`--eval-lax` / `-E` behaves like `--eval-args`, but tokens that fail to parse
448452
as Ruby (for example a bare `https://example.com`) produce a warning and are

lib/rubycli.rb

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -221,24 +221,33 @@ def execute(
221221
raise Error, '--json-args cannot be combined with --eval-args or --eval-lax'
222222
end
223223

224-
runner_target, full_path = prepare_runner_target(
225-
target_path,
226-
class_name,
227-
new: new,
228-
new_args: new_args,
229-
json_mode: json,
230-
eval_mode: eval_args,
231-
eval_lax: eval_lax,
232-
pre_scripts: pre_scripts,
233-
constant_mode: constant_mode
234-
)
235-
236-
original_program_name = $PROGRAM_NAME
224+
original_program_name = nil
237225
original_argv = nil
238-
$PROGRAM_NAME = File.basename(full_path)
239-
original_argv = ARGV.dup
240-
ARGV.replace(Array(cli_args).dup)
241-
run_with_modes(runner_target, json: json, eval_args: eval_args, eval_lax: eval_lax)
226+
execution = proc do
227+
runner_target, full_path = prepare_runner_target(
228+
target_path,
229+
class_name,
230+
new: new,
231+
new_args: new_args,
232+
json_mode: json,
233+
eval_mode: eval_args,
234+
eval_lax: eval_lax,
235+
pre_scripts: pre_scripts,
236+
constant_mode: constant_mode
237+
)
238+
239+
original_program_name = $PROGRAM_NAME
240+
$PROGRAM_NAME = File.basename(full_path)
241+
original_argv = ARGV.dup
242+
ARGV.replace(Array(cli_args).dup)
243+
run_with_modes(runner_target, json: json, eval_args: eval_args, eval_lax: eval_lax)
244+
end
245+
246+
if eval_args
247+
Rubycli.eval_coercer.with_eval_binding(&execution)
248+
else
249+
execution.call
250+
end
242251
ensure
243252
$PROGRAM_NAME = original_program_name if original_program_name
244253
ARGV.replace(original_argv) if original_argv
@@ -372,7 +381,7 @@ def instantiate_target(target, initializer_args = nil)
372381
else
373382
target
374383
end
375-
rescue ::ArgumentError => e
384+
rescue ::ArgumentError, Rubycli::ArgumentError => e
376385
raise Error, "Failed to instantiate target: #{e.message}"
377386
end
378387

@@ -382,7 +391,7 @@ def run_with_modes(target, json:, eval_args:, eval_lax:)
382391
if json
383392
Rubycli.with_json_mode(true, &runner)
384393
elsif eval_args
385-
Rubycli.with_eval_mode(true, lax: eval_lax, &runner)
394+
Rubycli.with_eval_mode(true, lax: eval_lax, reuse_binding: true, &runner)
386395
else
387396
runner.call
388397
end
@@ -399,7 +408,7 @@ def parse_initializer_arguments(raw_value, target, json_mode:, eval_mode:, eval_
399408
keyword_args = {}
400409

401410
Rubycli.argument_mode_controller.with_json_mode(json_mode) do
402-
Rubycli.argument_mode_controller.with_eval_mode(eval_mode, lax: eval_lax) do
411+
Rubycli.argument_mode_controller.with_eval_mode(eval_mode, lax: eval_lax, reuse_binding: true) do
403412
positional_args, keyword_args = Rubycli.argument_parser.parse(tokens.dup, initializer_method)
404413
Rubycli.apply_argument_coercions(positional_args, keyword_args)
405414
Rubycli.argument_parser.validate_inputs(initializer_method, positional_args, keyword_args)
@@ -480,13 +489,12 @@ def documentation_methods_for(target, full_path, instantiate:)
480489
return methods
481490
end
482491

483-
normalized = normalize_path(full_path)
484-
class_methods = collect_defined_methods(target.singleton_class, normalized)
492+
class_methods = target.singleton_class.public_instance_methods(false)
485493
.map { |name| target.method(name) }
486494
.select { |method_obj| Rubycli.cli.send(:exposable_method?, method_obj) }
487495
return class_methods unless instantiate
488496

489-
instance_methods = collect_defined_methods(target, normalized)
497+
instance_methods = target.public_instance_methods(false)
490498
.map { |name| target.instance_method(name) }
491499
.select { |method_obj| Rubycli.cli.send(:exposable_method?, method_obj) }
492500
target.is_a?(Class) ? instance_methods + class_methods : instance_methods

lib/rubycli/argument_parser.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -743,13 +743,17 @@ def converter_for_single_type(type)
743743
inner = normalized[6..-2].strip
744744
element_converter = converter_for_single_type(inner)
745745
->(value) {
746-
list_items(value).map { |item| element_converter ? element_converter.call(item) : item }
746+
list_items(value).map do |item|
747+
inner == 'String' && item.is_a?(String) ? item : (element_converter ? element_converter.call(item) : item)
748+
end
747749
}
748750
elsif normalized.end_with?('[]')
749751
inner = normalized[0..-3]
750752
element_converter = converter_for_single_type(inner)
751753
->(value) {
752-
list_items(value).map { |item| element_converter ? element_converter.call(item) : item }
754+
list_items(value).map do |item|
755+
inner == 'String' && item.is_a?(String) ? item : (element_converter ? element_converter.call(item) : item)
756+
end
753757
}
754758
elsif normalized == 'Array'
755759
->(value) { list_items(value) }
@@ -782,7 +786,7 @@ def convert_option_value(keyword, value, option_meta, type_converters)
782786

783787
def looks_like_option?(token)
784788
return false unless token
785-
return false if token == '--'
789+
return false if token == '--' || token == '-'
786790

787791
token.start_with?('-') && !token.match?(
788792
/\A-(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?\z/

0 commit comments

Comments
 (0)