From 0b6b05d1aabb5401934a64cd683a460c5f9470c9 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 31 Mar 2021 06:14:41 +0200 Subject: [PATCH 1/3] Implement nested `url do` API. --- Library/Homebrew/cask/dsl.rb | 34 ++++-- Library/Homebrew/cask/url.rb | 209 ++++++++++++++++++++++++++++------ Library/Homebrew/cask/url.rbi | 6 + 3 files changed, 205 insertions(+), 44 deletions(-) create mode 100644 Library/Homebrew/cask/url.rbi diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index a46a7c18bc..d7648987a0 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -96,6 +96,7 @@ module Cask @token = cask.token end + # @api public def name(*args) @name ||= [] return @name if args.empty? @@ -103,6 +104,7 @@ module Cask @name.concat(args.flatten) end + # @api public def desc(description = nil) set_unique_stanza(:desc, description.nil?) { description } end @@ -121,6 +123,7 @@ module Cask raise CaskInvalidError.new(cask, "'#{stanza}' stanza failed with: #{e}") end + # @api public def homepage(homepage = nil) set_unique_stanza(:homepage, homepage.nil?) { homepage } end @@ -176,32 +179,32 @@ module Cask @language_blocks.keys.flatten end - def url(*args, **options) + # @api public + def url(*args, **options, &block) caller_location = caller_locations[0] - set_unique_stanza(:url, args.empty? && options.empty? && !block_given?) do - if block_given? - LazyObject.new do - *args = yield - options = args.last.is_a?(Hash) ? args.pop : {} - URL.new(*args, **options, from_block: true, caller_location: caller_location) - end + set_unique_stanza(:url, args.empty? && options.empty? && !block) do + if block + URL.new(*args, **options, caller_location: caller_location, dsl: self, &block) else URL.new(*args, **options, caller_location: caller_location) end end end + # @api public def appcast(*args) set_unique_stanza(:appcast, args.empty?) { DSL::Appcast.new(*args) } end + # @api public def container(*args) set_unique_stanza(:container, args.empty?) do DSL::Container.new(*args) end end + # @api public def version(arg = nil) set_unique_stanza(:version, arg.nil?) do if !arg.is_a?(String) && arg != :latest @@ -212,6 +215,7 @@ module Cask end end + # @api public def sha256(arg = nil) set_unique_stanza(:sha256, arg.nil?) do case arg @@ -226,6 +230,7 @@ module Cask end # `depends_on` uses a load method so that multiple stanzas can be merged. + # @api public def depends_on(*args) @depends_on ||= DSL::DependsOn.new return @depends_on if args.empty? @@ -238,6 +243,7 @@ module Cask @depends_on end + # @api public def conflicts_with(*args) # TODO: remove this constraint, and instead merge multiple conflicts_with stanzas set_unique_stanza(:conflicts_with, args.empty?) { DSL::ConflictsWith.new(*args) } @@ -251,6 +257,7 @@ module Cask cask.caskroom_path end + # @api public def staged_path return @staged_path if @staged_path @@ -258,6 +265,7 @@ module Cask @staged_path = caskroom_path.join(cask_version.to_s) end + # @api public def caveats(*strings, &block) @caveats ||= DSL::Caveats.new(cask) if block @@ -276,10 +284,12 @@ module Cask @caveats&.discontinued? end + # @api public def auto_updates(auto_updates = nil) set_unique_stanza(:auto_updates, auto_updates.nil?) { auto_updates } end + # @api public def livecheck(&block) @livecheck ||= Livecheck.new(self) return @livecheck unless block @@ -317,8 +327,6 @@ module Cask end end - # No need to define it as it's the default/superclass implementation. - # rubocop:disable Style/MissingRespondToMissing def method_missing(method, *) if method Utils.method_missing_message(method, token) @@ -327,8 +335,12 @@ module Cask super end end - # rubocop:enable Style/MissingRespondToMissing + def respond_to_missing?(*) + true + end + + # @api public def appdir cask.config.appdir end diff --git a/Library/Homebrew/cask/url.rb b/Library/Homebrew/cask/url.rb index 5819f43606..3d39fbfb7a 100644 --- a/Library/Homebrew/cask/url.rb +++ b/Library/Homebrew/cask/url.rb @@ -4,21 +4,146 @@ # Class corresponding to the `url` stanza. # # @api private -class URL +class URL < Delegator extend T::Sig - attr_reader :uri, :specs, - :verified, :using, - :tag, :branch, :revisions, :revision, - :trust_cert, :cookies, :referer, :header, :user_agent, - :data + # @api private + class DSL + extend T::Sig - extend Forwardable - def_delegators :uri, :path, :scheme, :to_s + attr_reader :uri, :specs, + :verified, :using, + :tag, :branch, :revisions, :revision, + :trust_cert, :cookies, :referer, :header, :user_agent, + :data + + extend Forwardable + def_delegators :uri, :path, :scheme, :to_s + + # @api public + sig { + params( + uri: T.any(URI::Generic, String), + verified: T.nilable(String), + using: T.nilable(Symbol), + tag: T.nilable(String), + branch: T.nilable(String), + revisions: T.nilable(T::Array[String]), + revision: T.nilable(String), + trust_cert: T.nilable(T::Boolean), + cookies: T.nilable(T::Hash[String, String]), + referer: T.nilable(T.any(URI::Generic, String)), + header: T.nilable(String), + user_agent: T.nilable(T.any(Symbol, String)), + data: T.nilable(T::Hash[String, String]), + ).void + } + def initialize( + uri, + verified: nil, + using: nil, + tag: nil, + branch: nil, + revisions: nil, + revision: nil, + trust_cert: nil, + cookies: nil, + referer: nil, + header: nil, + user_agent: nil, + data: nil + ) + + @uri = URI(uri) + + specs = {} + specs[:verified] = @verified = verified + specs[:using] = @using = using + specs[:tag] = @tag = tag + specs[:branch] = @branch = branch + specs[:revisions] = @revisions = revisions + specs[:revision] = @revision = revision + specs[:trust_cert] = @trust_cert = trust_cert + specs[:cookies] = @cookies = cookies + specs[:referer] = @referer = referer + specs[:header] = @header = header + specs[:user_agent] = @user_agent = user_agent || :default + specs[:data] = @data = data + + @specs = specs.compact + end + end + + # @api private + class BlockDSL + extend T::Sig + + module PageWithURL + extend T::Sig + + # @api public + sig { returns(URI::Generic) } + attr_accessor :url + end + + sig { + params( + uri: T.nilable(T.any(URI::Generic, String)), + dsl: T.nilable(Cask::DSL), + block: T.proc.params(arg0: T.all(String, PageWithURL)).returns(T.untyped), + ).void + } + def initialize(uri, dsl: nil, &block) + @uri = uri + @dsl = dsl + @block = block + end + + sig { returns(T.untyped) } + def call + if @uri + result = curl_output("--fail", "--silent", "--location", @uri) + result.assert_success! + + page = result.stdout + page.extend PageWithURL + page.url = URI(@uri) + + instance_exec(page, &@block) + else + instance_exec(&@block) + end + end + + # @api public + sig { + params( + uri: T.any(URI::Generic, String), + block: T.proc.params(arg0: T.all(String, PageWithURL)).returns(T.untyped), + ).void + } + def url(uri, &block) + self.class.new(uri, &block).call + end + private :url + + # @api public + def method_missing(method, *args, **options, &block) + if @dsl.respond_to?(method) + T.unsafe(@dsl).public_send(method, *args, **options, &block) + else + super + end + end + + def respond_to_missing?(method, include_all) + @dsl.respond_to?(method, include_all) || super + end + end sig { params( - uri: T.any(URI::Generic, String), + uri: T.nilable(T.any(URI::Generic, String)), verified: T.nilable(String), using: T.nilable(Symbol), tag: T.nilable(String), @@ -31,12 +156,13 @@ class URL header: T.nilable(String), user_agent: T.nilable(T.any(Symbol, String)), data: T.nilable(T::Hash[String, String]), - from_block: T::Boolean, caller_location: Thread::Backtrace::Location, - ).returns(T.untyped) + dsl: T.nilable(Cask::DSL), + block: T.nilable(T.proc.params(arg0: T.all(String, BlockDSL::PageWithURL)).returns(T.untyped)), + ).void } def initialize( - uri, + uri = nil, verified: nil, using: nil, tag: nil, @@ -49,32 +175,49 @@ class URL header: nil, user_agent: nil, data: nil, - from_block: false, - caller_location: T.must(caller_locations).fetch(0) + caller_location: T.must(caller_locations).fetch(0), + dsl: nil, + &block ) + super( + if block + LazyObject.new do + *args = BlockDSL.new(uri, dsl: dsl, &block).call + options = args.last.is_a?(Hash) ? args.pop : {} + uri = T.let(args.first, T.any(URI::Generic, String)) + DSL.new(uri, **options) + end + else + DSL.new( + T.must(uri), + verified: verified, + using: using, + tag: tag, + branch: branch, + revisions: revisions, + revision: revision, + trust_cert: trust_cert, + cookies: cookies, + referer: referer, + header: header, + user_agent: user_agent, + data: data, + ) + end + ) - @uri = URI(uri) - - specs = {} - specs[:verified] = @verified = verified - specs[:using] = @using = using - specs[:tag] = @tag = tag - specs[:branch] = @branch = branch - specs[:revisions] = @revisions = revisions - specs[:revision] = @revision = revision - specs[:trust_cert] = @trust_cert = trust_cert - specs[:cookies] = @cookies = cookies - specs[:referer] = @referer = referer - specs[:header] = @header = header - specs[:user_agent] = @user_agent = user_agent || :default - specs[:data] = @data = data - - @specs = specs.compact - - @from_block = from_block + @from_block = !block.nil? @caller_location = caller_location end + def __getobj__ + @dsl + end + + def __setobj__(dsl) + @dsl = dsl + end + sig { returns(T.nilable(String)) } def raw_interpolated_url return @raw_interpolated_url if defined?(@raw_interpolated_url) diff --git a/Library/Homebrew/cask/url.rbi b/Library/Homebrew/cask/url.rbi new file mode 100644 index 0000000000..6f32e1b3b2 --- /dev/null +++ b/Library/Homebrew/cask/url.rbi @@ -0,0 +1,6 @@ +# typed: strict +# typed: false + +class URL + include Kernel +end From d2a3944e70dc8c78424c0122d11a5b329ed1403a Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 31 Mar 2021 06:15:06 +0200 Subject: [PATCH 2/3] Skip `url do` URLs in offline audit. --- Library/Homebrew/cask/audit.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index e548ebc943..80e05ec81e 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -308,7 +308,7 @@ module Cask LIVECHECK_REFERENCE_URL = "https://github.com/Homebrew/homebrew-cask/blob/HEAD/doc/cask_language_reference/stanzas/livecheck.md" def check_hosting_with_livecheck(livecheck_result:) - return if cask.appcast || cask.livecheckable? + return if block_url_offline? || cask.appcast || cask.livecheckable? return if livecheck_result == :auto_detected add_livecheck = "please add a livecheck. See #{Formatter.url(LIVECHECK_REFERENCE_URL)}" @@ -419,9 +419,16 @@ module Cask URI(cask.url.to_s).scheme == "file" end + def block_url_offline? + return false if online? + + cask.url.from_block? + end + VERIFIED_URL_REFERENCE_URL = "https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/url.md#when-url-and-homepage-hostnames-differ-add-verified" def check_unnecessary_verified + return if block_url_offline? return unless verified_present? return unless url_match_homepage? return unless verified_matches_url? @@ -432,7 +439,7 @@ module Cask end def check_missing_verified - return if cask.url.from_block? + return if block_url_offline? return if file_url? return if url_match_homepage? return if verified_present? @@ -443,6 +450,7 @@ module Cask end def check_no_match + return if block_url_offline? return unless verified_present? return if verified_matches_url? @@ -592,7 +600,7 @@ module Cask return if cask.tap == "homebrew/cask-versions" odebug "Auditing GitHub prerelease" - user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) if @online + user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) if online? return if user.nil? tag = SharedAudits.github_tag_from_url(cask.url) @@ -604,7 +612,7 @@ module Cask def check_gitlab_prerelease_version return if cask.tap == "homebrew/cask-versions" - user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) if @online + user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) if online? return if user.nil? odebug "Auditing GitLab prerelease" @@ -616,7 +624,7 @@ module Cask end def check_github_repository_archived - user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) if @online + user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) if online? return if user.nil? odebug "Auditing GitHub repo archived" @@ -636,7 +644,7 @@ module Cask end def check_gitlab_repository_archived - user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) if @online + user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) if online? return if user.nil? odebug "Auditing GitLab repo archived" From c07205caf2146dfd3d496a4b887befb83ae04455 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 31 Mar 2021 06:25:33 +0200 Subject: [PATCH 3/3] Adjust `audit` spec. --- Library/Homebrew/test/cask/audit_spec.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 168230fe31..5b12bb80fa 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -795,7 +795,17 @@ describe Cask::Audit, :cask do end end - context "when doing the audit" do + context "when doing an offline audit" do + let(:online) { false } + + it "does not evaluate the block" do + expect(run).not_to pass + end + end + + context "when doing and online audit" do + let(:online) { true } + it "evaluates the block" do expect(run).to fail_with(/Boom/) end