diff --git a/lib/rdoc.rb b/lib/rdoc.rb index b42059c712..16530bd61d 100644 --- a/lib/rdoc.rb +++ b/lib/rdoc.rb @@ -153,6 +153,7 @@ def self.home autoload :RDoc, "#{__dir__}/rdoc/rdoc" + autoload :Checker, "#{__dir__}/rdoc/checker" autoload :CrossReference, "#{__dir__}/rdoc/cross_reference" autoload :ERBIO, "#{__dir__}/rdoc/erbio" autoload :ERBPartial, "#{__dir__}/rdoc/erb_partial" diff --git a/lib/rdoc/checker.rb b/lib/rdoc/checker.rb new file mode 100644 index 0000000000..f00c941ddc --- /dev/null +++ b/lib/rdoc/checker.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +## +# RDoc::Checker collects documentation quality check warnings during RDoc +# execution. Warnings are reported at the end and cause a non-zero exit status. +# +# Example usage: +# RDoc::Checker.add("Missing reference", file: "lib/foo.rb", line: 42) +# +# At the end of rdoc execution, if any warnings were added, they will be +# printed and the process will exit with status 1. + +class RDoc::Checker + + ## + # Represents a single check warning with optional file and line information. + + class Warning + attr_reader :message, :file, :line + + def initialize(message, file: nil, line: nil) + @message = message + @file = file + @line = line + end + + def to_s + if file && line + "#{file}:#{line}: #{message}" + elsif file + "#{file}: #{message}" + else + message + end + end + end + + @warnings = [] + + class << self + ## + # Add a warning to the checker. + # + # message - The warning message + # file - Optional file path where the issue was found + # line - Optional line number where the issue was found + + def add(message, file: nil, line: nil) + @warnings << Warning.new(message, file: file, line: line) + end + + ## + # Returns all collected warnings. + + def warnings + @warnings + end + + ## + # Clear all warnings. Used between test runs. + + def clear + @warnings = [] + end + + ## + # Returns true if any warnings have been collected. + + def any? + @warnings.any? + end + + ## + # Print all collected warnings to stdout. + # Returns early if no warnings exist. + + def report + return if @warnings.empty? + + puts + puts "Documentation check failures:" + @warnings.each do |warning| + puts " #{warning}" + end + puts + puts "#{@warnings.size} check(s) failed" + end + end +end diff --git a/lib/rdoc/markup/to_html_crossref.rb b/lib/rdoc/markup/to_html_crossref.rb index ad600663bf..9810e9d4e2 100644 --- a/lib/rdoc/markup/to_html_crossref.rb +++ b/lib/rdoc/markup/to_html_crossref.rb @@ -158,7 +158,18 @@ def link(name, text, code = true, rdoc_ref: false) case ref when String then if rdoc_ref && @options.warn_missing_rdoc_ref - puts "#{@from_path}: `rdoc-ref:#{name}` can't be resolved for `#{text}`" + # If we can, provide the source file where the document is written + # Or we fall back to the from_path, which is the html file being generated + source_file = if @context.respond_to?(:top_level) && @context.top_level + @context.top_level.full_name + else + @from_path + end + RDoc::Checker.add( + "`rdoc-ref:#{name}` can't be resolved for `#{text}`", + file: source_file, + line: @context&.line + ) end ref else diff --git a/lib/rdoc/rdoc.rb b/lib/rdoc/rdoc.rb index 8beeac52f5..6b91ed21e8 100644 --- a/lib/rdoc/rdoc.rb +++ b/lib/rdoc/rdoc.rb @@ -491,7 +491,14 @@ def document(options) puts @stats.summary.accept RDoc::Markup::ToRdoc.new end - exit @stats.fully_documented? if @options.coverage_report + failed_checks = RDoc::Checker.any? + RDoc::Checker.report if failed_checks + + if @options.coverage_report + exit(@stats.fully_documented? && !failed_checks) + else + exit 1 if failed_checks + end end ## diff --git a/test/rdoc/markup/to_html_crossref_test.rb b/test/rdoc/markup/to_html_crossref_test.rb index cf61460297..99366f5dad 100644 --- a/test/rdoc/markup/to_html_crossref_test.rb +++ b/test/rdoc/markup/to_html_crossref_test.rb @@ -124,13 +124,15 @@ def test_convert_RDOCLINK_rdoc_ref end def test_convert_RDOCLINK_rdoc_ref_not_found - result = nil - stdout, _ = capture_output do - result = @to.convert 'rdoc-ref:FOO' - end + RDoc::Checker.clear + + result = @to.convert 'rdoc-ref:FOO' assert_equal para("FOO"), result - assert_include stdout, "index.html: `rdoc-ref:FOO` can't be resolved for `FOO`" + assert RDoc::Checker.any? + warning = RDoc::Checker.warnings.first + assert_equal "xref_data.rb", warning.file + assert_match(/rdoc-ref:FOO/, warning.message) end def test_convert_RDOCLINK_rdoc_ref_method @@ -217,11 +219,14 @@ def test_gen_url end def test_gen_url_rdoc_ref_not_found - stdout, _ = capture_output do - @to.gen_url 'rdoc-ref:FOO', 'FOO' - end + RDoc::Checker.clear + + @to.gen_url 'rdoc-ref:FOO', 'FOO' - assert_include stdout, "index.html: `rdoc-ref:FOO` can't be resolved for `FOO`" + assert RDoc::Checker.any? + warning = RDoc::Checker.warnings.first + assert_equal "xref_data.rb", warning.file + assert_match(/rdoc-ref:FOO/, warning.message) end def test_handle_regexp_CROSSREF diff --git a/test/rdoc/rdoc_checker_integration_test.rb b/test/rdoc/rdoc_checker_integration_test.rb new file mode 100644 index 0000000000..417dab7524 --- /dev/null +++ b/test/rdoc/rdoc_checker_integration_test.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require_relative 'helper' + +class RDocCheckerIntegrationTest < RDoc::TestCase + def test_document_reports_checker_warnings + RDoc::Checker.add("test warning", file: "test.rb") + + out, _err = capture_output do + temp_dir do + File.write("test.rb", "# comment\nclass Foo; end") + + assert_raise(SystemExit) do + @rdoc.document(["test.rb"]) + end + end + end + + assert_match(/Documentation check failures:/, out) + assert_match(/test warning/, out) + end + + def test_document_exits_with_status_1_when_warnings + RDoc::Checker.add("test warning") + + temp_dir do + File.write("test.rb", "# comment\nclass Foo; end") + + error = assert_raise(SystemExit) do + capture_output do + @rdoc.document(["test.rb"]) + end + end + + assert_equal 1, error.status + end + end + + def test_document_does_not_exit_when_no_warnings + temp_dir do + File.write("test.rb", "# comment\nclass Foo; end") + + # Should not raise SystemExit + capture_output do + @rdoc.document(["test.rb"]) + end + end + end + + def test_document_coverage_report_with_warnings_exits_failure + @options.coverage_report = true + RDoc::Checker.add("test warning") + + temp_dir do + File.write("test.rb", "# Documented class\nclass Foo; end") + + error = assert_raise(SystemExit) do + capture_output do + @rdoc.document(["test.rb"]) + end + end + + # Even if fully documented, warnings should cause failure + assert_equal false, error.success? + end + end +end diff --git a/test/rdoc/rdoc_checker_test.rb b/test/rdoc/rdoc_checker_test.rb new file mode 100644 index 0000000000..07cd144baf --- /dev/null +++ b/test/rdoc/rdoc_checker_test.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require_relative 'helper' + +class RDocCheckerTest < RDoc::TestCase + def setup + super + RDoc::Checker.clear + end + + def test_warning_to_s_with_file_and_line + warning = RDoc::Checker::Warning.new("test message", file: "foo.rb", line: 42) + assert_equal "foo.rb:42: test message", warning.to_s + end + + def test_warning_to_s_with_file_only + warning = RDoc::Checker::Warning.new("test message", file: "foo.rb") + assert_equal "foo.rb: test message", warning.to_s + end + + def test_warning_to_s_with_message_only + warning = RDoc::Checker::Warning.new("test message") + assert_equal "test message", warning.to_s + end + + def test_add_collects_warnings + RDoc::Checker.add("warning 1") + RDoc::Checker.add("warning 2", file: "bar.rb") + + assert_equal 2, RDoc::Checker.warnings.size + assert_equal "warning 1", RDoc::Checker.warnings[0].message + assert_equal "bar.rb", RDoc::Checker.warnings[1].file + end + + def test_any_returns_false_when_empty + refute RDoc::Checker.any? + end + + def test_any_returns_true_when_warnings_exist + RDoc::Checker.add("a warning") + assert RDoc::Checker.any? + end + + def test_clear_removes_all_warnings + RDoc::Checker.add("warning") + assert RDoc::Checker.any? + + RDoc::Checker.clear + refute RDoc::Checker.any? + end + + def test_report_outputs_nothing_when_empty + out, _err = capture_output do + RDoc::Checker.report + end + + assert_empty out + end + + def test_report_outputs_warnings_grouped + RDoc::Checker.add("first warning", file: "a.rb", line: 1) + RDoc::Checker.add("second warning", file: "b.rb") + + out, _err = capture_output do + RDoc::Checker.report + end + + assert_match(/Documentation check failures:/, out) + assert_match(/a\.rb:1: first warning/, out) + assert_match(/b\.rb: second warning/, out) + assert_match(/2 check\(s\) failed/, out) + end +end diff --git a/test/rdoc/support/test_case.rb b/test/rdoc/support/test_case.rb index cb1a3bddd9..8f1f042ccd 100644 --- a/test/rdoc/support/test_case.rb +++ b/test/rdoc/support/test_case.rb @@ -60,6 +60,7 @@ def setup @rdoc.generator = Object.new RDoc::Markup::PreProcess.reset + RDoc::Checker.clear end ##