Merge pull request #20116 from abitrolly/create-tests

Refactor `FormulaCreator` args and call `parse_url` automatically

Library/Homebrew/dev-cmd/create.rb

@@ -183,41 +183,42 @@ module Homebrew
           :zig
         end
 
-        fc = FormulaCreator.new(
+        formula_creator = FormulaCreator.new(
-          args.set_name,
-          args.set_version,
-          tap:     args.tap,
           url:     args.named.fetch(0),
+          name:    args.set_name,
+          version: args.set_version,
+          tap:     args.tap,
           mode:,
           license: args.set_license,
           fetch:   !args.no_fetch?,
           head:    args.HEAD?,
         )
-        fc.parse_url
+
         # ask for confirmation if name wasn't passed explicitly
         if args.set_name.blank?
-          print "Formula name [#{fc.name}]: "
+          print "Formula name [#{formula_creator.name}]: "
-          fc.name = __gets || fc.name
+          confirmed_name = __gets
+          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(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
+            if Formula.aliases.include?(formula_creator.name)
-              realname = Formulary.canonical_name(fc.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
@@ -225,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[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 audit --new #{formula_creator.name}
-            HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source --verbose --debug #{fc.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 #{fc.name}
+            HOMEBREW_NO_INSTALL_FROM_API=1 brew test #{formula_creator.name}
         EOS
         path
       end

Library/Homebrew/formula_creator.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "digest"
@@ -7,62 +7,81 @@ require "erb"
 module Homebrew
   # Class for generating a formula from a template.
   class FormulaCreator
+    sig { returns(String) }
     attr_accessor :name
 
+    sig { returns(Version) }
+    attr_reader :version
+
+    sig { returns(T::Boolean) }
+    attr_reader :head
+
     sig {
-      params(name: T.nilable(String), version: T.nilable(String), tap: T.nilable(String), url: String,
+      params(url: String, name: T.nilable(String), version: T.nilable(String), tap: T.nilable(String),
              mode: T.nilable(Symbol), license: T.nilable(String), fetch: T::Boolean, head: T::Boolean).void
     }
-    def initialize(name, version, tap:, url:, mode:, license:, fetch:, head:)
+    def initialize(url:, name: nil, version: nil, tap: nil, mode: nil, license: nil, fetch: false, head: false)
-      @name = name
-      @version = Version.new(version) if version
-      @tap = Tap.fetch(tap || "homebrew/core")
       @url = url
-      @mode = mode
-      @license = license
-      @fetch = fetch
-      @head = head
-    end
 
-    sig { void }
+      if name.blank?
-    def verify
+        stem = Pathname.new(url).stem
-      raise TapUnavailableError, @tap.name unless @tap.installed?
+        name = if stem.start_with?("index.cgi") && stem.include?("=")
-    end
+          # special cases first
-
+          # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary
-    sig { params(url: String).returns(T.nilable(String)) }
+          stem.rpartition("=").last
-    def self.name_from_url(url)
+        elsif url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/}
-      stem = Pathname.new(url).stem
+          # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz
-      # special cases first
+          T.must(Regexp.last_match(1))
-      if stem.start_with? "index.cgi"
+        else
-        # gitweb URLs e.g. http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary
+          # e.g. http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz
-        stem.rpartition("=").last
+          pathver = Version.parse(stem).to_s
-      elsif url =~ %r{github\.com/\S+/(\S+)/(archive|releases)/}
+          stem.sub(/[-_.]?#{Regexp.escape(pathver)}$/, "")
-        # e.g. https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz
+        end
-        Regexp.last_match(1)
+        odebug "name from url: #{name}"
-      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
-    end
+      @name = T.let(name, String)
 
-    sig { void }
+      version = if version.present?
-    def parse_url
+        Version.new(version)
-      @name = FormulaCreator.name_from_url(@url) if @name.blank?
+      else
-      odebug "name_from_url: #{@name}"
+        Version.detect(url)
-      @version = Version.detect(@url) if @version.nil?
+      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)
+        repository = Regexp.last_match(2)
-        @github = GitHub.repository(user, repo) if @fetch
+        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)
+        repository = Regexp.last_match(2)
-        @github = GitHub.repository(user, repo) if @fetch
+        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) }
@@ -91,7 +110,7 @@ module Homebrew
             raise "Downloaded URL is not archive"
           end
 
-          @sha256 = filepath.sha256
+          @sha256 = T.let(filepath.sha256, T.nilable(String))
         end
 
         if @github
@@ -106,6 +125,8 @@ module Homebrew
       path
     end
 
+    private
+
     sig { params(name: String).returns(String) }
     def latest_versioned_formula(name)
       name_prefix = "#{name}@"

Library/Homebrew/test/formula_creator_spec.rb

@@ -3,28 +3,45 @@
 require "formula_creator"
 
 RSpec.describe Homebrew::FormulaCreator do
-  it "gets name from GitHub archive URL" do
+  describe ".new" do
-    t = described_class.name_from_url("https://github.com/abitrolly/lapce/archive/v0.3.0.tar.gz")
+    tests = {
-    expect(t).to eq("lapce")
+      "generic tarball URL": {
-  end
+        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",
+        name: "libzipper",
+      },
+      "GitHub repo URL":     {
+        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",
+        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",
+        name:    "stella",
+        version: "6.7",
+      },
+    }
 
-  it "gets name from gitweb URL" do
+    tests.each do |description, test|
-    t = described_class.name_from_url("http://www.codesrc.com/gitweb/index.cgi?p=libzipper.git;a=summary")
+      it "parses #{description}" do
-    expect(t).to eq("libzipper")
+        formula_creator = described_class.new(url: test.fetch(:url))
-  end
+        expect(formula_creator.name).to eq(test.fetch(:name))
-
+        if (version = test[:version])
-  it "gets name from GitHub repo URL" do
+          expect(formula_creator.version).to eq(version)
-    t = described_class.name_from_url("https://github.com/abitrolly/lapce.git")
+        else
-    expect(t).to eq("lapce")
+          expect(formula_creator.version).to be_null
-  end
+        end
-
+        expect(formula_creator.head).to eq(test.fetch(:head, false))
-  it "gets name from GitHub download URL" do
+      end
-    t = described_class.name_from_url("https://github.com/stella-emu/stella/releases/download/6.7/stella-6.7-src.tar.xz")
+    end
-    expect(t).to eq("stella")
-  end
-
-  it "gets name from generic tarball URL" do
-    t = described_class.name_from_url("http://digit-labs.org/files/tools/synscan/releases/synscan-5.02.tar.gz")
-    expect(t).to eq("synscan")
   end
 end