livecheck: allow custom url in extract_plist strategy

[bevanjkay]

• 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?

---

This allows a custom url to be passed to livecheck when using the :extract_plist strategy.

This is useful in edge cases that meet a couple of criteria:

• There is no version information available utilising the other livecheck methods
• There is an unversioned download link that points to the latest version
• There is a versioned download link available that we can use for download

At the moment in these cases we are limited to using the unversioned url OR using the versioned url and skipping the livecheck.

An example of a cask that is currently not functioning as intended is the speedify cask.

[BrewTestBot]

Review period will end on 2022-05-31 at 06:46:29 UTC.

[bevanjkay]

I'm getting a typecheck error, I'd appreciate some direction on how to get it sorted.

brew typecheck   
livecheck/strategy/extract_plist.rb:104: Method url does not exist on T.class_of(Homebrew::Livecheck::Strategy::ExtractPlist) https://srb.help/7003
     104 |              url url.to_s
                        ^^^

[samford]

In general, the idea behind this PR makes sense to me. Checking a versioned url with ExtractPlist would be a waste of effort (as it would simply give the current version) and, as you mentioned, it's necessary to check an unversioned url in cases like speedify to identify the latest version 👍

---

I'm getting a typecheck error, I'd appreciate some direction on how to get it sorted.

brew typecheck   
livecheck/strategy/extract_plist.rb:104: Method url does not exist on T.class_of(Homebrew::Livecheck::Strategy::ExtractPlist) https://srb.help/7003
     104 |              url url.to_s
                        ^^^

I tinkered with this and the only way I found to avoid the typecheck error was to use Cask::CaskLoader.load with a string, like we do in tests (e.g., livecheck/livecheck_spec.rb). For example, the existing code would be like this:

Cask::CaskLoader.load(+<<-RUBY, config: cask.config)
  cask "livecheck-url-cask" do
    url "#{url}"
  end
RUBY

That said, we can simplify this and avoid the UnversionedCaskChecker changes if we can create a copy of the cask with a substitute url (instead of passing in a supplementary livecheck_url cask to use for UnversionedCaskChecker#installer). It would be ideal if we could simply replace the underlying cask.url URL::DSL object but Cask::DSL#set_unique_stanza won't allow this (e.g., cask.url { url } produces a Cask::CaskInvalidError (Cask 'speedify' definition is invalid: 'url' stanza may only appear once.) error).

However, if there aren't any objections to using #instance_variable_set in this context, we can sidestep the issue by directly replacing the @url instance variable (in the Cask::DSL object). I've added a suggestion below that demonstrates this approach but it shouldn't be committed unless/until the approach is approved.

[BrewTestBot]

Review period ended.

[samford]

In the interest of merging this, I rebased the branch and pushed a couple commits. The first commit adds a missing documentation comment and adds url to the #find_versions return hash (to align it with other strategies). The second commit reworks how we handle the new url argument (and reverts the UnversionedCaskChecker changes) per the previous suggestion.

[MikeMcQuaid]

Need new API for this.

[samford]

I've rebased this and incorporated the previously-discussed allow_reassignment approach in Cask, so it's ready for review.

Past that, I added a commit to ensure that livecheck only requires a URL when a livecheck block specifies a strategy and the strategy's #find_versions method has a required url parameter. Up to this point, we've had to add a url to livecheck blocks using strategy :extract_plist, even though ExtractPlist#find_versions checks a cask instead of a URL. Now that ExtractPlist supports an optional URL, it would be clearer to only include a URL in related livecheck blocks when it's intended to override the cask url.

[Regarding disabling the Metrics/CyclomaticComplexity RuboCop for Livecheck#latest_version: I have some ideas on how to refactor the Livecheck module (and this method) to resolve the existing offenses. There are a few other things that I want to get finished first but I'm hoping to take care of this in the not too distant future.]

[MikeMcQuaid]

Looks great, nice work @samford!

[samford]

Thanks, everyone!