(PUP-7310) Fix interpolation in externalized strings

lib/puppet/type/cron.rb

@@ -195,7 +195,7 @@ def should=(ary)
       if retval
         return retval.to_s
       else
-        self.fail _("#{value} is not a valid #{self.class.name}")
+        self.fail _("%{value} is not a valid %{name}") % { value: value, name: self.class.name }
       end
     end
   end
@@ -253,7 +253,7 @@ def specials
     end
 
     validate do |value|
-      raise ArgumentError, _("Invalid special schedule #{value.inspect}") unless specials.include?(value)
+      raise ArgumentError, _("Invalid special schedule %{value}") % { value: value.inspect } unless specials.include?(value)
     end
 
     def munge(value)
@@ -326,7 +326,7 @@ def alpha
 
     validate do |value|
       unless value =~ /^\s*(\w+)\s*=\s*(.*)\s*$/ or value == :absent or value == "absent"
-        raise ArgumentError, _("Invalid environment setting #{value.inspect}")
+        raise ArgumentError, _("Invalid environment setting %{value}") % { value: value.inspect }
       end
     end
 
@@ -433,7 +433,7 @@ def should_to_s(newvalue = @should)
     [ :minute, :hour, :weekday, :monthday, :month ].each do |field|
       next unless self[field]
       next if self[field] == :absent
-      raise ArgumentError, _("#{self.ref} cannot specify both a special schedule and a value for #{field}")
+      raise ArgumentError, _("%{cron} cannot specify both a special schedule and a value for %{field}") % { cron: self.ref, field: field }
     end
   end
 

lib/puppet/type/exec.rb

@@ -159,9 +159,9 @@ def sync
         unless self.should.include?(@status.exitstatus.to_s)
           if @resource.parameter(:command).sensitive
             # Don't print sensitive commands in the clear
-            self.fail(_("[command redacted] returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]"))
+            self.fail(_("[command redacted] returned %{status} instead of one of [%{expected}]") % { status: @status.exitstatus, expected: self.should.join(",") })
           else
-            self.fail(_("'#{self.resource[:command]}' returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]"))
+            self.fail(_("'%{cmd}' returned %{status} instead of one of [%{expected}]") % { cmd: self.resource[:command], status: @status.exitstatus, expected: self.should.join(",") })
           end
         end
 
@@ -179,7 +179,7 @@ def sync
         any output is logged at the `err` log level."
 
       validate do |command|
-        raise ArgumentError, _("Command must be a String, got value of class #{command.class}") unless command.is_a? String
+        raise ArgumentError, _("Command must be a String, got value of class %{klass}") % { klass: command.class } unless command.is_a? String
       end
     end
 
@@ -261,7 +261,7 @@ def value=(*values)
         values = [values] unless values.is_a? Array
         values.each do |value|
           unless value =~ /\w+=/
-            raise ArgumentError, _("Invalid environment setting '#{value}'")
+            raise ArgumentError, _("Invalid environment setting '%{value}'") % { value: value }
           end
         end
       end
@@ -274,7 +274,7 @@ def value=(*values)
         if value =~ /^0?[0-7]{1,4}$/
           return value.to_i(8)
         else
-          raise Puppet::Error, _("The umask specification is invalid: #{value.inspect}")
+          raise Puppet::Error, _("The umask specification is invalid: %{value}") % { value: value.inspect }
         end
       end
     end
@@ -443,7 +443,7 @@ def check(value)
         begin
           output, status = provider.run(value, true)
         rescue Timeout::Error
-          err _("Check #{value.inspect} exceeded timeout")
+          err _("Check %{value} exceeded timeout") % { value: value.inspect }
           return false
         end
 
@@ -494,7 +494,7 @@ def check(value)
         begin
           output, status = provider.run(value, true)
         rescue Timeout::Error
-          err _("Check #{value.inspect} exceeded timeout")
+          err _("Check %{value} exceeded timeout") % { value: value.inspect }
           return false
         end
 

lib/puppet/type/file.rb

@@ -53,7 +53,7 @@ def self.title_patterns
 
     validate do |value|
       unless Puppet::Util.absolute_path?(value)
-        fail Puppet::Error, _("File paths must be fully qualified, not '#{value}'")
+        fail Puppet::Error, _("File paths must be fully qualified, not '%{path}'") % { path: value }
       end
     end
 
@@ -121,7 +121,7 @@ def self.title_patterns
       when String
         value
       else
-        self.fail _("Invalid backup type #{value.inspect}")
+        self.fail _("Invalid backup type %{value}") % { value: value.inspect }
       end
     end
   end
@@ -165,7 +165,7 @@ def self.title_patterns
       when :false; false
       when :remote; :remote
       else
-        self.fail _("Invalid recurse value #{value.inspect}")
+        self.fail _("Invalid recurse value %{value}") % { value: value.inspect }
       end
     end
   end
@@ -198,7 +198,7 @@ def self.title_patterns
       when Integer; value
       when /^\d+$/; Integer(value)
       else
-        self.fail _("Invalid recurselimit value #{value.inspect}")
+        self.fail _("Invalid recurselimit value %{value}") % { value: value.inspect }
       end
     end
   end
@@ -368,14 +368,14 @@ def self.title_patterns
     end
     creator_count += 1 if @parameters.include?(:source)
 
-    self.fail _("You cannot specify more than one of #{CREATORS.collect { |p| p.to_s}.join(", ")}") if creator_count > 1
+    self.fail _("You cannot specify more than one of %{creators}") % { creators: CREATORS.collect { |p| p.to_s}.join(", ") } if creator_count > 1
 
     self.fail _("You cannot specify a remote recursion without a source") if !self[:source] && self[:recurse] == :remote
 
     self.fail _("You cannot specify source when using checksum 'none'") if self[:checksum] == :none && !self[:source].nil?
 
     SOURCE_ONLY_CHECKSUMS.each do |checksum_type|
-      self.fail _("You cannot specify content when using checksum '#{checksum_type}'") if self[:checksum] == checksum_type && !self[:content].nil?
+      self.fail _("You cannot specify content when using checksum '%{checksum_type}'") % { checksum_type: checksum_type } if self[:checksum] == checksum_type && !self[:content].nil?
     end
 
     self.warning _("Possible error: recurselimit is set but not recurse, no recursion will happen") if !self[:recurse] && self[:recurselimit]
@@ -386,7 +386,7 @@ def self.title_patterns
     end
 
     if self[:checksum] && self[:checksum_value] && !send("#{self[:checksum]}?", self[:checksum_value])
-      self.fail _("Checksum value '#{self[:checksum_value]}' is not a valid checksum type #{self[:checksum]}")
+      self.fail _("Checksum value '%{value}' is not a valid checksum type %{checksum}") % { value: self[:checksum_value], checksum: self[:checksum] }
     end
 
     self.warning _("Checksum value is ignored unless content or source are specified") if self[:checksum_value] && !self[:content] && !self[:source]
@@ -431,7 +431,7 @@ def bucket
     end
 
     unless catalog and filebucket = catalog.resource(:filebucket, backup) or backup == "puppet"
-      fail _("Could not find filebucket #{backup} specified in backup")
+      fail _("Could not find filebucket %{backup} specified in backup") % { backup: backup }
     end
 
     return default_bucket unless filebucket
@@ -737,7 +737,7 @@ def remove_existing(should)
     when "link", "file"
       return remove_file(current_type, wanted_type)
     else
-      self.fail _("Could not back up files of type #{current_type}")
+      self.fail _("Could not back up files of type %{current_type}") % { current_type: current_type }
     end
   end
 
@@ -989,7 +989,7 @@ def fail_if_checksum_is_wrong(path, content_checksum)
     newsum = parameter(:checksum).sum_file(path)
     return if [:absent, nil, content_checksum].include?(newsum)
 
-    self.fail _("File written to disk did not match checksum; discarding changes (#{content_checksum} vs #{newsum})")
+    self.fail _("File written to disk did not match checksum; discarding changes (%{content_checksum} vs %{newsum})") % { content_checksum: content_checksum, newsum: newsum }
   end
 
   def write_temporary_file?

lib/puppet/type/filebucket.rb

@@ -111,7 +111,7 @@ def mkbucket
       begin
         @bucket = Puppet::FileBucket::Dipper.new(args)
       rescue => detail
-        message = _("Could not create #{type} filebucket: #{detail}")
+        message = _("Could not create %{type} filebucket: %{detail}") % { type: type, detail: detail }
         self.log_exception(detail, message)
         self.fail(message)
       end

lib/puppet/type/group.rb

@@ -66,7 +66,7 @@ def sync
           if gid =~ /^[-0-9]+$/
             gid = Integer(gid)
           else
-            self.fail _("Invalid GID #{gid}")
+            self.fail _("Invalid GID %{gid}") % { gid: gid }
           end
         when Symbol
           unless gid == :absent

lib/puppet/type/host.rb

@@ -29,7 +29,7 @@ def valid_newline?(addr)
 
       validate do |value|
         return true if ((valid_v4?(value) || valid_v6?(value)) && (valid_newline?(value)))
-        raise Puppet::Error, _("Invalid IP address #{value.inspect}")
+        raise Puppet::Error, _("Invalid IP address %{value}") % { value: value.inspect }
       end
     end
 

lib/puppet/type/macauthorization.rb

@@ -31,7 +31,7 @@ def munge_boolean(value)
   def munge_integer(value)
       Integer(value)
   rescue ArgumentError
-      fail "munge_integer only takes integers")
+      fail "munge_integer only takes integers"
   end
 
   newparam(:name) do

lib/puppet/type/mailalias.rb

@@ -36,7 +36,7 @@ def should_to_s(value)
 
       validate do |value|
 	unless Puppet::Util.absolute_path?(value)
-	  fail Puppet::Error, _("File paths must be fully qualified, not '#{value}'")
+	  fail Puppet::Error, _("File paths must be fully qualified, not '%{value}'") % { value: value }
 	end
       end
     end

lib/puppet/type/maillist.rb

@@ -11,7 +11,7 @@ module Puppet
       end
 
       def change_to_s(current_value, newvalue)
-        return _("Purged #{resource}") if newvalue == :purged
+        return _("Purged %{resource}") % { resource: resource } if newvalue == :purged
         super
       end
 

lib/puppet/type/mount.rb

@@ -65,7 +65,7 @@ module Puppet
                      # the wrong attributes so I sync AFTER the umount
           return :mount_unmounted
         else
-          raise Puppet::Error, _("Unexpected change from #{current_value} to unmounted}")
+          raise Puppet::Error, _("Unexpected change from %{current} to unmounted}") % { current: current_value }
         end
       end
 
@@ -131,7 +131,7 @@ def syncothers
         path, depending on the operating system."
 
       validate do |value|
-        raise Puppet::Error, _("device must not contain whitespace: #{value}") if value =~ /\s/
+        raise Puppet::Error, _("device must not contain whitespace: %{value}") % { value: value } if value =~ /\s/
       end
     end
 
@@ -157,7 +157,7 @@ def syncothers
       end
 
       validate do |value|
-        raise Puppet::Error, _("blockdevice must not contain whitespace: #{value}") if value =~ /\s/
+        raise Puppet::Error, _("blockdevice must not contain whitespace: %{value}") % { value: value } if value =~ /\s/
       end
     end
 
@@ -166,7 +166,7 @@ def syncothers
         operating system.  This is a required option."
 
       validate do |value|
-        raise Puppet::Error, _("fstype must not contain whitespace: #{value}") if value =~ /\s/
+        raise Puppet::Error, _("fstype must not contain whitespace: %{value}") % { value: value } if value =~ /\s/
         raise Puppet::Error, _("fstype must not be an empty string") if value.empty?
       end
     end
@@ -177,7 +177,7 @@ def syncothers
         Consult the fstab(5) man page for system-specific details."
 
       validate do |value|
-        raise Puppet::Error, _("options must not contain whitespace: #{value}") if value =~ /\s/
+        raise Puppet::Error, _("options must not contain whitespace: %{value}") % { value: value } if value =~ /\s/
         raise Puppet::Error, _("options must not be an empty string") if value.empty?
       end
     end
@@ -243,7 +243,7 @@ def munge(value)
       isnamevar
 
       validate do |value|
-        raise Puppet::Error, _("name must not contain whitespace: #{value}") if value =~ /\s/
+        raise Puppet::Error, _("name must not contain whitespace: %{value}") % { value: value } if value =~ /\s/
       end
 
       munge do |value|

lib/puppet/type/package.rb

@@ -111,7 +111,7 @@ module Puppet
         begin
           provider.update
         rescue => detail
-          self.fail Puppet::Error, _("Could not update: #{detail}"), detail
+          self.fail Puppet::Error, _("Could not update: %{detail}") % { detail: detail }, detail
         end
 
         if current == :absent
@@ -125,7 +125,7 @@ module Puppet
         begin
           provider.install
         rescue => detail
-          self.fail Puppet::Error, _("Could not update: #{detail}"), detail
+          self.fail Puppet::Error, _("Could not update: %{detail}") % { detail: detail }, detail
         end
 
         if self.retrieve == :absent
@@ -160,7 +160,7 @@ def insync?(is)
                 @latest = provider.latest
                 @lateststamp = Time.now.to_i
               rescue => detail
-                error = Puppet::Error.new(_("Could not get latest version: #{detail}"))
+                error = Puppet::Error.new(_("Could not get latest version: %{detail}") % { detail: detail })
                 error.set_backtrace(detail.backtrace)
                 raise error
               end
@@ -260,7 +260,7 @@ def change_to_s(currentvalue, newvalue)
 
       validate do |value|
         if !value.is_a?(String)
-          raise ArgumentError, _("Name must be a String not #{value.class}")
+          raise ArgumentError, _("Name must be a String not %{klass}") % { klass: value.class }
         end
       end
     end

lib/puppet/type/resources.rb

@@ -12,7 +12,7 @@
     desc "The name of the type to be managed."
 
     validate do |name|
-      raise ArgumentError, _("Could not find resource type '#{name}'") unless Puppet::Type.type(name)
+      raise ArgumentError, _("Could not find resource type '%{name}'") % { name: name } unless Puppet::Type.type(name)
     end
 
     munge { |v| v.to_s }
@@ -30,7 +30,7 @@
     validate do |value|
       if munge(value)
         unless @resource.resource_type.respond_to?(:instances)
-          raise ArgumentError, _("Purging resources of type #{@resource[:name]} is not supported, since they cannot be queried from the system")
+          raise ArgumentError, _("Purging resources of type %{res_type} is not supported, since they cannot be queried from the system") % { res_type: @resource[:name] }
         end
         raise ArgumentError, _("Purging is only supported on types that accept 'ensure'") unless @resource.resource_type.validproperty?(:ensure)
       end
@@ -54,7 +54,7 @@
         false
       when Integer; value
       else
-        raise ArgumentError, _("Invalid value #{value.inspect}")
+        raise ArgumentError, _("Invalid value %{value}") % { value: value.inspect }
       end
     end
 
@@ -81,7 +81,7 @@
           when String
             Integer(v)
           else
-            raise ArgumentError, _("Invalid value #{v.inspect}.")
+            raise ArgumentError, _("Invalid value %{value}.") % { value: v.inspect }
         end
       end
     end
@@ -100,7 +100,7 @@ def check(resource)
   def able_to_ensure_absent?(resource)
       resource[:ensure] = :absent
   rescue ArgumentError, Puppet::Error
-      err _("The 'ensure' attribute on #{self[:name]} resources does not accept 'absent' as a value")
+      err _("The 'ensure' attribute on %{name} resources does not accept 'absent' as a value") % { name: self[:name] }
       false
   end