From 51b16585fb32f2d33a3ec7dd8d6ca5d6e0d844c7 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Mon, 20 Nov 2023 14:03:50 +0000 Subject: [PATCH 1/5] formula_creator: Remove `path` attr to reduce code complexity `path` attribute is used only once, and it is easier to calculate it on the fly than to update its state after different methods. --- Library/Homebrew/dev-cmd/create.rb | 4 ++-- Library/Homebrew/formula_creator.rb | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index f02b61769aa7e..36ae89bd499d6 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -200,7 +200,7 @@ def create_formula(args:) end end - fc.generate! + path = fc.generate! formula = Homebrew.with_no_api_env do Formula[fc.name] @@ -208,7 +208,7 @@ def create_formula(args:) PyPI.update_python_resources! formula, ignore_non_pypi_packages: true if args.python? puts "Please run `brew audit --new #{fc.name}` before submitting, thanks." - fc.path + path end def __gets diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 05679630e2c00..f90eb4be0b50c 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -10,7 +10,7 @@ module Homebrew # @api private class FormulaCreator attr_reader :args, :url, :sha256, :desc, :homepage - attr_accessor :name, :version, :tap, :path, :mode, :license + attr_accessor :name, :version, :tap, :mode, :license def initialize(args) @args = args @@ -34,7 +34,6 @@ def url=(url) @name = path.basename.to_s[/(.*?)[-_.]?#{Regexp.escape(path.version.to_s)}/, 1] end end - update_path @version = if @version Version.new(@version) else @@ -42,12 +41,6 @@ def url=(url) end end - def update_path - return if @name.nil? || @tap.nil? - - @path = @tap.new_formula_path(@name) - end - def fetch? !args.no_fetch? end @@ -57,6 +50,7 @@ def head? end def generate! + path = @tap.new_formula_path(@name) raise "#{path} already exists" if path.exist? if version.nil? || version.null? @@ -86,6 +80,7 @@ def generate! path.dirname.mkpath path.write ERB.new(template, trim_mode: ">").result(binding) + path end sig { returns(String) } From d8f19ff881e1015c1bd0f2960f56eaa849440ef4 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 23 Nov 2023 18:04:03 +0300 Subject: [PATCH 2/5] Rename `generate!` to `write_formula!` --- Library/Homebrew/dev-cmd/create.rb | 2 +- Library/Homebrew/formula_creator.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index 36ae89bd499d6..26ab371728d1f 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -200,7 +200,7 @@ def create_formula(args:) end end - path = fc.generate! + path = fc.write_formula! formula = Homebrew.with_no_api_env do Formula[fc.name] diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index f90eb4be0b50c..3c7b3933d7b64 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -49,7 +49,7 @@ def head? @head || args.HEAD? end - def generate! + def write_formula! path = @tap.new_formula_path(@name) raise "#{path} already exists" if path.exist? From 4fed4529d3021e2f766135c7f4c432ba68ad5aeb Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 23 Nov 2023 18:06:40 +0300 Subject: [PATCH 3/5] Raise when formula name is empty P.S. That still doesn't check if the name is invalid, like "34" --- Library/Homebrew/formula_creator.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 3c7b3933d7b64..14ea817481aa3 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -50,6 +50,7 @@ def head? end def write_formula! + raise "name should not be empty" if @name.to_s == '' path = @tap.new_formula_path(@name) raise "#{path} already exists" if path.exist? From bfbea8be269d66a1f73f1029f1019476c24749e1 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 23 Nov 2023 18:12:12 +0300 Subject: [PATCH 4/5] Style fix --- Library/Homebrew/formula_creator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 14ea817481aa3..043083bc6528a 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -50,7 +50,8 @@ def head? end def write_formula! - raise "name should not be empty" if @name.to_s == '' + raise "name should not be empty" if @name.to_s == "" + path = @tap.new_formula_path(@name) raise "#{path} already exists" if path.exist? From 4063675e0ec5e9fc082ce3f23d254ff7add37a9c Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 23 Nov 2023 18:53:22 +0300 Subject: [PATCH 5/5] Suggestion by @MikeMcQuaid "ActiveSupport adds blank? to NilClass." --- Library/Homebrew/formula_creator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 043083bc6528a..cdbcf8bf27bc0 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -50,7 +50,8 @@ def head? end def write_formula! - raise "name should not be empty" if @name.to_s == "" + raise ArgumentError, "name is blank!" if @name.blank? + raise ArgumentError, "tap is blank!" if @tap.blank? path = @tap.new_formula_path(@name) raise "#{path} already exists" if path.exist?