Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic line number support #154

Merged
merged 7 commits into from
Jan 29, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/html/proofer/check_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def run
fail NotImplementedError, 'HTML::Proofer::CheckRunner subclasses must implement #run'
end

def add_issue(desc, status = -1)
@issues << Issue.new(@path, desc, status)
def add_issue(desc, line_number = nil, status = -1)
@issues << Issue.new(@path, desc, line_number, status)
end

def add_to_external_urls(href)
Expand Down
9 changes: 5 additions & 4 deletions lib/html/proofer/check_runner/issue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
class HTML::Proofer::CheckRunner

class Issue
attr_reader :path, :desc, :status
attr_reader :path, :desc, :status, :line_number

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

def to_s
"#{@path}: #{desc}"
"#{@path}: #{@desc}#{@line_number}"
end
end

Expand Down Expand Up @@ -53,7 +54,7 @@ def report(sorted_issues, first_report, second_report)
if first_report == :status
@logger.log :error, :red, " * #{issue}"
else
@logger.log :error, :red, " * #{issue.send(second_report)}"
@logger.log :error, :red, " * #{issue.send(second_report)}#{issue.line_number}"
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/html/proofer/checkable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Proofer
# Represents the superclass from which all checks derive.
class Checkable
include HTML::Utils
attr_reader :line

def initialize(obj, check)
obj.attributes.each_pair do |attribute, value|
Expand All @@ -18,6 +19,7 @@ def initialize(obj, check)
@check = check
@checked_paths = {}
@type = self.class.name
@line = obj.line

if @href && @check.options[:href_swap]
@href = swap(@href, @check.options[:href_swap])
Expand Down
8 changes: 4 additions & 4 deletions lib/html/proofer/checks/images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ def run
next if img.ignore?

# screenshot filenames should return because of terrible names
next add_issue "image has a terrible filename (#{img.src})" if img.terrible_filename?
next add_issue("image has a terrible filename (#{img.src})", i.line) if img.terrible_filename?

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

# check alt tag
add_issue "image #{img.src} does not have an alt attribute" unless img.valid_alt_tag?
add_issue("image #{img.src} does not have an alt attribute", i.line) unless img.valid_alt_tag?
end

external_urls
Expand Down
20 changes: 10 additions & 10 deletions lib/html/proofer/checks/links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,22 @@ def run

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

if link.scheme == 'mailto'
add_issue "#{link.href} contains no email address" if link.path.empty?
add_issue "#{link.href} contain an invalid email address" unless link.path.include?('@')
add_issue("#{link.href} contains no email address", l.line) if link.path.empty?
add_issue("#{link.href} contain an invalid email address", l.line) unless link.path.include?('@')
end

if link.scheme == 'tel'
add_issue "#{link.href} contains no phone number" if link.path.empty?
add_issue("#{link.href} contains no phone number", l.line) if link.path.empty?
end

# is there even a href?
if link.missing_href?
add_issue('anchor has no href attribute')
add_issue('anchor has no href attribute', l.line)
next
end

Expand All @@ -63,20 +63,20 @@ def run
add_to_external_urls link.href
next
elsif !link.internal?
add_issue "internally linking to #{link.href}, which does not exist" unless link.exists?
add_issue("internally linking to #{link.href}, which does not exist", l.line) unless link.exists?
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")
add_issue("internally linking to a directory #{link.absolute_path} without trailing slash", l.line)
next
end

# verify the target hash
if link.hash
if link.internal?
unless hash_check @html, link.hash
add_issue "linking to internal hash ##{link.hash} that does not exist"
add_issue("linking to internal hash ##{link.hash} that does not exist", l.line)
end
elsif link.external?
external_link_check(link)
Expand All @@ -89,11 +89,11 @@ def run

def external_link_check(link)
if !link.exists?
add_issue "trying to find hash of #{link.href}, but #{link.absolute_path} does not exist"
add_issue("trying to find hash of #{link.href}, but #{link.absolute_path} does not exist", l.line)
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"
add_issue("linking to #{link.href}, but #{link.hash} does not exist", link.line)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/html/proofer/checks/scripts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ def run

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

Expand Down
4 changes: 2 additions & 2 deletions lib/html/proofer/url_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ def handle_timeout

def add_failed_tests(filenames, desc, status = nil)
if filenames.nil?
@failed_tests << CheckRunner::Issue.new('', desc, status)
@failed_tests << CheckRunner::Issue.new('', desc, nil, status)
else
filenames.each { |f| @failed_tests << CheckRunner::Issue.new(f, desc, status) }
filenames.each { |f| @failed_tests << CheckRunner::Issue.new(f, desc, nil, status) }
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/html/proofer/scripts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
it 'fails for missing internal src' do
file = "#{FIXTURES_DIR}/scripts/script_missing_internal.html"
proofer = run_proofer(file)
expect(proofer.failed_tests.first).to match(/doesnotexist.js does not exist/)
expect(proofer.failed_tests.first).to match(/doesnotexist.js does not exist \(line 5\)/)
end

it 'works for present content' do
Expand Down
22 changes: 11 additions & 11 deletions spec/html/proofer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
it 'is a list of the formatted errors' do
brokenLinkInternalFilepath = "#{FIXTURES_DIR}/links/brokenLinkInternal.html"
proofer = run_proofer(brokenLinkInternalFilepath)
expect(proofer.failed_tests).to eq(["spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./notreal.html, which does not exist", "spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./missingImageAlt.html, which does not exist"])
expect(proofer.failed_tests).to eq(["spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./notreal.html, which does not exist (line 5)", "spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./missingImageAlt.html, which does not exist (line 6)"])
end
end

Expand Down Expand Up @@ -40,25 +40,25 @@

expect(output.strip).to eq('''
- spec/html/proofer/fixtures/sorting/path/multiple_issues.html
* image gpl.png does not have an alt attribute
* internal image gpl.png does not exist
* tel: contains no phone number
* 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)
- 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)
* image has a terrible filename (./Screen Shot 2012-08-09 at 7.51.18 AM.png) (line 1)
'''.strip)
end

it 'understands sorting by issue' do
output = send_proofer_output("#{FIXTURES_DIR}/sorting/issue", :error_sort => :desc)
expect(output.strip).to eq('''
- image ./gpl.png does not have an alt attribute
* spec/html/proofer/fixtures/sorting/issue/broken_image_one.html
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html
* spec/html/proofer/fixtures/sorting/issue/broken_image_one.html (line 1)
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html (line 1)
- internal image ./gpl.png does not exist
* spec/html/proofer/fixtures/sorting/issue/broken_image_one.html
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html
* spec/html/proofer/fixtures/sorting/issue/broken_image_one.html (line 1)
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html (line 1)
- internal image NOT_AN_IMAGE does not exist
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html
* spec/html/proofer/fixtures/sorting/issue/broken_image_two.html (line 4)
'''.strip)
end

Expand All @@ -67,7 +67,7 @@
output = send_proofer_output("#{FIXTURES_DIR}/sorting/status", :typhoeus => { :followlocation => false }, :error_sort => :status)
expect(output.strip).to eq('''
- -1
* spec/html/proofer/fixtures/sorting/status/broken_link.html: internally linking to nowhere.fooof, which does not exist
* spec/html/proofer/fixtures/sorting/status/broken_link.html: internally linking to nowhere.fooof, which does not exist (line 3)
- 404
* spec/html/proofer/fixtures/sorting/status/a_404.html: External link http://upload.wikimedia.org/wikipedia/en/thumb/not_here.png failed: 404 No error
* spec/html/proofer/fixtures/sorting/status/broken_link.html: External link http://upload.wikimedia.org/wikipedia/en/thumb/fooooof.png failed: 404 No error
Expand Down