Skip to content

Commit

Permalink
Merge pull request #356 from gjtorikian/add-content-to-error
Browse files Browse the repository at this point in the history
Report on the content of the error, when able
  • Loading branch information
gjtorikian authored Sep 22, 2016
2 parents df42f37 + 677c16d commit d9093e2
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 41 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ HTML-Proofer can be as noisy or as quiet as you'd like. If you set the `:log_lev

Want to write your own test? Sure, that's possible!

Just create a class that inherits from `HTMLProofer::Check`. This subclass must define one method called `run`. This is called on your content, and is responsible for performing the validation on whatever elements you like. When you catch a broken issue, call `add_issue(message, line: line)` to explain the error.
Just create a class that inherits from `HTMLProofer::Check`. This subclass must define one method called `run`. This is called on your content, and is responsible for performing the validation on whatever elements you like. When you catch a broken issue, call `add_issue(message, line: line, content: content)` to explain the error. `line` refers to the line numbers, and `content` is the node content of the broken element.

If you're working with the element's attributes (as most checks do), you'll also want to call `create_element(node)` as part of your suite. This constructs an object that contains all the attributes of the HTML element you're iterating on.

Expand Down
8 changes: 4 additions & 4 deletions lib/html-proofer/check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ def create_element(node)
end

def run
fail NotImplementedError, 'HTMLProofer::Check subclasses must implement #run'
raise NotImplementedError, 'HTMLProofer::Check subclasses must implement #run'
end

def add_issue(desc, line: nil, status: -1)
@issues << Issue.new(@path, desc, line: line, status: status)
def add_issue(desc, line: nil, status: -1, content: nil)
@issues << Issue.new(@path, desc, line: line, status: status, content: content)
end

def add_to_external_urls(url, _)
def add_to_external_urls(url)
return if @external_urls[url]
add_path_for_url(url)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/html-proofer/check/favicon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ def run

return if found

add_issue 'no favicon specified'
add_issue('no favicon specified')
end
end
13 changes: 7 additions & 6 deletions lib/html-proofer/check/images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,33 @@ def run
@html.css('img').each do |node|
@img = create_element(node)
line = node.line
content = node.content

next if @img.ignore?

# screenshot filenames should return because of terrible names
if terrible_filename?
add_issue("image has a terrible filename (#{@img.url})", line: line)
add_issue("image has a terrible filename (#{@img.url})", line: line, content: content)
next
end

# does the image exist?
if missing_src?
add_issue('image has no src or srcset attribute', line: line)
add_issue('image has no src or srcset attribute', line: line, content: content)
else
if @img.remote?
add_to_external_urls(@img.url, line)
add_to_external_urls(@img.url)
elsif !@img.exists?
add_issue("internal image #{@img.url} does not exist", line: line)
add_issue("internal image #{@img.url} does not exist", line: line, content: content)
end
end

if !@img.ignore_alt? && (@img.alt.nil? || (empty_alt_tag? && !@img.ignore_empty_alt?))
add_issue("image #{@img.url} does not have an alt attribute", line: line)
add_issue("image #{@img.url} does not have an alt attribute", line: line, content: content)
end

if @img.check_img_http? && @img.scheme == 'http'
add_issue("image #{@img.url} uses the http scheme", line: line)
add_issue("image #{@img.url} uses the http scheme", line: line, content: content)
end
end

Expand Down
47 changes: 24 additions & 23 deletions lib/html-proofer/check/links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def placeholder?
def run
@html.css('a, link').each do |node|
@link = create_element(node)
line = @node.line
line = node.line
content = node.to_s

next if @link.ignore?

Expand All @@ -21,84 +22,84 @@ def run

# is it even a valid URL?
unless @link.valid?
add_issue("#{@link.href} is an invalid URL", line: line)
add_issue("#{@link.href} is an invalid URL", line: line, content: content)
next
end

check_schemes(@link, line)
check_schemes(@link, line, content)

# is there even an href?
if missing_href?
# HTML5 allows dropping the href: http://git.io/vBX0z
next if @html.internal_subset.name == 'html' && @html.internal_subset.external_id.nil?
add_issue('anchor has no href attribute', line: line)
add_issue('anchor has no href attribute', line: line, content: content)
next
end

# intentionally here because we still want valid? & missing_href? to execute
next if @link.non_http_remote?
# does the file even exist?
if @link.remote?
add_to_external_urls(@link.href, line)
add_to_external_urls(@link.href)
next
elsif !@link.internal? && !@link.exists?
add_issue("internally linking to #{@link.href}, which does not exist", line: line)
add_issue("internally linking to #{@link.href}, which does not exist", line: line, content: content)
end

# does the local directory have a trailing slash?
if @link.unslashed_directory? @link.absolute_path
add_issue("internally linking to a directory #{@link.absolute_path} without trailing slash", line: line)
add_issue("internally linking to a directory #{@link.absolute_path} without trailing slash", line: line, content: content)
next
end

# verify the target hash
handle_hash(@link, line) if @link.hash
handle_hash(@link, line, content) if @link.hash
end

external_urls
end

def check_schemes(link, line)
def check_schemes(link, line, content)
case link.scheme
when 'mailto'
handle_mailto(link, line)
handle_mailto(link, line, content)
when 'tel'
handle_tel(link, line)
handle_tel(link, line, content)
when 'http'
return unless @options[:enforce_https]
add_issue("#{link.href} is not an HTTPS link", line: line)
add_issue("#{link.href} is not an HTTPS link", line: line, content: content)
end
end

def handle_mailto(link, line)
def handle_mailto(link, line, content)
if link.path.empty?
add_issue("#{link.href} contains no email address", line: line)
add_issue("#{link.href} contains no email address", line: line, content: content)
elsif !link.path.include?('@')
add_issue("#{link.href} contains an invalid email address", line: line)
add_issue("#{link.href} contains an invalid email address", line: line, content: content)
end
end

def handle_tel(link, line)
add_issue("#{link.href} contains no phone number", line: line) if link.path.empty?
def handle_tel(link, line, content)
add_issue("#{link.href} contains no phone number", line: line, content: content) if link.path.empty?
end

def handle_hash(link, line)
def handle_hash(link, line, content)
if link.internal?
unless hash_check @html, link.hash
add_issue("linking to internal hash ##{link.hash} that does not exist", line: line)
add_issue("linking to internal hash ##{link.hash} that does not exist", line: line, content: content)
end
elsif link.external?
external_link_check(link, line)
external_link_check(link, line, content)
end
end

def external_link_check(link, line)
def external_link_check(link, line, content)
if !link.exists?
add_issue("trying to find hash of #{link.href}, but #{link.absolute_path} does not exist", line: line)
add_issue("trying to find hash of #{link.href}, but #{link.absolute_path} does not exist", line: line, content: content)
else
target_html = create_nokogiri link.absolute_path
unless hash_check target_html, link.hash
add_issue("linking to #{link.href}, but #{link.hash} does not exist", line: line)
add_issue("linking to #{link.href}, but #{link.hash} does not exist", line: line, content: content)
end
end
end
Expand Down
7 changes: 4 additions & 3 deletions lib/html-proofer/check/scripts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,18 @@ def run
@html.css('script').each do |node|
@script = create_element(node)
line = node.line
content = node.content

next if @script.ignore?
next unless node.text.strip.empty?

# does the script exist?
if missing_src?
add_issue('script is empty and has no src attribute', line: line)
add_issue('script is empty and has no src attribute', line: line, content: content)
elsif @script.remote?
add_to_external_urls(@script.src, line)
add_to_external_urls(@script.src)
elsif !@script.exists?
add_issue("internal script #{@script.src} does not exist", line: line)
add_issue("internal script #{@script.src} does not exist", line: line, content: content)
end
end

Expand Down
11 changes: 8 additions & 3 deletions lib/html-proofer/issue.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
module HTMLProofer
class Issue
attr_reader :path, :desc, :status, :line
attr_reader :path, :desc, :status, :line, :content

def initialize(path, desc, line: nil, status: -1)
def initialize(path, desc, line: nil, status: -1, content: nil)
@line = line.nil? ? '' : " (line #{line})"
@path = path
@desc = desc
@status = status
@content = content
end

def to_s
Expand Down Expand Up @@ -52,7 +53,11 @@ def report(sorted_issues, first_report, second_report)
if first_report == :status
@logger.log :error, " * #{issue}"
else
@logger.log :error, " * #{issue.send(second_report)}#{issue.line}"
msg = " * #{issue.send(second_report)}#{issue.line}"
if !issue.content.nil? && !issue.content.empty?
msg = "#{msg}\n #{issue.content}"
end
@logger.log(:error, msg)
end
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/html-proofer/proofer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
* image gpl.png does not have an alt attribute (line 7)
* internal image gpl.png does not exist (line 7)
* tel: contains no phone number (line 5)
<a href="tel:">Tel me</a>
- spec/html-proofer/fixtures/sorting/path/single_issue.html
* image has a terrible filename (./Screen Shot 2012-08-09 at 7.51.18 AM.png) (line 1)
'''.strip)
Expand Down

0 comments on commit d9093e2

Please sign in to comment.