Skip to content

Commit

Permalink
Only 'verified' stanzas with 0 or >1 path components should end with "/"
Browse files Browse the repository at this point in the history
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.
```
  • Loading branch information
issyl0 committed Apr 2, 2023
1 parent 17c0eaa commit d6aa751
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 12 deletions.
9 changes: 8 additions & 1 deletion Library/Homebrew/rubocops/cask/url.rb
Expand Up @@ -24,6 +24,7 @@ class Url < Base
def on_url_stanza(stanza)
return if stanza.stanza_node.block_type?

url_string = stanza.stanza_node.first_argument.str_content
hash_node = stanza.stanza_node.last_argument
return unless hash_node.hash_type?

Expand All @@ -40,7 +41,9 @@ def on_url_stanza(stanza)
end
end

next unless value_node.str_content.gsub(%r{https?://}, "").include?("/") # Skip if the stanza has no path.
# Skip if the URL and the verified value are the same.
next if value_node.str_content == without_scheme(url_string)
# Skip if the verified value ends with a slash.
next if value_node.str_content.end_with?("/")

add_offense(
Expand All @@ -51,6 +54,10 @@ def on_url_stanza(stanza)
end
end
end

def without_scheme(stanza)
stanza.delete_prefix("https://").delete_prefix("http://")
end
end
end
end
Expand Down
127 changes: 116 additions & 11 deletions Library/Homebrew/test/rubocops/cask/url_spec.rb
Expand Up @@ -14,7 +14,7 @@
<<~CASK
cask "foo" do
url "https://example.com/download/foo-v1.2.0.dmg",
verified: "example.com"
verified: "example.com/download/"
end
CASK
end
Expand All @@ -27,7 +27,7 @@
<<~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
Expand All @@ -38,15 +38,120 @@
severity: :convention,
line: 3,
column: 16,
source: "\"https://example.com\"",
source: "\"https://example.com/download/\"",
}]
end

let(:correct_source) 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
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 url 'verified' value does not have a path component but doesn't end with a /" 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

context "when url 'verified' value has a single 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

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
Expand All @@ -59,8 +164,8 @@
let(:source) do
<<~CASK
cask "foo" do
url "https://example.com/download/foo-v1.2.0.dmg",
verified: "example.com/download/"
url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg",
verified: "github.com/Foo/foo/"
end
CASK
end
Expand All @@ -72,8 +177,8 @@
let(:source) do
<<~CASK
cask "foo" do
url "https://example.com/download/foo-v1.2.0.dmg",
verified: "example.com/download"
url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg",
verified: "github.com/Foo/foo"
end
CASK
end
Expand All @@ -84,15 +189,15 @@
severity: :convention,
line: 3,
column: 16,
source: "\"example.com/download\"",
source: "\"github.com/Foo/foo\"",
}]
end

let(:correct_source) do
<<~CASK
cask "foo" do
url "https://example.com/download/foo-v1.2.0.dmg",
verified: "example.com/download/"
url "https://github.com/Foo/foo/releases/download/v1.2.0/foo-v1.2.0.dmg",
verified: "github.com/Foo/foo/"
end
CASK
end
Expand Down

0 comments on commit d6aa751

Please sign in to comment.