rubocops/livecheck: Rework LivecheckUrlProvided

[samford]

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

---

The existing LivecheckUrlProvided RuboCop requires a url for all livecheck blocks except those using skip, formula, or cask, as those only appear in a livecheck block with no other DSL methods.

We now have a throttle method that can be used alongside other DSL methods (e.g., url, regex, strategy) or by itself. brew style currently fails when throttle is used by itself (e.g., Homebrew/homebrew-core#166501), so this updates the conditions to allow this usage.

I'm currently doing this by specifically exempting livecheck blocks that only include #throttle (i.e., so we don't unexpectedly skip the check for livecheck blocks that use #throttle alongside other methods). I'm not very versed with RuboCop, so there may be a better way of achieving the same goal.

I've gone about this by reworking the LivecheckUrlProvided RuboCop to only require a url when a livecheck block uses #regex and/or #strategy, as that information is intended for a specific URL. This allows us to get rid of the special-case conditions for #skip, #formula, and #cask (and avoid adding additional conditions for #throttle).

[reitermarkus]

so we don't unexpectedly skip the check

Can you think of an example when this would be the case? Do we have other methods that require a URL, or is the check really only there to avoid an empty livecheck block without a URL?

[samford]

Can you think of an example when this would be the case?

In this particular context, I was referring to something [undesirable] like the following:

livecheck do
  regex(/^v?(\d+(?:\.\d+)+)$/i)
  throttle 10
end

livecheck do
  strategy :page_match
  throttle 10
end

If we skipped the LivecheckUrlProvided RuboCop for #throttle in the same way that we do for #skip, #formula, and #cask (i.e., checking for presence), then we could end up with livecheck blocks like those that go against our current set of expectations.

Do we have other methods that require a URL, or is the check really only there to avoid an empty livecheck block without a URL?

The idea behind the url requirement is that it makes failures less expensive (i.e., without a url, livecheck will needlessly try other formula/cask URLs before failing) and it makes checks easier to reason about at a glance (i.e., we know the URL and don't have to think as much about what livecheck may or may not be checking). Basically, a livecheck block with a regex and/or strategy is intended for a specific URL, so url should be required (except for ExtractPlist, which doesn't use a URL).

Thinking about this, it may be better to rework the RuboCop to only require a url when #regex or #strategy are present in a livecheck block, instead of having special-case exemptions. I've tested it out and it seems reasonable but I may be overlooking something.

Edit: I updated this to rework the RuboCop as described, as that should meet the same goals while requiring less maintenance when methods are added to the livecheck DSL.

[MikeMcQuaid]

Seems fine, thanks @samford!