From 4967f89a769a7b5c6ca91377526e3f53ceabd6a3 Mon Sep 17 00:00:00 2001
From: Josh Holtrop <jholtrop@gmail.com>
Date: Sun, 14 Apr 2019 14:10:13 -0400
Subject: [PATCH] fix inconsistent type handling in VarSet#expand_varref - #73

---
 lib/rscons/varset.rb            | 26 +++++++++++---------------
 spec/rscons/environment_spec.rb |  2 +-
 spec/rscons/varset_spec.rb      | 17 +++++++++++++++--
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/lib/rscons/varset.rb b/lib/rscons/varset.rb
index f9183e7..267a8a5 100644
--- a/lib/rscons/varset.rb
+++ b/lib/rscons/varset.rb
@@ -94,46 +94,42 @@ module Rscons
     # @return [nil, String, Array, Symbol, TrueClass, FalseClass]
     #   Expanded value with "$!{var}" variable references replaced.
     def expand_varref(varref, lambda_args)
-      if varref.is_a?(String)
+      case varref
+      when Symbol, true, false, nil
+        varref
+      when String
         if varref =~ /^(.*)\$\{([^}]+)\}(.*)$/
           prefix, varname, suffix = $1, $2, $3
           prefix = expand_varref(prefix, lambda_args) unless prefix.empty?
           varval = expand_varref(self[varname], lambda_args)
           # suffix needs no expansion since the regex matches the last occurence
-          if varval.is_a?(String) or varval.nil?
+          case varval
+          when Symbol, true, false, nil, String
             if prefix.is_a?(Array)
               prefix.map {|p| "#{p}#{varval}#{suffix}"}
             else
               "#{prefix}#{varval}#{suffix}"
             end
-          elsif varval.is_a?(Array)
+          when Array
             if prefix.is_a?(Array)
               varval.map {|vv| prefix.map {|p| "#{p}#{vv}#{suffix}"}}.flatten
             else
               varval.map {|vv| "#{prefix}#{vv}#{suffix}"}
             end
           else
-            raise "I do not know how to expand a variable reference to a #{varval.class.name} (from #{varname.inspect} => #{self[varname].inspect})"
+            raise "Unknown construction variable type: #{varval.class} (from #{varname.inspect} => #{self[varname].inspect})"
           end
         else
           varref
         end
-      elsif varref.is_a?(Array)
+      when Array
         varref.map do |ent|
           expand_varref(ent, lambda_args)
         end.flatten
-      elsif varref.is_a?(Proc)
+      when Proc
         expand_varref(varref[*lambda_args], lambda_args)
-      elsif varref.nil?
-        nil
-      elsif varref.is_a?(Symbol)
-        varref
-      elsif varref.is_a?(TrueClass)
-        varref
-      elsif varref.is_a?(FalseClass)
-        varref
       else
-        raise "Unknown varref type: #{varref.class} (#{varref.inspect})"
+        raise "Unknown construction variable type: #{varref.class} (#{varref.inspect})"
       end
     end
 
diff --git a/spec/rscons/environment_spec.rb b/spec/rscons/environment_spec.rb
index c449cd7..223bfef 100644
--- a/spec/rscons/environment_spec.rb
+++ b/spec/rscons/environment_spec.rb
@@ -184,7 +184,7 @@ module Rscons
         env["foo"] = {}
         expect(env.expand_varref(["-p${path}", "${flags}"])).to eq ["-pdir1", "-pdir2", "-x", "-y", "-z"]
         expect(env.expand_varref("foo")).to eq "foo"
-        expect {env.expand_varref("${foo}")}.to raise_error /Unknown.varref.type/
+        expect {env.expand_varref("${foo}")}.to raise_error /Unknown.construction.variable.type/
         expect(env.expand_varref("${specialflag}")).to eq "-z"
         expect(env.expand_varref("${path}")).to eq ["dir1", "dir2"]
       end
diff --git a/spec/rscons/varset_spec.rb b/spec/rscons/varset_spec.rb
index 5d8bee5..ba920ef 100644
--- a/spec/rscons/varset_spec.rb
+++ b/spec/rscons/varset_spec.rb
@@ -166,13 +166,26 @@ module Rscons
         expect(v.expand_varref(false, :lambda_args)).to eq(false)
       end
       it "raises an error when given an invalid argument" do
-        expect { v.expand_varref({a: :b}, :lambda_args) }.to raise_error /Unknown varref type: Hash/
+        expect { v.expand_varref({a: :b}, :lambda_args) }.to raise_error /Unknown construction variable type: Hash/
       end
       it "raises an error when an expanded variable is an unexpected type" do
         expect(v).to receive(:[]).at_least(1).times.with("bad").and_return("bad_val")
         expect(v).to receive(:expand_varref).with("bad_val", :lambda_args).and_return({a: :b})
         expect(v).to receive(:expand_varref).and_call_original
-        expect { v.expand_varref("${bad}", :lambda_args) }.to raise_error /I do not know how to expand a variable reference to a Hash/
+        expect { v.expand_varref("${bad}", :lambda_args) }.to raise_error /Unknown construction variable type: Hash/
+      end
+      it "expands symbols within a string outside an array" do
+        v['var'] = :a_symbol
+        expect(v.expand_varref("this is ${var}", :lambda_args)).to eq "this is a_symbol"
+      end
+      it "expands booleans within a string outside an array" do
+        v['var'] = false
+        expect(v.expand_varref("this is ${var}", :lambda_args)).to eq "this is false"
+      end
+      it "expands symbols and booleans in an array" do
+        v['var'] = [:a_symbol, false]
+        expanded = v.expand_varref("this is ${var}", :lambda_args)
+        expect(expanded).to eq(["this is a_symbol", "this is false"])
       end
     end