Merge pull request #680 from gjtorikian/abstract-reporter

Abstract reporter

README.md

@@ -261,15 +261,15 @@ The `HTMLProofer` constructor takes an optional hash of additional options:
 | :----- | :---------- | :------ |
 | `allow_hash_href` | If `true`, assumes `href="#"` anchors are valid | `true` |
 | `allow_missing_href` | If `true`, does not flag `a` tags missing `href`. In HTML5, this is technically allowed, but could also be human error. | `false` |
-| `assume_extension` | Automatically add specified extension to file paths, to allow extensionless URLs (as supported by static sites) | `.html` |
+| `assume_extension` | Automatically add specified extension to files for internal links, to allow extensionless URLs (as supported by most servers) | `.html` |
 | `attribute_override` | JSON-formatted string that maps elements names to the attribute to check | `{}` |
 | `checks`| An array of Strings indicating which checks you want to run | `['Links', 'Images', 'Scripts']`
 | `check_external_hash` | Checks whether external hashes exist (even if the webpage exists) | `false` |
 | `check_sri` | Check that `<link>` and `<script>` external resources use SRI |false |
 | `directory_index_file` | Sets the file to look for when a link refers to a directory. | `index.html` |
 | `disable_external` | If `true`, does not run the external link checker | `false` |
 | `enforce_https` | Fails a link if it's not marked as `https`. | `true` |
-| `extensions` | An array of Strings indicating the extensions you would like to check (including the dot) | `['.html']`
+| `extensions` | An array of Strings indicating the file extensions you would like to check (including the dot) | `['.html']`
 | `ignore_files` | An array of Strings or RegExps containing file paths that are safe to ignore. | `[]` |
 | `ignore_empty_mailto` | If `true`, allows `mailto:` `href`s which do not contain an email address. | `false`
 | `ignore_missing_alt` | If `true`, ignores images with empty/missing alt tags | `false` |
@@ -428,31 +428,43 @@ Here's an example custom test demonstrating these concepts. It reports `mailto`
 
 ``` ruby
 class MailToOctocat < ::HTMLProofer::Check
-  def mailto?
-    return false if @link.data_proofer_ignore || @link.href.nil?
-    @link.href.match /mailto/
-  end
-
-  def octocat?
-    return false if @link.data_proofer_ignore || @link.href.nil?
-    @link.href.match /octocat@github.com/
+  def mailto_octocat?
+    @link.url.raw_attribute == 'mailto:octocat@github.com'
   end
 
   def run
     @html.css('a').each do |node|
       @link = create_element(node)
-      line = node.line
 
-      if mailto? && octocat?
-        return add_failure("Don't email the Octocat directly!", line: line)
-      end
+      next if @link.ignore?
+
+      return add_failure("Don't email the Octocat directly!", line: @link.line) if mailto_octocat?
     end
   end
 end
 ```
 
+Don't forget to include this new check in HTMLProofer's options, for example:
+
+```ruby
+# removes default checks and just runs this one
+HTMLProofer.check_directories(["out/"], {checks: ['MailToOctocat']})
+```
+
 See our [list of third-party custom classes](https://github.com/gjtorikian/html-proofer/wiki/Extensions-(custom-classes)) and add your own to this list.
 
+## Reporting
+
+By default, HTML-Proofer has its own reporting mechanism to print errors at the end of the run. You can choose to use your own reporter by passing in your own subclass of `HTMLProofer::Reporter`:
+
+``` ruby
+proofer = HTMLProofer.check_directory(item, opts)
+proofer.reporter = MyCustomReporter.new(logger: proofer.logger)
+proofer.run
+```
+
+Your custom reporter must implement the `report` function which implements the behavior you wish to see. The `logger` kwarg is optional.
+
 ## Troubleshooting
 
 Here are some brief snippets identifying some common problems that you can work around. For more information, check out [our wiki](https://github.com/gjtorikian/html-proofer/wiki).

bin/htmlproofer

@@ -18,15 +18,15 @@ Mercenary.program(:htmlproofer) do |p|
   p.option 'allow_hash_href', '--allow-hash-href', 'If `true`, assumes `href="#"` anchors are valid'
   p.option 'allow_missing_href', '--allow-missing-href', 'If `true`, does not flag `a` tags missing `href`. In HTML5, this is technically allowed, but could also be human error.'
   p.option 'as_links', '--as-links', 'Assumes that `PATH` is a comma-separated array of links to check.'
-  p.option 'assume_extension', '--assume-extension <ext>', 'Automatically add specified extension to file paths, to allow extensionless URLs (as supported by static sites) (default: `.html`).'
+  p.option 'assume_extension', '--assume-extension <ext>', 'Automatically add specified extension to files for internal links, to allow extensionless URLs (as supported by most servers) (default: `.html`).'
   p.option 'attribute_override', '--attribute-override CONFIG', String, 'JSON-formatted string that maps elements names to the attribute to check (default: `{}`).'
   p.option 'checks', '--checks check1,[check2,...]', Array, 'A comma-separated list of Strings indicating which checks you want to run (default: `["Links", "Images", "Scripts"]`)'
   p.option 'check_external_hash', '--check-external-hash', 'Checks whether external hashes exist (even if the webpage exists) (default: `false`).'
   p.option 'check_sri', '--check-sri', 'Check that `<link>` and `<script>` external resources use SRI (default: `false`).'
   p.option 'directory_index_file', '--directory-index-file <filename>', String, 'Sets the file to look for when a link refers to a directory. (default: `index.html`)'
   p.option 'disable_external', '--disable-external', 'If `true`, does not run the external link checker (default: `false`)'
   p.option 'enforce_https', '--enforce-https', 'Fails a link if it\'s not marked as `https` (default: `true`).'
-  p.option 'extensions', '--extensions ext1,[ext2,...[', Array, 'A comma-separated list of Strings indicating the extensions you would like to check (including the dot) (default: `.html`)'
+  p.option 'extensions', '--extensions ext1,[ext2,...[', Array, 'A comma-separated list of Strings indicating the file extensions you would like to check (including the dot) (default: `.html`)'
   p.option 'ignore_files', '--ignore-files file1,[file2,...]', Array, 'A comma-separated list of Strings or RegExps containing file paths that are safe to ignore'
   p.option 'ignore_empty_mailto', '--ignore-empty-mailto', 'If `true`, allows `mailto:` `href`s which do not contain an email address'
   p.option 'ignore_missing_alt', '--empty-alt-ignore', 'If `true`, ignores images with empty/missing alt tags'

lib/html_proofer/reporter.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module HTMLProofer
+  class Reporter
+    include HTMLProofer::Utils
+
+    attr_reader :failures
+
+    def initialize(logger: nil)
+      @logger = logger
+    end
+
+    def failures=(failures)
+      @failures = failures.group_by(&:check_name) \
+                          .transform_values { |issues| issues.sort_by { |issue| [issue.path, issue.line] } } \
+                          .sort
+    end
+
+    def report
+      raise NotImplementedError, 'HTMLProofer::Reporter subclasses must implement #report'
+    end
+  end
+end

lib/html_proofer/reporter/cli.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class HTMLProofer::Reporter::Cli < HTMLProofer::Reporter
+  def report
+    msg = failures.each_with_object([]) do |(check_name, failures), arr|
+      str = ["For the #{check_name} check, the following failures were found:\n"]
+
+      failures.each do |failure|
+        path_str = blank?(failure.path) ? '' : "* In #{failure.path}"
+
+        line_str = failure.line.nil? ? '' : " (line #{failure.line})"
+
+        status_str = failure.status.nil? ? '' : " (#{failure.status})"
+
+        str << <<~MSG
+          #{path_str}#{line_str}:
+
+             #{failure.description}#{status_str}
+        MSG
+      end
+
+      arr << str.join("\n")
+    end
+
+    @logger.log(:error, msg.join("\n"))
+  end
+end

lib/html_proofer/runner.rb

@@ -4,8 +4,8 @@ module HTMLProofer
   class Runner
     include HTMLProofer::Utils
 
-    attr_reader :options, :cache, :logger, :internal_urls, :external_urls, :failure_reporter, :checked_paths, :current_check
-    attr_accessor :current_path, :current_source
+    attr_reader :options, :cache, :logger, :internal_urls, :external_urls, :checked_paths, :current_check
+    attr_accessor :current_path, :current_source, :reporter
 
     URL_TYPES = %i[external internal].freeze
 
@@ -29,6 +29,8 @@ def initialize(src, opts = {})
       @current_check = nil
       @current_source = nil
       @current_path = nil
+
+      @reporter = Reporter::Cli.new(logger: @logger)
     end
 
     def run
@@ -46,13 +48,13 @@ def run
 
       @cache.write
 
-      @failure_reporter = FailureReporter.new(@failures, @logger)
+      @reporter.failures = @failures
 
       if @failures.empty?
         @logger.log :info, 'HTML-Proofer finished successfully.'
       else
         @failures.uniq!
-        print_failed_tests
+        report_failed_checks
       end
     end
 
@@ -185,12 +187,12 @@ def checks
       @checks
     end
 
-    def failed_tests
-      @failure_reporter.failures
+    def failed_checks
+      @reporter.failures.flatten.select { |f| f.is_a?(Failure) }
     end
 
-    def print_failed_tests
-      @failure_reporter.report(:cli)
+    def report_failed_checks
+      @reporter.report
 
       failure_text = pluralize(@failures.length, 'failure', 'failures')
       @logger.log :fatal, "\nHTML-Proofer found #{failure_text}!"

lib/html_proofer/url_validator.rb

@@ -10,7 +10,7 @@ def initialize(runner)
       @cache = @runner.cache
       @logger = @runner.logger
 
-      @failed_tests = []
+      @failed_checks = []
     end
   end
 end

lib/html_proofer/url_validator/external.rb

@@ -26,7 +26,7 @@ def validate
         run_external_link_checker(@external_urls)
       end
 
-      @failed_tests
+      @failed_checks
     end
 
     def remove_query_values(external_urls)
@@ -212,9 +212,9 @@ def handle_connection_failure(href, metadata, response_code, status_message)
 
     def add_failure(metadata, description, status = nil)
       if blank?(metadata) # possible if we're checking an array of links
-        @failed_tests << Failure.new('', 'Links > External', description, status: status)
+        @failed_checks << Failure.new('', 'Links > External', description, status: status)
       else
-        metadata.each { |m| @failed_tests << Failure.new(m[:filename], 'Links > External', description, line: m[:line], status: status) }
+        metadata.each { |m| @failed_checks << Failure.new(m[:filename], 'Links > External', description, line: m[:line], status: status) }
       end
     end
 

lib/html_proofer/url_validator/internal.rb

@@ -18,7 +18,7 @@ def validate
         run_internal_link_checker(@internal_urls)
       end
 
-      @failed_tests
+      @failed_checks
     end
 
     def run_internal_link_checker(links)
@@ -30,13 +30,13 @@ def run_internal_link_checker(links)
           @runner.current_path = metadata[:current_path]
 
           unless file_exists?(url)
-            @failed_tests << Failure.new(url.to_s, 'Links > Internal', "internally linking to #{url}, which does not exist", line: metadata[:line], status: nil, content: nil)
+            @failed_checks << Failure.new(@runner.current_path, 'Links > Internal', "internally linking to #{url}, which does not exist", line: metadata[:line], status: nil, content: nil)
             @cache.add_internal(url.to_s, metadata, false)
             next
           end
 
           unless hash_exists?(url)
-            @failed_tests << Failure.new(url.to_s, 'Links > Internal', "internally linking to #{url}; the file exists, but the hash does not", line: metadata[:line], status: nil, content: nil)
+            @failed_checks << Failure.new(@runner.current_path, 'Links > Internal', "internally linking to #{url}; the file exists, but the hash does not", line: metadata[:line], status: nil, content: nil)
             @cache.add_internal(url.to_s, metadata, false)
             next
           end
@@ -45,7 +45,7 @@ def run_internal_link_checker(links)
         end
       end
 
-      @failed_tests
+      @failed_checks
     end
 
     private def file_exists?(url)

spec/html-proofer/check_spec.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+class MailToOctocat < ::HTMLProofer::Check
+  def mailto_octocat?
+    @link.url.raw_attribute == 'mailto:octocat@github.com'
+  end
+
+  def run
+    @html.css('a').each do |node|
+      @link = create_element(node)
+
+      next if @link.ignore?
+
+      return add_failure("Don't email the Octocat directly!", line: @link.line) if mailto_octocat?
+    end
+  end
+end
+
+describe HTMLProofer::Reporter do
+  it 'supports a custom reporter' do
+    file = File.join(FIXTURES_DIR, 'links', 'mailto_octocat.html')
+    cassette_name = make_cassette_name(file, {})
+
+    VCR.use_cassette(cassette_name, record: :new_episodes) do
+      proofer = make_proofer(file, :file, { checks: ['MailToOctocat'] })
+      output = capture_stderr { proofer.run }
+      expect(output).to include("In #{file} (line 1)")
+    end
+  end
+end

spec/html-proofer/failure_reporter_spec.rb → spec/html-proofer/cli_reporter_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe HTMLProofer::FailureReporter do
+describe HTMLProofer::Reporter::Cli do
   describe 'cli_report' do
     it 'reports all issues accurately' do
       errors = File.join(FIXTURES_DIR, 'sorting', 'kitchen_sinkish.html')
@@ -67,7 +67,7 @@
 
         For the Links > Internal check, the following failures were found:
 
-        * In nowhere.fooof (line 24):
+        * In spec/html-proofer/fixtures/sorting/kitchen_sinkish.html (line 24):
 
            internally linking to nowhere.fooof, which does not exist
 

spec/html-proofer/element_spec.rb

@@ -49,7 +49,7 @@
     it 'does not explode if given a bad attribute' do
       broken_attribute = File.join(FIXTURES_DIR, 'html', 'invalid_attribute.html')
       proofer = run_proofer(broken_attribute, :file)
-      expect(proofer.failed_tests.length).to eq 0
+      expect(proofer.failed_checks.length).to eq 0
     end
   end
 end