From c5d091af219e6930aa6b5c85b3e3c19febb9e6eb Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 17 Jun 2025 11:44:15 +0100 Subject: [PATCH] Refactor/typecheck `create` and `formula_creator` --- Library/Homebrew/dev-cmd/create.rb | 30 +++---- Library/Homebrew/formula_creator.rb | 79 +++++++++++-------- Library/Homebrew/test/formula_creator_spec.rb | 51 ++++++------ 3 files changed, 84 insertions(+), 76 deletions(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index b74be43d40..edf874afeb 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -183,7 +183,7 @@ module Homebrew :zig end - fc = FormulaCreator.new( + formula_creator = FormulaCreator.new( url: args.named.fetch(0), name: args.set_name, version: args.set_version, @@ -193,30 +193,32 @@ module Homebrew fetch: !args.no_fetch?, head: args.HEAD?, ) + # ask for confirmation if name wasn't passed explicitly if args.set_name.blank? - print "Formula name [#{fc.name}]: " - fc.name = __gets || fc.name + print "Formula name [#{formula_creator.name}]: " + confirmed_name = __gets.presence + formula_creator.name = confirmed_name if confirmed_name.present? end - fc.verify + formula_creator.verify_tap_available! # Check for disallowed formula, or names that shadow aliases, # unless --force is specified. unless args.force? - if (reason = MissingFormula.disallowed_reason(T.must(fc.name))) + if (reason = MissingFormula.disallowed_reason(formula_creator.name)) odie <<~EOS - The formula '#{fc.name}' is not allowed to be created. + The formula '#{formula_creator.name}' is not allowed to be created. #{reason} If you really want to create this formula use `--force`. EOS end Homebrew.with_no_api_env do - if Formula.aliases.include? fc.name - realname = Formulary.canonical_name(fc.name) + if Formula.aliases.include?(formula_creator.name) + realname = Formulary.canonical_name(formula_creator.name) odie <<~EOS - The formula '#{realname}' is already aliased to '#{fc.name}'. + The formula '#{realname}' is already aliased to '#{formula_creator.name}'. Please check that you are not creating a duplicate. To force creation use `--force`. EOS @@ -224,19 +226,19 @@ module Homebrew end end - path = fc.write_formula! + path = formula_creator.write_formula! formula = Homebrew.with_no_api_env do CoreTap.instance.clear_cache - Formula[T.must(fc.name)] + Formula[formula_creator.name] end PyPI.update_python_resources! formula, ignore_non_pypi_packages: true if args.python? puts <<~EOS Please audit and test formula before submitting: - HOMEBREW_NO_INSTALL_FROM_API=1 brew audit --new #{fc.name} - HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source --verbose --debug #{fc.name} - HOMEBREW_NO_INSTALL_FROM_API=1 brew test #{fc.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew audit --new #{formula_creator.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source --verbose --debug #{formula_creator.name} + HOMEBREW_NO_INSTALL_FROM_API=1 brew test #{formula_creator.name} EOS path end diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 399ebaaec5..4fc3d7c430 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -1,4 +1,4 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "digest" @@ -7,10 +7,10 @@ require "erb" module Homebrew # Class for generating a formula from a template. class FormulaCreator - sig { returns(T.nilable(String)) } + sig { returns(String) } attr_accessor :name - sig { returns(T.nilable(Version)) } + sig { returns(Version) } attr_reader :version sig { returns(T::Boolean) } @@ -22,55 +22,66 @@ module Homebrew } def initialize(url:, name: nil, version: nil, tap: nil, mode: nil, license: nil, fetch: false, head: false) @url = url - @name = name.presence - @version = T.let(Version.new(version), T.nilable(Version)) if version.present? - @tap = T.let(Tap.fetch(tap.presence || "homebrew/core"), Tap) - @mode = T.let(mode.presence, T.nilable(Symbol)) - @license = T.let(license.presence, T.nilable(String)) - @fetch = fetch - @head = head - parse_url - end - - sig { void } - def verify - raise TapUnavailableError, @tap.name unless @tap.installed? - end - - sig { void } - def parse_url - @name ||= begin - stem = Pathname.new(@url).stem - name = if stem.start_with? "index.cgi" + if name.blank? + stem = Pathname.new(url).stem + name = if stem.start_with?("index.cgi") && stem.include?("=") # special cases first # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary stem.rpartition("=").last - elsif @url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} + elsif url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/} # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz - Regexp.last_match(1) + T.must(Regexp.last_match(1)) else # e.g. http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz pathver = Version.parse(stem).to_s stem.sub(/[-_.]?#{Regexp.escape(pathver)}$/, "") end odebug "name from url: #{name}" - name end + @name = T.let(name, String) - @version ||= Version.detect(@url) + version = if version.present? + Version.new(version) + else + Version.detect(url) + end + @version = T.let(version, Version) - case @url + tap = if tap.blank? + CoreTap.instance + else + Tap.fetch(tap) + end + @tap = T.let(tap, Tap) + + @mode = T.let(mode.presence, T.nilable(Symbol)) + @license = T.let(license.presence, T.nilable(String)) + @fetch = fetch + + case url when %r{github\.com/(\S+)/(\S+)\.git} - @head = true + head = true user = Regexp.last_match(1) - repo = Regexp.last_match(2) - @github = GitHub.repository(user, repo) if @fetch + repository = Regexp.last_match(2) + github = GitHub.repository(user, repository) if fetch when %r{github\.com/(\S+)/(\S+)/(archive|releases)/} user = Regexp.last_match(1) - repo = Regexp.last_match(2) - @github = GitHub.repository(user, repo) if @fetch + repository = Regexp.last_match(2) + github = GitHub.repository(user, repository) if fetch end + @head = head + @github = T.let(github, T.untyped) + + @sha256 = T.let(nil, T.nilable(String)) + @desc = T.let(nil, T.nilable(String)) + @homepage = T.let(nil, T.nilable(String)) + @license = T.let(nil, T.nilable(String)) + end + + sig { void } + def verify_tap_available! + raise TapUnavailableError, @tap.name unless @tap.installed? end sig { returns(Pathname) } @@ -114,6 +125,8 @@ module Homebrew path end + private + sig { params(name: String).returns(String) } def latest_versioned_formula(name) name_prefix = "#{name}@" diff --git a/Library/Homebrew/test/formula_creator_spec.rb b/Library/Homebrew/test/formula_creator_spec.rb index 772a123d05..28d5c03f0d 100644 --- a/Library/Homebrew/test/formula_creator_spec.rb +++ b/Library/Homebrew/test/formula_creator_spec.rb @@ -6,48 +6,41 @@ RSpec.describe Homebrew::FormulaCreator do describe ".new" do tests = { "generic tarball URL": { - url: "http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz", - expected: { - name: "synscan", - version: "5.02", - }, + url: "http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz", + name: "synscan", + version: "5.02", }, "gitweb URL": { - url: "http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary", - expected: { - name: "libzipper", - }, + url: "http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary", + name: "libzipper", }, "GitHub repo URL": { - url: "https://github.com/abitrolly/lapce.git", - expected: { - name: "lapce", - head: true, - }, + url: "https://github.com/abitrolly/lapce.git", + name: "lapce", + head: true, }, "GitHub archive URL": { - url: "https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz", - expected: { - name: "lapce", - version: "0.3.0", - }, + url: "https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz", + name: "lapce", + version: "0.3.0", }, "GitHub download URL": { - url: "https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz", - expected: { - name: "stella", - version: "6.7", - }, + url: "https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz", + name: "stella", + version: "6.7", }, } tests.each do |description, test| it "parses #{description}" do - fc = described_class.new(url: test.fetch(:url)) - ex = test.fetch(:expected) - expect(fc.name).to eq(ex[:name]) if ex.key?(:name) - expect(fc.version).to eq(ex[:version]) if ex.key?(:version) - expect(fc.head).to eq(ex[:head]) if ex.key?(:head) + formula_creator = described_class.new(url: test.fetch(:url)) + expect(formula_creator.name).to eq(test.fetch(:name)) + if (version = test[:version]) + expect(formula_creator.version).to eq(version) + else + expect(formula_creator.version).to be_null + end + expect(formula_creator.head).to eq(test.fetch(:head, false)) end end end