diff --git a/.github/workflows/ci-checks.yml b/.github/workflows/ci-checks.yml index e1f90283..5c020044 100644 --- a/.github/workflows/ci-checks.yml +++ b/.github/workflows/ci-checks.yml @@ -76,21 +76,32 @@ jobs: typecheck: timeout-minutes: 10 - name: typecheck (${{ matrix.format }}) + name: typecheck (rbi) + permissions: + contents: read + runs-on: ${{ inputs.runner }} + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false + - name: Set up Ruby + uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1 + with: + ruby-version: '4.0' + bundler-cache: true + - name: Typecheck rbi files + run: bundle exec rake typecheck:sorbet + + validate-rbs: + timeout-minutes: 10 + name: validate (rbs) permissions: contents: read runs-on: ${{ inputs.runner }} env: - # Steep defaults to two workers in CI; eight is the measured balance for this runner. + # The consolidated green path always uses one worker; keep the reference + # fallback tuned so failing builds produce exact diagnostics promptly. STEEP_JOBS: 8 - strategy: - fail-fast: false - matrix: - include: - - format: rbi - task: typecheck:sorbet - - format: rbs - task: typecheck:steep steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: @@ -100,8 +111,8 @@ jobs: with: ruby-version: '4.0' bundler-cache: true - - name: Typecheck ${{ matrix.format }} files - run: bundle exec rake ${{ matrix.task }} + - name: Validate rbs files + run: bundle exec rake validate:rbs package: timeout-minutes: 10 @@ -151,6 +162,7 @@ jobs: - stainless-artifact - lint - typecheck + - validate-rbs - package - test-ruby if: ${{ always() }} @@ -162,11 +174,13 @@ jobs: ARTIFACT_RESULT: ${{ needs.stainless-artifact.result }} LINT_RESULT: ${{ needs.lint.result }} TYPECHECK_RESULT: ${{ needs.typecheck.result }} + RBS_VALIDATION_RESULT: ${{ needs.validate-rbs.result }} PACKAGE_RESULT: ${{ needs.package.result }} TEST_RESULT: ${{ needs.test-ruby.result }} run: | test "$ARTIFACT_RESULT" = "success" || test "$ARTIFACT_RESULT" = "skipped" test "$LINT_RESULT" = "success" test "$TYPECHECK_RESULT" = "success" + test "$RBS_VALIDATION_RESULT" = "success" test "$PACKAGE_RESULT" = "success" test "$TEST_RESULT" = "success" diff --git a/Rakefile b/Rakefile index 48461f77..f246c53e 100644 --- a/Rakefile +++ b/Rakefile @@ -59,6 +59,7 @@ RuboCop::RakeTask.new(:"lint:rubocop") do |task| task.patterns = FileList[ "./{lib,test,rbi,examples}/**/*.rb", "./{lib,test,rbi,examples}/**/*.rbi", + "./scripts/validate-rbs" ] task.formatters = %w[github] if ENV.key?("CI") @@ -129,12 +130,9 @@ end desc("Format everything") multitask(format: [:"format:rb", :"format:rbi", :"format:rbs"]) -desc("Typecheck `*.rbs`") -multitask(:"typecheck:steep") do - steep = %w[steep check] - steep += ["--jobs", ENV.fetch("STEEP_JOBS")] if ENV.key?("STEEP_JOBS") - steep += %w[--format=github] if ENV.key?("CI") - sh(*steep) +desc("Validate `*.rbs`") +multitask(:"validate:rbs") do + ruby(*%w[scripts/validate-rbs]) end directory(examples) @@ -149,8 +147,8 @@ directory(tapioca) do sh(*%w[tapioca init]) end -desc("Typecheck everything") -multitask(typecheck: [:"typecheck:steep", :"typecheck:sorbet"]) +desc("Typecheck and validate everything") +multitask(typecheck: [:"typecheck:sorbet", :"validate:rbs"]) desc("Lint and typecheck") multitask(lint: [:"lint:rubocop", :typecheck]) diff --git a/scripts/validate-rbs b/scripts/validate-rbs new file mode 100755 index 00000000..5d0453a7 --- /dev/null +++ b/scripts/validate-rbs @@ -0,0 +1,123 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "fileutils" +require "open3" +require "pathname" +require "tmpdir" + +module OpenAI + module RBSValidation + # These directives change name resolution on a per-file basis, so signatures + # containing them cannot be safely combined and must take the reference path. + FILE_SCOPED_DIRECTIVE = /^\s*(?:use\b|#\s*resolve-type-names\s*:)/ + + module_function + + def run(root: Pathname.pwd, stdout: $stdout, stderr: $stderr, env: ENV.to_h) + steepfile = root.join("Steepfile") + raise Errno::ENOENT, steepfile unless steepfile.file? + + signatures = signature_paths(root) + raise "No RBS files found under #{root.join('sig')}" if signatures.empty? + + status = + if signatures.any? { requires_reference_check?(_1) } + run_reference_check(root, stdout: stdout, stderr: stderr, env: env) + else + fast_status = run_parse_check(root, env: env) + fast_status = run_consolidated_check(root, steepfile, signatures, env: env) if fast_status.success? + unless fast_status.success? + stdout.puts( + "Fast RBS validation failed; checking original signature files for exact diagnostics." + ) + fast_status = run_reference_check(root, stdout: stdout, stderr: stderr, env: env) + end + fast_status + end + + if status.success? + noun = signatures.one? ? "file" : "files" + stdout.puts("Validated #{signatures.size} RBS #{noun} with no errors.") + 0 + else + 1 + end + rescue StandardError => e + message = + if e.respond_to?(:detailed_message) + e.detailed_message(highlight: false) + else + "#{e.class}: #{e.message}" + end + stderr.puts(message) + 1 + end + + def signature_paths(root) + Dir.glob(root.join("sig/**/*.rbs")).map { Pathname(_1) } + end + private_class_method :signature_paths + + def requires_reference_check?(path) + contents = path.binread + # RBS 3.9 treats NUL as EOF. In a combined buffer, that would hide every + # declaration from later files even though the originals parse separately. + # Match the complete file because RBS's magic-comment grammar permits its + # whitespace matcher to span lines (for example, "#\nresolve-type-names:"). + contents.include?("\0") || contents.match?(FILE_SCOPED_DIRECTIVE) + end + private_class_method :requires_reference_check? + + def run_parse_check(root, env:) + _stdout, _stderr, status = Open3.capture3(env, "rbs", "parse", "sig", chdir: root.to_s) + status + end + private_class_method :run_parse_check + + def run_consolidated_check(root, steepfile, signatures, env:) + Dir.mktmpdir("openai-rbs-validation") do |directory| + temporary_root = Pathname(directory) + temporary_sig = temporary_root.join("sig") + temporary_sig.mkpath + temporary_steepfile = temporary_root.join("Steepfile") + FileUtils.cp(steepfile, temporary_steepfile) + # Steep validates reopened namespaces once per input file. Combining the + # generated declarations avoids that repeated work; Steep still performs + # all parsing and semantic validation through its public CLI. + concatenate(signatures, temporary_sig.join("all.rbs")) + + command = [env.fetch("STEEP_COMMAND", "steep"), "check", "--no-type-check", "--jobs=1"] + command << "--steepfile=#{temporary_steepfile}" + _stdout, _stderr, status = Open3.capture3(env, *command, chdir: root.to_s) + status + end + end + private_class_method :run_consolidated_check + + def concatenate(signatures, destination) + destination.open("wb") do |combined| + signatures.each do |path| + contents = path.binread + combined.write(contents) + combined.write("\n") unless contents.end_with?("\n") + combined.write("\n") + end + end + end + private_class_method :concatenate + + def run_reference_check(root, stdout:, stderr:, env:) + command = [env.fetch("STEEP_COMMAND", "steep"), "check", "--no-type-check"] + command << "--jobs=#{env.fetch('STEEP_JOBS')}" if env.key?("STEEP_JOBS") + command << "--format=github" if env.key?("CI") + output, errors, status = Open3.capture3(env, *command, chdir: root.to_s) + stdout.print(output) + stderr.print(errors) + status + end + private_class_method :run_reference_check + end +end + +exit(OpenAI::RBSValidation.run) if $PROGRAM_NAME == __FILE__ diff --git a/test/scripts/validate_rbs_test.rb b/test/scripts/validate_rbs_test.rb new file mode 100644 index 00000000..805338fb --- /dev/null +++ b/test/scripts/validate_rbs_test.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true + +require "fileutils" +require "minitest/autorun" +require "open3" +require "pathname" +require "rbconfig" +require "tmpdir" + +class ValidateRBSScriptTest < Minitest::Test + SCRIPT = Pathname(__dir__).join("../../scripts/validate-rbs").expand_path + + def test_validates_reopened_namespaces + stdout, stderr, status = run_validation( + { + "first.rbs" => <<~RBS, + module Example + class First + def value: () -> String + end + end + RBS + "second.rbs" => <<~RBS + module Example + class Second + def value: () -> Integer + end + end + RBS + } + ) + + assert_predicate(status, :success?, stderr) + assert_includes(stdout, "Validated 2 RBS files with no errors.") + end + + def test_does_not_allow_invalid_files_to_repair_each_other + stdout, _stderr, status = run_validation( + { + "a.rbs" => "class Example\n", + "b.rbs" => "end\n" + } + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::SyntaxError") + assert_match(/[ab]\.rbs/, stdout) + end + + def test_rechecks_original_files_when_a_signature_contains_nul + stdout, _stderr, status = run_validation( + { + "a.rbs" => "module Good\nend\n\x00", + "b.rbs" => "module Invalid\n type bad = MissingType\nend\n" + } + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::UnknownTypeName") + assert_includes(stdout, "b.rbs") + end + + def test_uses_one_consolidated_steep_check_on_success + stdout, stderr, status, calls = run_validation( + {"valid.rbs" => "module Example\nend\n"}, + fake_steep_results: [0] + ) + + assert_predicate(status, :success?, stderr) + assert_empty(stderr) + assert_includes(stdout, "Validated 1 RBS file with no errors.") + assert_equal(1, calls.size) + assert_equal(%w[check --no-type-check --jobs=1], calls.first.take(3)) + assert(calls.first.last.start_with?("--steepfile="), calls.first.inspect) + end + + def test_rechecks_original_files_when_consolidated_check_fails + stdout, stderr, status, calls = run_validation( + {"valid.rbs" => "module Example\nend\n"}, + env: {"CI" => "1", "STEEP_JOBS" => "8"}, + fake_steep_results: [1, 0] + ) + + assert_predicate(status, :success?, stderr) + assert_empty(stderr) + assert_includes(stdout, "checking original signature files") + assert_equal(2, calls.size) + assert(calls.first.last.start_with?("--steepfile="), calls.first.inspect) + assert_equal(%w[check --no-type-check --jobs=8 --format=github], calls.last) + end + + def test_checks_original_files_directly_when_a_signature_has_use_directives + _stdout, stderr, status, calls = run_validation( + {"uses.rbs" => "use Example::*\nmodule UsesExample\nend\n"}, + fake_steep_results: [0] + ) + + assert_predicate(status, :success?, stderr) + assert_empty(stderr) + assert_equal([%w[check --no-type-check]], calls) + end + + def test_checks_original_files_when_a_use_directive_has_no_whitespace + stdout, _stderr, status = run_validation( + { + "a.rbs" => "use::Example::Foo\nmodule Example\n class Foo\n end\nend\n", + "b.rbs" => "module Invalid\n type bad = Foo\nend\n" + } + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::UnknownTypeName") + assert_includes(stdout, "b.rbs") + end + + def test_checks_original_files_directly_when_type_name_resolution_is_file_scoped + _stdout, stderr, status, calls = run_validation( + {"unresolved.rbs" => "# resolve-type-names: false\nmodule Example\nend\n"}, + fake_steep_results: [0] + ) + + assert_predicate(status, :success?, stderr) + assert_empty(stderr) + assert_equal([%w[check --no-type-check]], calls) + end + + def test_checks_original_files_when_type_name_resolution_directive_is_multiline + stdout, _stderr, status = run_validation( + { + "a.rbs" => "#\nresolve-type-names: false\nmodule Good\nend\n", + "b.rbs" => "class Broken\n def call: () -> Missing\nend\n" + } + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::UnknownTypeName") + assert_includes(stdout, "b.rbs") + end + + def test_reports_steep_semantic_errors + stdout, _stderr, status = run_validation( + { + "invalid.rbs" => <<~RBS + module Example + class Box[A < Numeric] + end + + class InvalidBox < Box[String] + end + end + RBS + } + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::UnsatisfiableTypeApplication") + assert_includes(stdout, "Type application of `::Example::Box` doesn't satisfy the constraints") + end + + def test_reports_syntax_errors + stdout, _stderr, status = run_validation( + {"invalid.rbs" => "module Example\n def broken: () ->\nend\n"} + ) + + refute_predicate(status, :success?) + assert_includes(stdout, "RBS::SyntaxError") + end + + def test_emits_github_annotations_in_ci + stdout, _stderr, status = run_validation( + {"invalid.rbs" => "module Example\n type invalid = MissingType\nend\n"}, + env: {"CI" => "1"} + ) + + refute_predicate(status, :success?) + assert_match(/^::error file=.*invalid\.rbs,line=2,.*::/, stdout) + assert_includes(stdout, "RBS::UnknownTypeName") + end + + private + + def run_validation(signatures, env: {}, fake_steep_results: nil) + env = {"CI" => nil}.merge(env) + + Dir.mktmpdir("openai-rbs-validation-test") do |dir| + root = Pathname(dir) + sig = root.join("sig") + sig.mkpath + root.join("Steepfile").write(<<~RUBY) + target(:lib) do + signature("sig") + end + RUBY + + signatures.each do |name, contents| + path = sig.join(name) + path.dirname.mkpath + path.write(contents) + end + + log = fake_steep_results && install_fake_steep(root) + if log + env = env.merge( + "FAKE_STEEP_LOG" => log.to_s, + "FAKE_STEEP_RESULTS" => fake_steep_results.join(","), + "STEEP_COMMAND" => root.join("bin/steep").to_s + ) + end + + stdout, stderr, status = Open3.capture3(env, RbConfig.ruby, SCRIPT.to_s, chdir: root.to_s) + calls = log&.file? ? log.readlines(chomp: true).map { _1.split("\t") } : [] + [stdout, stderr, status, calls] + end + end + + def install_fake_steep(root) + bin = root.join("bin") + bin.mkpath + executable = bin.join("steep") + executable.write(<<~RUBY) + #!/usr/bin/env ruby + # frozen_string_literal: true + + log = ENV.fetch("FAKE_STEEP_LOG") + call = File.file?(log) ? File.foreach(log).count : 0 + File.open(log, "a") { _1.puts(ARGV.join("\\t")) } + results = ENV.fetch("FAKE_STEEP_RESULTS").split(",").map { Integer(_1) } + exit(results.fetch(call, results.last)) + RUBY + executable.chmod(0o755) + root.join("steep.log") + end +end