Skip to content

Commit

Permalink
Extend Check.add_failure to allow an Element argument
Browse files Browse the repository at this point in the history
* So we can extract `line` and `content` from there instead of explicitly passing them every time.
  • Loading branch information
riccardoporreca committed Mar 14, 2023
1 parent 40c72a4 commit 41af0a1
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 62 deletions.
6 changes: 3 additions & 3 deletions lib/html_proofer/check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ def run
raise NotImplementedError, "HTMLProofer::Check subclasses must implement #run"
end

def add_failure(description, line: nil, status: nil, content: nil)
def add_failure(description, element: nil, line: nil, status: nil, content: nil)
@failures << Failure.new(
@runner.current_filename,
short_name,
description,
line: line,
line: element.nil? ? line : element.line,
status: status,
content: content,
content: element.nil? ? content : element.content,
)
end

Expand Down
6 changes: 2 additions & 4 deletions lib/html_proofer/check/favicon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@ def run
if @favicon.url.protocol_relative?
add_failure(
"favicon link #{@favicon.url} is a protocol-relative URL, use explicit https:// instead",
line: @favicon.line,
content: @favicon.content,
element: @favicon,
)
elsif @favicon.url.remote?
add_to_external_urls(@favicon.url, @favicon.line)
elsif !@favicon.url.exists?
add_failure(
"internal favicon #{@favicon.url.raw_attribute} does not exist",
line: @favicon.line,
content: @favicon.content,
element: @favicon,
)
end
else
Expand Down
25 changes: 9 additions & 16 deletions lib/html_proofer/check/images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,23 @@ def run
# screenshot filenames should return because of terrible names
add_failure(
"image has a terrible filename (#{@img.url.raw_attribute})",
line: @img.line,
content: @img.content,
element: @img,
) if terrible_filename?

# does the image exist?
if missing_src?
add_failure("image has no src or srcset attribute", line: @img.line, content: @img.content)
add_failure("image has no src or srcset attribute", element: @img)
elsif @img.url.protocol_relative?
add_failure(
"image link #{@img.url} is a protocol-relative URL, use explicit https:// instead",
line: @img.line,
content: @img.content,
element: @img,
)
elsif @img.url.remote?
add_to_external_urls(@img.url, @img.line)
elsif !@img.url.exists? && !@img.multiple_srcsets? && !@img.multiple_sizes?
add_failure(
"internal image #{@img.url.raw_attribute} does not exist",
line: @img.line,
content: @img.content,
element: @img,
)
elsif @img.multiple_srcsets? || @img.multiple_sizes?
@img.srcsets_wo_sizes.each do |srcset|
Expand All @@ -42,13 +39,12 @@ def run
if srcset_url.protocol_relative?
add_failure(
"image link #{srcset_url.url} is a protocol-relative URL, use explicit https:// instead",
line: @img.line,
content: @img.content,
element: @img,
)
elsif srcset_url.remote?
add_to_external_urls(srcset_url.url, @img.line)
elsif !srcset_url.exists?
add_failure("internal image #{srcset} does not exist", line: @img.line, content: @img.content)
add_failure("internal image #{srcset} does not exist", element: @img)
end
end
end
Expand All @@ -58,22 +54,19 @@ def run
if missing_alt_tag? && !ignore_missing_alt?
add_failure(
"image #{@img.url.raw_attribute} does not have an alt attribute",
line: @img.line,
content: @img.content,
element: @img,
)
elsif (empty_alt_tag? || alt_all_spaces?) && !ignore_empty_alt?
add_failure(
"image #{@img.url.raw_attribute} has an alt attribute, but no content",
line: @img.line,
content: @img.content,
element: @img,
)
end
end

add_failure(
"image #{@img.url.raw_attribute} uses the http scheme",
line: @img.line,
content: @img.content,
element: @img,
) if @runner.enforce_https? && @img.url.http?
end

Expand Down
33 changes: 13 additions & 20 deletions lib/html_proofer/check/links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,28 @@ def run
next if @link.ignore?

if !allow_hash_href? && @link.node["href"] == "#"
add_failure("linking to internal hash #, which points to nowhere", line: @link.line, content: @link.content)
add_failure("linking to internal hash #, which points to nowhere", element: @link)
next
end

# is there even an href?
if blank?(@link.url.raw_attribute)
next if allow_missing_href?

add_failure("'#{@link.node.name}' tag is missing a reference", line: @link.line, content: @link.content)
add_failure("'#{@link.node.name}' tag is missing a reference", element: @link)
next
end

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

if @link.url.protocol_relative?
add_failure(
"#{@link.url} is a protocol-relative URL, use explicit https:// instead",
line: @link.line,
content: @link.content,
element: @link,
)
next
end
Expand All @@ -50,7 +49,7 @@ def run
next if @link.node["rel"] == "dns-prefetch"

unless @link.url.path?
add_failure("#{@link.url.raw_attribute} is an invalid URL", line: @link.line, content: @link.content)
add_failure("#{@link.url.raw_attribute} is an invalid URL", element: @link)
next
end

Expand All @@ -60,8 +59,7 @@ def run
if @link.url.unslashed_directory?(@link.url.absolute_path)
add_failure(
"internally linking to a directory #{@link.url.raw_attribute} without trailing slash",
line: @link.line,
content: @link.content,
element: @link,
)
next
end
Expand All @@ -88,31 +86,28 @@ def check_schemes
when "http"
return unless @runner.options[:enforce_https]

add_failure("#{@link.url.raw_attribute} is not an HTTPS link", line: @link.line, content: @link.content)
add_failure("#{@link.url.raw_attribute} is not an HTTPS link", element: @link)
end
end

def handle_mailto
if @link.url.path.empty?
add_failure(
"#{@link.url.raw_attribute} contains no email address",
line: @link.line,
content: @link.content,
element: @link,
) unless ignore_empty_mailto?
elsif !/#{URI::MailTo::EMAIL_REGEXP}/o.match?(@link.url.path)
add_failure(
"#{@link.url.raw_attribute} contains an invalid email address",
line: @link.line,
content: @link.content,
element: @link,
)
end
end

def handle_tel
add_failure(
"#{@link.url.raw_attribute} contains no phone number",
line: @link.line,
content: @link.content,
element: @link,
) if @link.url.path.empty?
end

Expand All @@ -130,16 +125,14 @@ def check_sri
if blank?(@link.node["integrity"]) && blank?(@link.node["crossorigin"])
add_failure(
"SRI and CORS not provided in: #{@link.url.raw_attribute}",
line: @link.line,
content: @link.content,
element: @link,
)
elsif blank?(@link.node["integrity"])
add_failure("Integrity is missing in: #{@link.url.raw_attribute}", line: @link.line, content: @link.content)
add_failure("Integrity is missing in: #{@link.url.raw_attribute}", element: @link)
elsif blank?(@link.node["crossorigin"])
add_failure(
"CORS not provided for external resource in: #{@link.link.url.raw_attribute}",
line: @link.line,
content: @link.content,
element: @link,
)
end
end
Expand Down
12 changes: 5 additions & 7 deletions lib/html_proofer/check/open_graph.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,22 @@ def run

# does the open_graph exist?
if missing_content?
add_failure("open graph has no content attribute", line: @open_graph.line, content: @open_graph.content)
add_failure("open graph has no content attribute", element: @open_graph)
elsif empty_content?
add_failure("open graph content attribute is empty", line: @open_graph.line, content: @open_graph.content)
add_failure("open graph content attribute is empty", element: @open_graph)
elsif !@open_graph.url.valid?
add_failure("#{@open_graph.src} is an invalid URL", line: @open_graph.line)
add_failure("#{@open_graph.src} is an invalid URL", element: @open_graph)
elsif @open_graph.url.protocol_relative?
add_failure(
"open graph link #{@open_graph.url} is a protocol-relative URL, use explicit https:// instead",
line: @open_graph.line,
content: @open_graph.content,
element: @open_graph,
)
elsif @open_graph.url.remote?
add_to_external_urls(@open_graph.url, @open_graph.line)
else
add_failure(
"internal open graph #{@open_graph.url.raw_attribute} does not exist",
line: @open_graph.line,
content: @open_graph.content,
element: @open_graph,
) unless @open_graph.url.exists?
end
end
Expand Down
17 changes: 6 additions & 11 deletions lib/html_proofer/check/scripts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,19 @@ def run

# does the script exist?
if missing_src?
add_failure("script is empty and has no src attribute", line: @script.line, content: @script.content)
add_failure("script is empty and has no src attribute", element: @script)
elsif @script.url.protocol_relative?
add_failure(
"script link #{@script.url} is a protocol-relative URL, use explicit https:// instead",
line: @script.line,
content: @script.content,
element: @script,
)
elsif @script.url.remote?
add_to_external_urls(@script.url, @script.line)
check_sri if @runner.check_sri?
elsif !@script.url.exists?
add_failure(
"internal script reference #{@script.src} does not exist",
line: @script.line,
content: @script.content,
element: @script,
)
end
end
Expand All @@ -42,20 +40,17 @@ def check_sri
if blank?(@script.node["integrity"]) && blank?(@script.node["crossorigin"])
add_failure(
"SRI and CORS not provided in: #{@script.url.raw_attribute}",
line: @script.line,
content: @script.content,
element: @script,
)
elsif blank?(@script.node["integrity"])
add_failure(
"Integrity is missing in: #{@script.url.raw_attribute}",
line: @script.line,
content: @script.content,
element: @script,
)
elsif blank?(@script.node["crossorigin"])
add_failure(
"CORS not provided for external resource in: #{@script.url.raw_attribute}",
line: @script.line,
content: @script.content,
element: @script,
)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/html-proofer/check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def run

next if @link.ignore?

return add_failure("Don't email the Octocat directly!", line: @link.line) if mailto_octocat?
return add_failure("Don't email the Octocat directly!", element: @link) if mailto_octocat?
end
end
end
Expand Down

0 comments on commit 41af0a1

Please sign in to comment.