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

issyl0 · 2023-04-02T14:34:13Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

Fixes RuboCop for trailing slash in verified url stanza #15108.
These were being fixed manually, so let's make a RuboCop for any further occurrences since this is a good rule to enforce.

- These were being fixed manually[1], so let's make a RuboCop for any further occurrences since this is a good rule to enforce[2]. [1] - Homebrew/homebrew-cask#144179 (comment) [2] - Homebrew/homebrew-cask#80965 (comment)

Library/Homebrew/rubocops/cask/url.rb

reitermarkus · 2023-04-02T14:49:53Z

I think one test case is missing:

url "https://example.org/download",
    verified: "example.org/download"

In this case, the verified parameter should not end with a slash.

reitermarkus · 2023-04-02T14:56:34Z

Also

url "https://example.org/",
    verified: "example.org/"

I think we already have a cop to ensure the slash for the actual URL in this case, but the verified parameter also needs the slash.

issyl0 · 2023-04-02T16:40:44Z

I think one test case is missing:
url "https://example.org/download",
    verified: "example.org/download"
In this case, the verified parameter should not end with a slash.

🤔 So the following example does need the slash, but the above should not have one? Or did I misunderstand initially?

url "https://example.com/download/foo-v1.2.0.dmg",
     verified: "example.com/download"

reitermarkus · 2023-04-02T20:49:15Z

So the following example does need the slash, but the above should not have one?

Yes, exactly.

Library/Homebrew/test/rubocops/cask/url_spec.rb

Library/Homebrew/rubocops/cask/url.rb

Handle good things like: ```ruby url "https://example.org/download", verified: "example.org/download" # This is fine. ``` And bad things like: ```ruby url "https://example.org/", verified: "example.org" # This should end with a slash. ```

Co-authored-by: Markus Reiter <me@reitermark.us>

issyl0 · 2023-04-02T21:45:50Z

Thanks for the thorough review @reitermarkus!

Library/Homebrew/rubocops/cask/url.rb

- We see this a lot in real Casks.

issyl0 · 2023-04-02T22:42:01Z

Right, the real Casks report no offenses now I've fixed the handling for the syntax that I didn't see before. Tricksy Caskes.

MikeMcQuaid · 2023-04-03T11:40:35Z

Good with me once tap syntax is 🟢! Great work @issyl0 and thanks for the help here @reitermarkus.

Homebrew/homebrew-cask-fonts#7336 (comment)

reitermarkus · 2023-04-05T01:25:32Z

Library/Homebrew/rubocops/cask/url.rb

+            # 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


@issyl0, this only fixes this particular case.

What if this isn't a GitHub URL with this specific format?

url "https://example.org/long/path/app-1.2.3.dmg", verified: "example.org/long/path/app"

Granted I don't think we currently have any cask with such a format, but 2 is kinda arbitrary.

I think we are overcomplicating the condition for this cop.

If verified doesn't already end with a slash but the url starts with "#{verified}/", a slash should be added.

@reitermarkus That sounds good, could you open a PR for that?