Refactor/typecheck create and formula_creator

Author: MikeMcQuaid

Library/Homebrew/dev-cmd/create.rb

@@ -183,7 +183,7 @@ def create_formula
           :zig
         end
 
-        fc = FormulaCreator.new(
+        formula_creator = FormulaCreator.new(
           url:     args.named.fetch(0),
           name:    args.set_name,
           version: args.set_version,
@@ -193,50 +193,52 @@ def create_formula
           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
             end
           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

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 @@
 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 @@ class FormulaCreator
     }
     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 = if version.present?
+        Version.new(version)
+      else
+        Version.detect(url)
+      end
+      @version = T.let(version, Version)
 
-      @version ||= Version.detect(@url)
+      tap = if tap.blank?
+        CoreTap.instance
+      else
+        Tap.fetch(tap)
+      end
+      @tap = T.let(tap, Tap)
 
-      case @url
+      @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 @@ def write_formula!
       path
     end
 
+    private
+
     sig { params(name: String).returns(String) }
     def latest_versioned_formula(name)
       name_prefix = "#{name}@"

Library/Homebrew/test/formula_creator_spec.rb

@@ -6,48 +6,41 @@
   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