From 92e07f5c0b0c2b7f3200e30097472b56acbcf46e Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 15:58:52 +1100 Subject: [PATCH] rubocops/homepage: use rubocop v1 API --- Library/Homebrew/rubocops/homepage.rb | 62 ++--- .../Homebrew/test/rubocops/homepage_spec.rb | 225 ++++++++++-------- 2 files changed, 164 insertions(+), 123 deletions(-) diff --git a/Library/Homebrew/rubocops/homepage.rb b/Library/Homebrew/rubocops/homepage.rb index da27f11f70..928e30e717 100644 --- a/Library/Homebrew/rubocops/homepage.rb +++ b/Library/Homebrew/rubocops/homepage.rb @@ -8,19 +8,23 @@ module RuboCop module FormulaAudit # This cop audits the `homepage` URL in formulae. class Homepage < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) homepage_node = find_node_method_by_name(body_node, :homepage) - homepage = if homepage_node - string_content(parameters(homepage_node).first) - else - "" + + if homepage_node.nil? + problem "Formula should have a homepage." + return end - problem "Formula should have a homepage." if homepage_node.nil? || homepage.empty? + homepage_parameter_node = parameters(homepage_node).first + offending_node(homepage_parameter_node) + homepage = string_content(homepage_parameter_node) - unless homepage.match?(%r{^https?://}) - problem "The homepage should start with http or https (URL is #{homepage})." - end + problem "Formula should have a homepage." if homepage.empty? + + problem "The homepage should start with http or https." unless homepage.match?(%r{^https?://}) case homepage # Freedesktop is complicated to handle - It has SSL/TLS, but only on certain subdomains. @@ -29,25 +33,34 @@ module RuboCop # "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.include?("Software") - problem "#{homepage} should be styled `https://wiki.freedesktop.org/www/Software/project_name`" + problem "Freedesktop homepages should be styled "\ + "`https://wiki.freedesktop.org/www/Software/project_name`" else - problem "#{homepage} should be styled `https://wiki.freedesktop.org/project_name`" + problem "Freedesktop homepages 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" + problem "Google Code homepages should end with a slash" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage}/\"") + end when %r{^http://([^/]*)\.(sf|sourceforge)\.net(/|$)} - problem "#{homepage} should be `https://#{Regexp.last_match(1)}.sourceforge.io/`" + fixed = "https://#{Regexp.last_match(1)}.sourceforge.io/" + problem "Sourceforge homepages should be `#{fixed}`" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{fixed}\"") + end when /readthedocs\.org/ - offending_node(parameters(homepage_node).first) - problem "#{homepage} should be `#{homepage.sub("readthedocs.org", "readthedocs.io")}`" + fixed = homepage.sub("readthedocs.org", "readthedocs.io") + problem "Readthedocs homepages should be `#{fixed}`" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{fixed}\"") + end when %r{^https://github.com.*\.git} - offending_node(parameters(homepage_node).first) - problem "GitHub homepages (`#{homepage}`) should not end with .git" + problem "GitHub homepages should not end with .git" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage.delete_suffix(".git")}\"") + 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. @@ -81,20 +94,9 @@ module RuboCop %r{^http://code\.google\.com/}, %r{^http://bitbucket\.org/}, %r{^http://(?:[^/]*\.)?archive\.org} - problem "Please use https:// for #{homepage}" - end - end - - def autocorrect(node) - lambda do |corrector| - return if node.nil? - - homepage = string_content(node).dup - return if homepage.nil? || homepage.empty? - - homepage.sub!("readthedocs.org", "readthedocs.io") - homepage.delete_suffix!(".git") if homepage.start_with?("https://github.com") - corrector.replace(node.source_range, "\"#{homepage}\"") + problem "Please use https:// for #{homepage}" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage.sub("http", "https")}\"") + end end end end diff --git a/Library/Homebrew/test/rubocops/homepage_spec.rb b/Library/Homebrew/test/rubocops/homepage_spec.rb index cfabcb0921..1458673a9c 100644 --- a/Library/Homebrew/test/rubocops/homepage_spec.rb +++ b/Library/Homebrew/test/rubocops/homepage_spec.rb @@ -7,64 +7,149 @@ describe RuboCop::Cop::FormulaAudit::Homepage do subject(:cop) { described_class.new } context "When auditing homepage" do - it "When there is no homepage" do - source = <<~RUBY + it "reports an offense when there is no homepage" do + expect_offense(<<~RUBY) class Foo < Formula + ^^^^^^^^^^^^^^^^^^^ Formula should have a homepage. url 'https://brew.sh/foo-1.0.tgz' end RUBY - - expected_offenses = [{ message: "Formula should have a homepage.", - severity: :convention, - line: 1, - column: 0, - source: source }] - - inspect_source(source) - - expected_offenses.zip(cop.offenses).each do |expected, actual| - expect_offense(expected, actual) - end end - it "Homepage with ftp" do - source = <<~RUBY + it "reports an offense when the homepage is not HTTP or HTTPS" do + expect_offense(<<~RUBY) class Foo < Formula homepage "ftp://brew.sh/foo" + ^^^^^^^^^^^^^^^^^^^ The homepage should start with http or https. + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for freedesktop.org wiki pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://www.freedesktop.org/wiki/bar" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Freedesktop homepages should be styled `https://wiki.freedesktop.org/project_name` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for freedesktop.org software wiki pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://www.freedesktop.org/wiki/Software/baz" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Freedesktop homepages should be styled `https://wiki.freedesktop.org/www/Software/project_name` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports and corrects Google Code homepages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://code.google.com/p/qux" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Google Code homepages should end with a slash url "https://brew.sh/foo-1.0.tgz" end RUBY - expected_offenses = [{ message: "The homepage should start with http or " \ - "https (URL is ftp://brew.sh/foo).", - severity: :convention, - line: 2, - column: 2, - source: source }] + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://code.google.com/p/qux/" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end - inspect_source(source) + it "reports and corrects GitHub homepages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://github.com/foo/bar.git" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ GitHub homepages should not end with .git + url "https://brew.sh/foo-1.0.tgz" + end + RUBY - expected_offenses.zip(cop.offenses).each do |expected, actual| - expect_offense(expected, actual) + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://github.com/foo/bar" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + context "for Sourceforge" do + correct_formula = <<~RUBY + class Foo < Formula + homepage "https://foo.sourceforge.io/" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + it "reports and corrects [1]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sourceforge.net/" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) + end + + it "reports and corrects [2]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sourceforge.net" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) + end + + it "reports and corrects [3]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sf.net/" + ^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) end end - it "Homepage URLs" do + it "reports and corrects readthedocs.org pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://foo.readthedocs.org" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Readthedocs homepages should be `https://foo.readthedocs.io` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://foo.readthedocs.io" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for HTTP homepages" 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", + "sf" => "http://foo.sourceforge.io/", "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", - "dotgit" => "https://github.com/foo/bar.git", - "rtd" => "https://foo.readthedocs.org", + "dotgit" => "http://github.com/quux", } formula_homepages.each do |name, homepage| @@ -75,65 +160,19 @@ describe RuboCop::Cop::FormulaAudit::Homepage do end RUBY - inspect_source(source) - if homepage.include?("http://www.freedesktop.org") - if homepage.include?("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.include?("https://code.google.com") - expected_offenses = [{ message: "#{homepage} should end with a slash", - severity: :convention, - line: 2, - column: 2, - source: source }] - elsif homepage.match?(/foo\.(sf|sourceforge)\.net/) - expected_offenses = [{ message: "#{homepage} should be `https://foo.sourceforge.io/`", - severity: :convention, - line: 2, - column: 2, - source: source }] - elsif homepage.match?("https://github.com/foo/bar.git") - expected_offenses = [{ message: "GitHub homepages (`#{homepage}`) should not end with .git", - severity: :convention, - line: 2, - column: 11, - source: source }] - elsif homepage.match?("https://foo.readthedocs.org") - expected_offenses = [{ message: "#{homepage} should be `https://foo.readthedocs.io`", - severity: :convention, - line: 2, - column: 11, - 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) + expected_offenses = [{ message: "Please use https:// for #{homepage}", + severity: :convention, + line: 2, + column: 11, + source: source }] + + expected_offenses.zip([inspect_source(source).last]).each do |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 - - 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