Merge pull request #2631 from GauthamGoli/audit_homepage_rubocop1.2.1

audit: Port audit_homepage method to rubocop and add tests

6 files changed, 210 insertions, 143 deletions

diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml
index d77a8b100..12886a508 100644
--- a/Library/.rubocop.yml
+++ b/Library/.rubocop.yml
@@ -20,6 +20,9 @@ FormulaAuditStrict/ComponentsOrder:
 FormulaAuditStrict/ComponentsRedundancy:
   Enabled: true
 
+FormulaAudit/Homepage:
+  Enabled: true
+
 Metrics/AbcSize:
   Enabled: false
 
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index 9b14dc89c..cbe26b422 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -588,78 +588,9 @@ class FormulaAuditor
     homepage = formula.homepage
 
     if homepage.nil? || homepage.empty?
-      problem "Formula should have a homepage."
       return
     end
 
-    unless homepage =~ %r{^https?://}
-      problem "The homepage should start with http or https (URL is #{homepage})."
-    end
-
-    # Check for http:// GitHub homepage urls, https:// is preferred.
-    # Note: only check homepages that are repo pages, not *.github.com hosts
-    if homepage.start_with? "http://github.com/"
-      problem "Please use https:// for #{homepage}"
-    end
-
-    # Savannah has full SSL/TLS support but no auto-redirect.
-    # Doesn't apply to the download URLs, only the homepage.
-    if homepage.start_with? "http://savannah.nongnu.org/"
-      problem "Please use https:// for #{homepage}"
-    end
-
-    # Freedesktop is complicated to handle - It has SSL/TLS, but only on certain subdomains.
-    # To enable https Freedesktop change the URL from http://project.freedesktop.org/wiki to
-    # https://wiki.freedesktop.org/project_name.
-    # "Software" is redirected to https://wiki.freedesktop.org/www/Software/project_name
-    if homepage =~ %r{^http://((?:www|nice|libopenraw|liboil|telepathy|xorg)\.)?freedesktop\.org/(?:wiki/)?}
-      if homepage =~ /Software/
-        problem "#{homepage} should be styled `https://wiki.freedesktop.org/www/Software/project_name`"
-      else
-        problem "#{homepage} should be styled `https://wiki.freedesktop.org/project_name`"
-      end
-    end
-
-    # Google Code homepages should end in a slash
-    if homepage =~ %r{^https?://code\.google\.com/p/[^/]+[^/]$}
-      problem "#{homepage} should end with a slash"
-    end
-
-    # People will run into mixed content sometimes, but we should enforce and then add
-    # exemptions as they are discovered. Treat mixed content on homepages as a bug.
-    # Justify each exemptions with a code comment so we can keep track here.
-    case homepage
-    when %r{^http://[^/]*\.github\.io/},
-         %r{^http://[^/]*\.sourceforge\.io/}
-      problem "Please use https:// for #{homepage}"
-    end
-
-    if homepage =~ %r{^http://([^/]*)\.(sf|sourceforge)\.net(/|$)}
-      problem "#{homepage} should be `https://#{$1}.sourceforge.io/`"
-    end
-
-    # There's an auto-redirect here, but this mistake is incredibly common too.
-    # Only applies to the homepage and subdomains for now, not the FTP URLs.
-    if homepage =~ %r{^http://((?:build|cloud|developer|download|extensions|git|glade|help|library|live|nagios|news|people|projects|rt|static|wiki|www)\.)?gnome\.org}
-      problem "Please use https:// for #{homepage}"
-    end
-
-    # Compact the above into this list as we're able to remove detailed notations, etc over time.
-    case homepage
-    when %r{^http://[^/]*\.apache\.org},
-         %r{^http://packages\.debian\.org},
-         %r{^http://wiki\.freedesktop\.org/},
-         %r{^http://((?:www)\.)?gnupg\.org/},
-         %r{^http://ietf\.org},
-         %r{^http://[^/.]+\.ietf\.org},
-         %r{^http://[^/.]+\.tools\.ietf\.org},
-         %r{^http://www\.gnu\.org/},
-         %r{^http://code\.google\.com/},
-         %r{^http://bitbucket\.org/},
-         %r{^http://(?:[^/]*\.)?archive\.org}
-      problem "Please use https:// for #{homepage}"
-    end
-
     return unless @online
 
     return unless DevelopmentTools.curl_handles_most_https_homepages?
diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb
index 9dd365ee8..587bf4b20 100644
--- a/Library/Homebrew/rubocops.rb
+++ b/Library/Homebrew/rubocops.rb
@@ -2,3 +2,4 @@ require_relative "./rubocops/bottle_block_cop"
 require_relative "./rubocops/formula_desc_cop"
 require_relative "./rubocops/components_order_cop"
 require_relative "./rubocops/components_redundancy_cop"
+require_relative "./rubocops/homepage_cop"
diff --git a/Library/Homebrew/rubocops/homepage_cop.rb b/Library/Homebrew/rubocops/homepage_cop.rb
new file mode 100644
index 000000000..a40c7b049
--- /dev/null
+++ b/Library/Homebrew/rubocops/homepage_cop.rb
@@ -0,0 +1,82 @@
+require_relative "./extend/formula_cop"
+
+module RuboCop
+  module Cop
+    module FormulaAudit
+      # This cop audits `homepage` url in Formulae
+      class Homepage < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node)
+          homepage_node = find_node_method_by_name(formula_class_body_node, :homepage)
+          homepage = if homepage_node
+            string_content(parameters(homepage_node).first)
+          else
+            ""
+          end
+
+          problem "Formula should have a homepage." if homepage_node.nil? || homepage.empty?
+
+          unless homepage =~ %r{^https?://}
+            problem "The homepage should start with http or https (URL is #{homepage})."
+          end
+
+          case homepage
+          # Check for http:// GitHub homepage urls, https:// is preferred.
+          # Note: only check homepages that are repo pages, not *.github.com hosts
+          when %r{^http://github.com/}
+            problem "Please use https:// for #{homepage}"
+
+          # Savannah has full SSL/TLS support but no auto-redirect.
+          # Doesn't apply to the download URLs, only the homepage.
+          when %r{^http://savannah.nongnu.org/}
+            problem "Please use https:// for #{homepage}"
+
+          # Freedesktop is complicated to handle - It has SSL/TLS, but only on certain subdomains.
+          # To enable https Freedesktop change the URL from http://project.freedesktop.org/wiki to
+          # https://wiki.freedesktop.org/project_name.
+          # "Software" is redirected to https://wiki.freedesktop.org/www/Software/project_name
+          when %r{^http://((?:www|nice|libopenraw|liboil|telepathy|xorg)\.)?freedesktop\.org/(?:wiki/)?}
+            if homepage =~ /Software/
+              problem "#{homepage} should be styled `https://wiki.freedesktop.org/www/Software/project_name`"
+            else
+              problem "#{homepage} should be styled `https://wiki.freedesktop.org/project_name`"
+            end
+
+          # Google Code homepages should end in a slash
+          when %r{^https?://code\.google\.com/p/[^/]+[^/]$}
+            problem "#{homepage} should end with a slash"
+
+          # People will run into mixed content sometimes, but we should enforce and then add
+          # exemptions as they are discovered. Treat mixed content on homepages as a bug.
+          # Justify each exemptions with a code comment so we can keep track here.
+
+          when %r{^http://[^/]*\.github\.io/},
+               %r{^http://[^/]*\.sourceforge\.io/}
+            problem "Please use https:// for #{homepage}"
+
+          when %r{^http://([^/]*)\.(sf|sourceforge)\.net(/|$)}
+            problem "#{homepage} should be `https://#{$1}.sourceforge.io/`"
+
+          # There's an auto-redirect here, but this mistake is incredibly common too.
+          # Only applies to the homepage and subdomains for now, not the FTP URLs.
+          when %r{^http://((?:build|cloud|developer|download|extensions|git|glade|help|library|live|nagios|news|people|projects|rt|static|wiki|www)\.)?gnome\.org}
+            problem "Please use https:// for #{homepage}"
+
+          # Compact the above into this list as we're able to remove detailed notations, etc over time.
+          when %r{^http://[^/]*\.apache\.org},
+               %r{^http://packages\.debian\.org},
+               %r{^http://wiki\.freedesktop\.org/},
+               %r{^http://((?:www)\.)?gnupg\.org/},
+               %r{^http://ietf\.org},
+               %r{^http://[^/.]+\.ietf\.org},
+               %r{^http://[^/.]+\.tools\.ietf\.org},
+               %r{^http://www\.gnu\.org/},
+               %r{^http://code\.google\.com/},
+               %r{^http://bitbucket\.org/},
+               %r{^http://(?:[^/]*\.)?archive\.org}
+            problem "Please use https:// for #{homepage}"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb
index c914a9a20..b07ffaadc 100644
--- a/Library/Homebrew/test/dev-cmd/audit_spec.rb
+++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb
@@ -385,80 +385,6 @@ describe FormulaAuditor do
     end
   end
 
-  describe "#audit_homepage" do
-    specify "homepage URLs" do
-      fa = formula_auditor "foo", <<-EOS.undent, online: true
-        class Foo < Formula
-          homepage "ftp://example.com/foo"
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_homepage
-      expect(fa.problems)
-        .to eq(["The homepage should start with http or https (URL is #{fa.formula.homepage})."])
-
-      formula_homepages = {
-        "bar" => "http://www.freedesktop.org/wiki/bar",
-        "baz" => "http://www.freedesktop.org/wiki/Software/baz",
-        "qux" => "https://code.google.com/p/qux",
-        "quux" => "http://github.com/quux",
-        "corge" => "http://savannah.nongnu.org/corge",
-        "grault" => "http://grault.github.io/",
-        "garply" => "http://www.gnome.org/garply",
-        "sf1" => "http://foo.sourceforge.net/",
-        "sf2" => "http://foo.sourceforge.net",
-        "sf3" => "http://foo.sf.net/",
-        "sf4" => "http://foo.sourceforge.io/",
-        "waldo" => "http://www.gnu.org/waldo",
-      }
-
-      formula_homepages.each do |name, homepage|
-        fa = formula_auditor name, <<-EOS.undent
-          class #{Formulary.class_s(name)} < Formula
-            homepage "#{homepage}"
-            url "http://example.com/#{name}-1.0.tgz"
-          end
-        EOS
-
-        fa.audit_homepage
-        if homepage =~ %r{http:\/\/www\.freedesktop\.org}
-          if homepage =~ /Software/
-            expect(fa.problems.first).to match(
-              "#{homepage} should be styled " \
-              "`https://wiki.freedesktop.org/www/Software/project_name`",
-            )
-          else
-            expect(fa.problems.first).to match(
-              "#{homepage} should be styled " \
-              "`https://wiki.freedesktop.org/project_name`",
-            )
-          end
-        elsif homepage =~ %r{https:\/\/code\.google\.com}
-          expect(fa.problems.first)
-            .to match("#{homepage} should end with a slash")
-        elsif homepage =~ /foo\.(sf|sourceforge)\.net/
-          expect(fa.problems.first)
-            .to match("#{homepage} should be `https://foo.sourceforge.io/`")
-        else
-          expect(fa.problems.first)
-            .to match("Please use https:// for #{homepage}")
-        end
-      end
-    end
-
-    specify "missing homepage" do
-      fa = formula_auditor "foo", <<-EOS.undent, online: true
-        class Foo < Formula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_homepage
-      expect(fa.problems.first).to match("Formula should have a homepage.")
-    end
-  end
-
   describe "#audit_text" do
     specify "xcodebuild suggests symroot" do
       fa = formula_auditor "foo", <<-EOS.undent
diff --git a/Library/Homebrew/test/rubocops/homepage_cop_spec.rb b/Library/Homebrew/test/rubocops/homepage_cop_spec.rb
new file mode 100644
index 000000000..c03efd825
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/homepage_cop_spec.rb
@@ -0,0 +1,124 @@
+require "rubocop"
+require "rubocop/rspec/support"
+require_relative "../../extend/string"
+require_relative "../../rubocops/homepage_cop"
+
+describe RuboCop::Cop::FormulaAudit::Homepage do
+  subject(:cop) { described_class.new }
+
+  context "When auditing homepage" do
+    it "When there is no homepage" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+        end
+      EOS
+
+      expected_offenses = [{  message: "Formula should have a homepage.",
+                              severity: :convention,
+                              line: 1,
+                              column: 0,
+                              source: source }]
+
+      inspect_source(cop, source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "Homepage with ftp" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          homepage "ftp://example.com/foo"
+          url "http://example.com/foo-1.0.tgz"
+        end
+      EOS
+
+      expected_offenses = [{  message: "The homepage should start with http or https (URL is ftp://example.com/foo).",
+                              severity: :convention,
+                              line: 2,
+                              column: 2,
+                              source: source }]
+
+      inspect_source(cop, source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "Homepage URLs" do
+      formula_homepages = {
+        "bar" => "http://www.freedesktop.org/wiki/bar",
+        "baz" => "http://www.freedesktop.org/wiki/Software/baz",
+        "qux" => "https://code.google.com/p/qux",
+        "quux" => "http://github.com/quux",
+        "corge" => "http://savannah.nongnu.org/corge",
+        "grault" => "http://grault.github.io/",
+        "garply" => "http://www.gnome.org/garply",
+        "sf1" => "http://foo.sourceforge.net/",
+        "sf2" => "http://foo.sourceforge.net",
+        "sf3" => "http://foo.sf.net/",
+        "sf4" => "http://foo.sourceforge.io/",
+        "waldo" => "http://www.gnu.org/waldo",
+      }
+
+      formula_homepages.each do |name, homepage|
+        source = <<-EOS.undent
+          class #{name.capitalize} < Formula
+            homepage "#{homepage}"
+            url "http://example.com/#{name}-1.0.tgz"
+          end
+        EOS
+
+        inspect_source(cop, source)
+        if homepage =~ %r{http:\/\/www\.freedesktop\.org}
+          if homepage =~ /Software/
+            expected_offenses = [{  message: "#{homepage} should be styled " \
+                                             "`https://wiki.freedesktop.org/www/Software/project_name`",
+                                    severity: :convention,
+                                    line: 2,
+                                    column: 2,
+                                    source: source }]
+          else
+            expected_offenses = [{  message:  "#{homepage} should be styled " \
+                                              "`https://wiki.freedesktop.org/project_name`",
+                                    severity: :convention,
+                                    line: 2,
+                                    column: 2,
+                                    source: source }]
+          end
+        elsif homepage =~ %r{https:\/\/code\.google\.com}
+          expected_offenses = [{  message:  "#{homepage} should end with a slash",
+                                  severity: :convention,
+                                  line: 2,
+                                  column: 2,
+                                  source: source }]
+        elsif homepage =~ /foo\.(sf|sourceforge)\.net/
+          expected_offenses = [{  message:  "#{homepage} should be `https://foo.sourceforge.io/`",
+                                  severity: :convention,
+                                  line: 2,
+                                  column: 2,
+                                  source: source }]
+        else
+          expected_offenses = [{  message:  "Please use https:// for #{homepage}",
+                                  severity: :convention,
+                                  line: 2,
+                                  column: 2,
+                                  source: source }]
+        end
+        expected_offenses.zip([cop.offenses.last]).each do |expected, actual|
+          expect_offense(expected, actual)
+        end
+      end
+    end
+
+    def expect_offense(expected, actual)
+      expect(actual.message).to eq(expected[:message])
+      expect(actual.severity).to eq(expected[:severity])
+      expect(actual.line).to eq(expected[:line])
+      expect(actual.column).to eq(expected[:column])
+    end
+  end
+end