diff --git a/Library/Homebrew/rubocops/cask/url.rb b/Library/Homebrew/rubocops/cask/url.rb index b2fe767ac5..2d3882d938 100644 --- a/Library/Homebrew/rubocops/cask/url.rb +++ b/Library/Homebrew/rubocops/cask/url.rb @@ -8,13 +8,13 @@ module RuboCop # # @example # # bad - # url "https://example.com/foo.dmg", - # verified: "https://example.com" + # url "https://example.com/download/foo.dmg", + # verified: "https://example.com/download" # # # # good - # url "https://example.com/foo.dmg", - # verified: "example.com" + # url "https://example.com/download/foo.dmg", + # verified: "example.com/download/" # class Url < Base extend AutoCorrector @@ -24,19 +24,35 @@ module RuboCop def on_url_stanza(stanza) return if stanza.stanza_node.block_type? + url_stanza = stanza.stanza_node.first_argument hash_node = stanza.stanza_node.last_argument return unless hash_node.hash_type? hash_node.each_pair do |key_node, value_node| next unless key_node.source == "verified" next unless value_node.str_type? - next unless value_node.source.start_with?(%r{^"https?://}) + + if value_node.source.start_with?(%r{^"https?://}) + add_offense( + value_node.source_range, + message: "Verified URL parameter value should not contain a URL scheme.", + ) do |corrector| + corrector.replace(value_node.source_range, value_node.source.gsub(%r{^"https?://}, "\"")) + end + end + + # Skip if the URL and the verified value are the same. + next if value_node.source == url_stanza.source.gsub(%r{^"https?://}, "\"") + # Skip if the URL has two path components, eg: `https://github.com/google/fonts.git`. + next if url_stanza.source.gsub(%r{^"https?://}, "\"").count("/") == 2 + # Skip if the verified value ends with a slash. + next if value_node.str_content.end_with?("/") add_offense( value_node.source_range, - message: "Verified URL parameter value should not start with https:// or http://.", + message: "Verified URL parameter value should end with a /.", ) do |corrector| - corrector.replace(value_node.source_range, value_node.source.gsub(%r{^"https?://}, "\"")) + corrector.replace(value_node.source_range, value_node.source.gsub(/"$/, "/\"")) end end end diff --git a/Library/Homebrew/test/rubocops/cask/url_spec.rb b/Library/Homebrew/test/rubocops/cask/url_spec.rb index 23ef640b60..4698bd94a4 100644 --- a/Library/Homebrew/test/rubocops/cask/url_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/url_spec.rb @@ -14,7 +14,7 @@ describe RuboCop::Cop::Cask::Url do <<~CASK cask "foo" do url "https://example.com/download/foo-v1.2.0.dmg", - verified: "example.com" + verified: "example.com/download/" end CASK end @@ -27,18 +27,18 @@ describe RuboCop::Cop::Cask::Url do <<~CASK cask "foo" do url "https://example.com/download/foo-v1.2.0.dmg", - verified: "https://example.com" + verified: "https://example.com/download/" end CASK end let(:expected_offenses) do [{ - message: "Verified URL parameter value should not start with https:// or http://.", + message: "Verified URL parameter value should not contain a URL scheme.", severity: :convention, line: 3, - column: 14, - source: "\"https://example.com\"", + column: 16, + source: "\"https://example.com/download/\"", }] end @@ -46,7 +46,189 @@ describe RuboCop::Cop::Cask::Url do <<~CASK cask "foo" do url "https://example.com/download/foo-v1.2.0.dmg", - verified: "example.com" + verified: "example.com/download/" + end + CASK + end + + include_examples "reports offenses" + include_examples "autocorrects source" + end + + context "when url 'verified' value does not have a path component" do + context "when the URL ends with a slash" do + let(:source) do + <<~CASK + cask "foo" do + url "https://example.org/", + verified: "example.org/" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the URL does not end with a slash" do + let(:source) do + <<~CASK + cask "foo" do + url "https://example.org/", + verified: "example.org" + end + CASK + end + + let(:expected_offenses) do + [{ + message: "Verified URL parameter value should end with a /.", + severity: :convention, + line: 3, + column: 16, + source: "\"example.org\"", + }] + end + + let(:correct_source) do + <<~CASK + cask "foo" do + url "https://example.org/", + verified: "example.org/" + end + CASK + end + + include_examples "reports offenses" + include_examples "autocorrects source" + end + end + + context "when the URL does not end with a slash" do + describe "and it has one path component" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/Foo", + verified: "github.com/Foo" + end + CASK + end + + include_examples "does not report any offenses" + end + + describe "and it has two path components" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/foo/foo.git", + verified: "github.com/foo/foo" + end + CASK + end + + include_examples "does not report any offenses" + end + end + + context "when the url ends with a / and the verified value does too" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/", + verified: "github.com/" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url ends with a / and the verified value does not" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/", + verified: "github.com" + end + CASK + end + + let(:expected_offenses) do + [{ + message: "Verified URL parameter value should end with a /.", + severity: :convention, + line: 3, + column: 16, + source: "\"github.com\"", + }] + end + + let(:correct_source) do + <<~CASK + cask "foo" do + url "https://github.com/", + verified: "github.com/" + end + CASK + end + + include_examples "reports offenses" + include_examples "autocorrects source" + end + + context "when url 'verified' value has a path component that ends with a /" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg", + verified: "github.com/Foo/foo/" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url has interpolation in it and the verified url ends with a /" do + let(:source) do + <<~CASK + cask "foo" do + version "1.2.3" + url "https://example.com/download/foo-v\#{version}.dmg", + verified: "example.com/download/" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url 'verified' value has a path component that doesn't end with a /" do + let(:source) do + <<~CASK + cask "foo" do + url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg", + verified: "github.com/Foo/foo" + end + CASK + end + + let(:expected_offenses) do + [{ + message: "Verified URL parameter value should end with a /.", + severity: :convention, + line: 3, + column: 16, + source: "\"github.com/Foo/foo\"", + }] + end + + let(:correct_source) do + <<~CASK + cask "foo" do + url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg", + verified: "github.com/Foo/foo/" end CASK end