Refactor DSL and Artifacts.

9 files changed, 35 insertions, 42 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
index 1b18cc6e7..fa8fc2996 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
@@ -20,7 +20,7 @@ module Hbc
       end
 
       def self.for_cask(cask)
-        cask.artifacts[dsl_key].to_a
+        cask.artifacts[self].to_a
       end
 
       # TODO: this sort of logic would make more sense in dsl.rb, or a
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb
index 4e8edbc11..a3075ff40 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb
@@ -11,12 +11,6 @@ module Hbc
         dsl_key.to_s.prepend("uninstall_").to_sym
       end
 
-      def self.for_cask(cask)
-        [dsl_key, uninstall_dsl_key].flat_map do |key|
-          [*cask.artifacts[key]].map { |block| new(cask, key => block) }
-        end
-      end
-
       attr_reader :directives
 
       def initialize(cask, **directives)
diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb
index 9ab93a67f..2d8e4e4d7 100644
--- a/Library/Homebrew/cask/lib/hbc/audit.rb
+++ b/Library/Homebrew/cask/lib/hbc/audit.rb
@@ -218,7 +218,7 @@ module Hbc
     end
 
     def check_generic_artifacts
-      cask.artifacts[:artifact].each do |artifact|
+      cask.artifacts[Hbc::Artifact::Artifact].each do |artifact|
         unless artifact.target.absolute?
           add_error "target must be absolute path for #{artifact.class.english_name} #{artifact.source}"
         end
diff --git a/Library/Homebrew/cask/lib/hbc/container/base.rb b/Library/Homebrew/cask/lib/hbc/container/base.rb
index 52eb5aab1..4363168e7 100644
--- a/Library/Homebrew/cask/lib/hbc/container/base.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/base.rb
@@ -20,7 +20,7 @@ module Hbc
 
         unless children.count == 1 &&
                !nested_container.directory? &&
-               @cask.artifacts[:nested_container].empty? &&
+               Artifact::NestedContainer.for_cask(@cask).none? &&
                extract_nested_container(nested_container)
 
           children.each do |src|
diff --git a/Library/Homebrew/cask/lib/hbc/dsl.rb b/Library/Homebrew/cask/lib/hbc/dsl.rb
index 8d5b6c9a9..75c1ca873 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl.rb
@@ -43,7 +43,7 @@ module Hbc
       Artifact::Zap,
     ].freeze
 
-    ACTIVATABLE_ARTIFACT_TYPES = (ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key) - [:stage_only]).freeze
+    ACTIVATABLE_ARTIFACT_CLASSES = ORDINARY_ARTIFACT_CLASSES - [Artifact::StageOnly]
 
     ARTIFACT_BLOCK_CLASSES = [
       Artifact::PreflightBlock,
@@ -71,11 +71,12 @@ module Hbc
       :version,
       :appdir,
       *ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key),
-      *ACTIVATABLE_ARTIFACT_TYPES,
+      *ACTIVATABLE_ARTIFACT_CLASSES.map(&:dsl_key),
       *ARTIFACT_BLOCK_CLASSES.flat_map { |klass| [klass.dsl_key, klass.uninstall_dsl_key] },
     ].freeze
 
-    attr_reader :token, :cask
+    attr_reader :cask, :token
+
     def initialize(cask)
       @cask = cask
       @token = cask.token
@@ -175,7 +176,7 @@ module Hbc
           DSL::Container.new(*args).tap do |container|
             # TODO: remove this backward-compatibility section after removing nested_container
             if container&.nested
-              artifacts[:nested_container] << Artifact::NestedContainer.new(cask, container.nested)
+              artifacts[Artifact::NestedContainer] << Artifact::NestedContainer.new(cask, container.nested)
             end
           end
         end
@@ -250,15 +251,13 @@ module Hbc
     end
 
     ORDINARY_ARTIFACT_CLASSES.each do |klass|
-      type = klass.dsl_key
-
-      define_method(type) do |*args|
+      define_method(klass.dsl_key) do |*args|
         begin
-          if [*artifacts.keys, type].include?(:stage_only) && (artifacts.keys & ACTIVATABLE_ARTIFACT_TYPES).any?
+          if [*artifacts.keys, klass].include?(Artifact::StageOnly) && (artifacts.keys & ACTIVATABLE_ARTIFACT_CLASSES).any?
             raise CaskInvalidError.new(cask, "'stage_only' must be the only activatable artifact.")
           end
 
-          artifacts[type].add(klass.from_args(cask, *args))
+          artifacts[klass].add(klass.from_args(cask, *args))
         rescue CaskInvalidError
           raise
         rescue StandardError => e
@@ -270,7 +269,7 @@ module Hbc
     ARTIFACT_BLOCK_CLASSES.each do |klass|
       [klass.dsl_key, klass.uninstall_dsl_key].each do |dsl_key|
         define_method(dsl_key) do |&block|
-          artifacts[dsl_key] << block
+          artifacts[klass] << klass.new(cask, dsl_key => block)
         end
       end
     end
diff --git a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
index f984ac088..f3994b81f 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
@@ -3,17 +3,17 @@ require "hbc/system_command"
 module Hbc
   class DSL
     class Appcast
-      attr_reader :parameters, :checkpoint
+      attr_reader :uri, :checkpoint, :parameters
 
-      def initialize(uri, parameters = {})
-        @parameters     = parameters
-        @uri            = URI(uri)
-        @checkpoint     = @parameters[:checkpoint]
+      def initialize(uri, **parameters)
+        @uri        = URI(uri)
+        @parameters = parameters
+        @checkpoint = parameters[:checkpoint]
       end
 
       def calculate_checkpoint
         curl_executable, *args = curl_args(
-          "--compressed", "--location", "--fail", @uri,
+          "--compressed", "--location", "--fail", uri,
           user_agent: :fake
         )
         result = SystemCommand.run(curl_executable, args: args, print_stderr: false)
@@ -30,11 +30,11 @@ module Hbc
       end
 
       def to_yaml
-        [@uri, @parameters].to_yaml
+        [uri, parameters].to_yaml
       end
 
       def to_s
-        @uri.to_s
+        uri.to_s
       end
     end
   end
diff --git a/Library/Homebrew/cask/lib/hbc/dsl/base.rb b/Library/Homebrew/cask/lib/hbc/dsl/base.rb
index 63df847e9..b97c4e104 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl/base.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl/base.rb
@@ -10,14 +10,14 @@ module Hbc
 
       def_delegators :@cask, :token, :version, :caskroom_path, :staged_path, :appdir, :language
 
-      def system_command(executable, options = {})
-        @command.run!(executable, options)
+      def system_command(executable, **options)
+        @command.run!(executable, **options)
       end
 
       def method_missing(method, *)
         if method
           underscored_class = self.class.name.gsub(/([[:lower:]])([[:upper:]][[:lower:]])/, '\1_\2').downcase
-          section = underscored_class.downcase.split("::").last
+          section = underscored_class.split("::").last
           Utils.method_missing_message(method, @cask.to_s, section)
           nil
         else
diff --git a/Library/Homebrew/cask/lib/hbc/staged.rb b/Library/Homebrew/cask/lib/hbc/staged.rb
index dc21279de..6fd3f6709 100644
--- a/Library/Homebrew/cask/lib/hbc/staged.rb
+++ b/Library/Homebrew/cask/lib/hbc/staged.rb
@@ -4,7 +4,7 @@ module Hbc
       index =  0 if index == :first
       index =  1 if index == :second
       index = -1 if index == :last
-      @cask.artifacts[:app].to_a.at(index).target.join("Contents", "Info.plist")
+      @cask.artifacts[Artifact::App].to_a.at(index).target.join("Contents", "Info.plist")
     end
 
     def plist_exec(cmd)
diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb
index 7df8de6f8..e03af9aef 100644
--- a/Library/Homebrew/test/cask/dsl_spec.rb
+++ b/Library/Homebrew/test/cask/dsl_spec.rb
@@ -216,12 +216,12 @@ describe Hbc::DSL, :cask do
         app "Bar.app"
       end
 
-      expect(cask.artifacts[:app].map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"])
+      expect(cask.artifacts[Hbc::Artifact::App].map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"])
     end
 
     it "allow app stanzas to be empty" do
       cask = Hbc::Cask.new("cask-with-no-apps")
-      expect(cask.artifacts[:app]).to be_empty
+      expect(cask.artifacts[Hbc::Artifact::App]).to be_empty
     end
   end
 
@@ -249,7 +249,7 @@ describe Hbc::DSL, :cask do
         pkg "Bar.pkg"
       end
 
-      expect(cask.artifacts[:pkg].map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"])
+      expect(cask.artifacts[Hbc::Artifact::Pkg].map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"])
     end
   end
 
@@ -501,10 +501,10 @@ describe Hbc::DSL, :cask do
       let(:token) { "with-installer-script" }
 
       it "allows installer script to be specified" do
-        expect(cask.artifacts[:installer].first.path).to eq(Pathname("/usr/bin/true"))
-        expect(cask.artifacts[:installer].first.args[:args]).to eq(["--flag"])
-        expect(cask.artifacts[:installer].to_a[1].path).to eq(Pathname("/usr/bin/false"))
-        expect(cask.artifacts[:installer].to_a[1].args[:args]).to eq(["--flag"])
+        expect(cask.artifacts[Hbc::Artifact::Installer].first.path).to eq(Pathname("/usr/bin/true"))
+        expect(cask.artifacts[Hbc::Artifact::Installer].first.args[:args]).to eq(["--flag"])
+        expect(cask.artifacts[Hbc::Artifact::Installer].to_a[1].path).to eq(Pathname("/usr/bin/false"))
+        expect(cask.artifacts[Hbc::Artifact::Installer].to_a[1].args[:args]).to eq(["--flag"])
       end
     end
 
@@ -512,7 +512,7 @@ describe Hbc::DSL, :cask do
       let(:token) { "with-installer-manual" }
 
       it "allows installer manual to be specified" do
-        installer = cask.artifacts[:installer].first
+        installer = cask.artifacts[Hbc::Artifact::Installer].first
         expect(installer).to be_a(Hbc::Artifact::Installer::ManualInstaller)
         expect(installer.path).to eq(cask.staged_path.join("Caffeine.app"))
       end
@@ -524,7 +524,7 @@ describe Hbc::DSL, :cask do
       let(:token) { "stage-only" }
 
       it "allows stage_only stanza to be specified" do
-        expect(cask.artifacts[:stage_only]).not_to be_empty
+        expect(cask.artifacts[Hbc::Artifact::StageOnly]).not_to be_empty
       end
     end
 
@@ -550,7 +550,7 @@ describe Hbc::DSL, :cask do
       let(:token) { "appdir-interpolation" }
 
       it "is allowed" do
-        expect(cask.artifacts[:binary].first.source).to eq(Hbc.appdir/"some/path")
+        expect(cask.artifacts[Hbc::Artifact::Binary].first.source).to eq(Hbc.appdir/"some/path")
       end
     end
 
@@ -563,7 +563,7 @@ describe Hbc::DSL, :cask do
           binary "#{appdir}/some/path"
         end
 
-        expect(cask.artifacts[:binary].first.source).to eq(original_appdir/"some/path")
+        expect(cask.artifacts[Hbc::Artifact::Binary].first.source).to eq(original_appdir/"some/path")
       ensure
         Hbc.appdir = original_appdir
       end