Merge pull request #15125 from issyl0/rubocops-cask-verified-url-with-path-ending-slash

rubocops/cask: Ensure that "verified" URLs with paths end with "/"

Library/Homebrew/rubocops/cask/url.rb

@@ -8,13 +8,13 @@ module RuboCop
       #
       # @example
       #   # bad
-      #   url "https://example.com/foo.dmg",
+      #   url "https://example.com/download/foo.dmg",
-      #     verified: "https://example.com"
+      #       verified: "https://example.com/download"
       #
       #
       #   # good
-      #   url "https://example.com/foo.dmg",
+      #   url "https://example.com/download/foo.dmg",
-      #     verified: "example.com"
+      #       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

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,
+        column:   16,
-        source:   "\"https://example.com\"",
+        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