From 34f3740695fa3d3fe7438a5b9270a6f18b1b1ec1 Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 00:09:54 -0800
Subject: [PATCH 1/7] Add basic line number support

---
 lib/html/proofer/check_runner.rb       |  4 ++--
 lib/html/proofer/check_runner/issue.rb |  7 ++++---
 lib/html/proofer/checks/images.rb      |  8 ++++----
 lib/html/proofer/checks/links.rb       | 20 ++++++++++----------
 lib/html/proofer/checks/scripts.rb     |  4 ++--
 lib/html/proofer/url_validator.rb      |  4 ++--
 spec/html/proofer/scripts_spec.rb      |  2 +-
 7 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/lib/html/proofer/check_runner.rb b/lib/html/proofer/check_runner.rb
index 3e588e18..cea1c359 100644
--- a/lib/html/proofer/check_runner.rb
+++ b/lib/html/proofer/check_runner.rb
@@ -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)
diff --git a/lib/html/proofer/check_runner/issue.rb b/lib/html/proofer/check_runner/issue.rb
index 2956d11b..828fd01d 100644
--- a/lib/html/proofer/check_runner/issue.rb
+++ b/lib/html/proofer/check_runner/issue.rb
@@ -2,11 +2,12 @@
 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 #{line_number || 'not given'})"
       @path = path
-      @desc = desc
+      @desc = desc << @line_number
       @status = status
     end
 
diff --git a/lib/html/proofer/checks/images.rb b/lib/html/proofer/checks/images.rb
index 33dc96be..6b42f6d3 100644
--- a/lib/html/proofer/checks/images.rb
+++ b/lib/html/proofer/checks/images.rb
@@ -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
diff --git a/lib/html/proofer/checks/links.rb b/lib/html/proofer/checks/links.rb
index e4a34565..557f3eca 100644
--- a/lib/html/proofer/checks/links.rb
+++ b/lib/html/proofer/checks/links.rb
@@ -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
 
@@ -63,12 +63,12 @@ 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
 
@@ -76,7 +76,7 @@ def run
       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)
@@ -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", l.line)
       end
     end
   end
diff --git a/lib/html/proofer/checks/scripts.rb b/lib/html/proofer/checks/scripts.rb
index 172af5d9..5b934d25 100644
--- a/lib/html/proofer/checks/scripts.rb
+++ b/lib/html/proofer/checks/scripts.rb
@@ -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
 
diff --git a/lib/html/proofer/url_validator.rb b/lib/html/proofer/url_validator.rb
index c2bfff43..79c393ca 100644
--- a/lib/html/proofer/url_validator.rb
+++ b/lib/html/proofer/url_validator.rb
@@ -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
 
diff --git a/spec/html/proofer/scripts_spec.rb b/spec/html/proofer/scripts_spec.rb
index edc3ce76..43bcdd07 100644
--- a/spec/html/proofer/scripts_spec.rb
+++ b/spec/html/proofer/scripts_spec.rb
@@ -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

From d9867867db538c1964b810b16e8c423f897abd7a Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 08:45:23 -0800
Subject: [PATCH 2/7] Fix some broken test output

---
 lib/html/proofer/checks/images.rb |  2 +-
 lib/html/proofer/checks/links.rb  |  2 +-
 spec/html/proofer_spec.rb         | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/html/proofer/checks/images.rb b/lib/html/proofer/checks/images.rb
index 6b42f6d3..0bb84ffd 100644
--- a/lib/html/proofer/checks/images.rb
+++ b/lib/html/proofer/checks/images.rb
@@ -34,7 +34,7 @@ def run
 
       # does the image exist?
       if img.missing_src?
-        add_issue 'image has no src or srcset attribute', i.line
+        add_issue('image has no src or srcset attribute', i.line)
       else
         if img.remote?
           add_to_external_urls img.src
diff --git a/lib/html/proofer/checks/links.rb b/lib/html/proofer/checks/links.rb
index 557f3eca..49f47361 100644
--- a/lib/html/proofer/checks/links.rb
+++ b/lib/html/proofer/checks/links.rb
@@ -93,7 +93,7 @@ def external_link_check(link)
     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", l.line)
+        add_issue("linking to #{link.href}, but #{link.hash} does not exist", link.line)
       end
     end
   end
diff --git a/spec/html/proofer_spec.rb b/spec/html/proofer_spec.rb
index 91d47661..343d1c65 100644
--- a/spec/html/proofer_spec.rb
+++ b/spec/html/proofer_spec.rb
@@ -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", "spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./missingImageAlt.html (line 6), which does not exist"])
     end
   end
 
@@ -40,11 +40,11 @@
 
         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
 

From 1e009b56f046dfb9211b665f6121230990b03ee0 Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 08:52:26 -0800
Subject: [PATCH 3/7] Give `line` to every Checkable

---
 lib/html/proofer/checkable.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/html/proofer/checkable.rb b/lib/html/proofer/checkable.rb
index 5a96739b..189cc7f4 100644
--- a/lib/html/proofer/checkable.rb
+++ b/lib/html/proofer/checkable.rb
@@ -18,6 +18,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])

From 434995f718dfb5bc279502741e27e241f0d5b6a6 Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 08:52:34 -0800
Subject: [PATCH 4/7] Correct a typo

---
 spec/html/proofer_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/html/proofer_spec.rb b/spec/html/proofer_spec.rb
index 343d1c65..33c706ca 100644
--- a/spec/html/proofer_spec.rb
+++ b/spec/html/proofer_spec.rb
@@ -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 (line 3), which does not exist
 - 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

From 6a6f6af36b7effbb9e188ff58887013a76173cfa Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 09:25:11 -0800
Subject: [PATCH 5/7] Clean up the line number concatenation

---
 lib/html/proofer/check_runner/issue.rb |  8 ++++----
 lib/html/proofer/checkable.rb          |  1 +
 spec/html/proofer_spec.rb              | 12 ++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/lib/html/proofer/check_runner/issue.rb b/lib/html/proofer/check_runner/issue.rb
index 828fd01d..dc4d3837 100644
--- a/lib/html/proofer/check_runner/issue.rb
+++ b/lib/html/proofer/check_runner/issue.rb
@@ -5,9 +5,9 @@ class Issue
     attr_reader :path, :desc, :status, :line_number
 
     def initialize(path, desc, line_number = nil, status = -1)
-      @line_number = " (line #{line_number || 'not given'})"
+      @line_number = line_number.nil? ? '' : " (line #{line_number})"
       @path = path
-      @desc = desc << @line_number
+      @desc = desc
       @status = status
     end
 
@@ -52,9 +52,9 @@ def report(sorted_issues, first_report, second_report)
           matcher = issue.send(first_report)
         end
         if first_report == :status
-          @logger.log :error, :red, "  *  #{issue}"
+          @logger.log :error, :red, "  *  #{issue}#{issue.line_number}"
         else
-          @logger.log :error, :red, "  *  #{issue.send(second_report)}"
+          @logger.log :error, :red, "  *  #{issue.send(second_report)}#{issue.line_number}"
         end
       end
     end
diff --git a/lib/html/proofer/checkable.rb b/lib/html/proofer/checkable.rb
index 189cc7f4..cb5996e9 100644
--- a/lib/html/proofer/checkable.rb
+++ b/lib/html/proofer/checkable.rb
@@ -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|
diff --git a/spec/html/proofer_spec.rb b/spec/html/proofer_spec.rb
index 33c706ca..449cc4d6 100644
--- a/spec/html/proofer_spec.rb
+++ b/spec/html/proofer_spec.rb
@@ -52,13 +52,13 @@
         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
 
@@ -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 (line 3), 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

From 0c744db6d107ca1359c48afddf1a553b72a79bca Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 09:35:59 -0800
Subject: [PATCH 6/7] Correct the to_s method

---
 lib/html/proofer/check_runner/issue.rb | 2 +-
 spec/html/proofer_spec.rb              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/html/proofer/check_runner/issue.rb b/lib/html/proofer/check_runner/issue.rb
index dc4d3837..9c704d6d 100644
--- a/lib/html/proofer/check_runner/issue.rb
+++ b/lib/html/proofer/check_runner/issue.rb
@@ -12,7 +12,7 @@ def initialize(path, desc, line_number = nil, status = -1)
     end
 
     def to_s
-      "#{@path}: #{desc}"
+      "#{@path}: #{@desc}#{@line_number}"
     end
   end
 
diff --git a/spec/html/proofer_spec.rb b/spec/html/proofer_spec.rb
index 449cc4d6..e5a874ad 100644
--- a/spec/html/proofer_spec.rb
+++ b/spec/html/proofer_spec.rb
@@ -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 (line 6), 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", "spec/html/proofer/fixtures/links/brokenLinkInternal.html: internally linking to ./missingImageAlt.html, which does not exist (line 6)"])
     end
   end
 

From 5d0157575e79d9166ce0f204b7e34ce769d391b6 Mon Sep 17 00:00:00 2001
From: Garen Torikian <gjtorikian@gmail.com>
Date: Thu, 29 Jan 2015 09:43:29 -0800
Subject: [PATCH 7/7] Correct erroneous duplicates

---
 lib/html/proofer/check_runner/issue.rb | 2 +-
 spec/html/proofer_spec.rb              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/html/proofer/check_runner/issue.rb b/lib/html/proofer/check_runner/issue.rb
index 9c704d6d..6afc0271 100644
--- a/lib/html/proofer/check_runner/issue.rb
+++ b/lib/html/proofer/check_runner/issue.rb
@@ -52,7 +52,7 @@ def report(sorted_issues, first_report, second_report)
           matcher = issue.send(first_report)
         end
         if first_report == :status
-          @logger.log :error, :red, "  *  #{issue}#{issue.line_number}"
+          @logger.log :error, :red, "  *  #{issue}"
         else
           @logger.log :error, :red, "  *  #{issue.send(second_report)}#{issue.line_number}"
         end
diff --git a/spec/html/proofer_spec.rb b/spec/html/proofer_spec.rb
index e5a874ad..bf42343a 100644
--- a/spec/html/proofer_spec.rb
+++ b/spec/html/proofer_spec.rb
@@ -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 (line 6)"])
+      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