unifying interface of Psych

Author: Greg Houle

lib/psych.rb

@@ -230,8 +230,9 @@
 module Psych
   # The version of libyaml Psych is using
   LIBYAML_VERSION = Psych.libyaml_version.join '.'
-
-  FALLBACK        = Struct.new :to_ruby # :nodoc:
+  # Deprecation guard
+  NOT_GIVEN = Object.new
+  private_constant :NOT_GIVEN
 
   ###
   # Load +yaml+ in to a Ruby data structure.  If multiple documents are
@@ -247,7 +248,7 @@ module Psych
   #   Psych.load("---\n - a\n - b")   # => ['a', 'b']
   #
   #   begin
-  #     Psych.load("--- `", "file.txt")
+  #     Psych.load("--- `", filename: "file.txt")
   #   rescue Psych::SyntaxError => ex
   #     ex.file    # => 'file.txt'
   #     ex.message # => "(file.txt): found character that cannot start any token"
@@ -259,8 +260,16 @@ module Psych
   #   Psych.load("---\n foo: bar")                         # => {"foo"=>"bar"}
   #   Psych.load("---\n foo: bar", symbolize_names: true)  # => {:foo=>"bar"}
   #
-  def self.load yaml, filename = nil, fallback: false, symbolize_names: false
-    result = parse(yaml, filename, fallback: fallback)
+  # Raises a TypeError when `yaml` parameter is NilClass
+  #
+  def self.load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false
+    if legacy_filename != NOT_GIVEN
+      warn 'warning: Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.'
+      filename = legacy_filename
+    end
+
+    result = parse(yaml, filename: filename)
+    return fallback unless result
     result = result.to_ruby if result
     symbolize_names!(result) if symbolize_names
     result
@@ -279,27 +288,27 @@ def self.load yaml, filename = nil, fallback: false, symbolize_names: false
   # * Hash
   #
   # Recursive data structures are not allowed by default.  Arbitrary classes
-  # can be allowed by adding those classes to the +whitelist+.  They are
+  # can be allowed by adding those classes to the +whitelist_classes+ keyword argument.  They are
   # additive.  For example, to allow Date deserialization:
   #
-  #   Psych.safe_load(yaml, [Date])
+  #   Psych.safe_load(yaml, whitelist_classes: [Date])
   #
   # Now the Date class can be loaded in addition to the classes listed above.
   #
-  # Aliases can be explicitly allowed by changing the +aliases+ parameter.
+  # Aliases can be explicitly allowed by changing the +aliases+ keyword argument.
   # For example:
   #
   #   x = []
   #   x << x
   #   yaml = Psych.dump x
   #   Psych.safe_load yaml               # => raises an exception
-  #   Psych.safe_load yaml, [], [], true # => loads the aliases
+  #   Psych.safe_load yaml, aliases: true # => loads the aliases
   #
   # A Psych::DisallowedClass exception will be raised if the yaml contains a
   # class that isn't in the whitelist.
   #
   # A Psych::BadAlias exception will be raised if the yaml contains aliases
-  # but the +aliases+ parameter is set to false.
+  # but the +aliases+ keyword argument is set to false.
   #
   # +filename+ will be used in the exception message if any exception is raised
   # while parsing.
@@ -310,18 +319,38 @@ def self.load yaml, filename = nil, fallback: false, symbolize_names: false
   #   Psych.safe_load("---\n foo: bar")                         # => {"foo"=>"bar"}
   #   Psych.safe_load("---\n foo: bar", symbolize_names: true)  # => {:foo=>"bar"}
   #
-  def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil, symbolize_names: false
-    result = parse(yaml, filename)
-    return unless result
+  def self.safe_load yaml, legacy_whitelist_classes = NOT_GIVEN, legacy_whitelist_symbols = NOT_GIVEN, legacy_aliases = NOT_GIVEN, legacy_filename = NOT_GIVEN, whitelist_classes: [], whitelist_symbols: [], aliases: false, filename: nil, fallback: false, symbolize_names: false
+    if legacy_whitelist_classes != NOT_GIVEN
+      warn 'warning: Passing whitelist_classes with the 2nd argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, whitelist_classes: ...) instead.'
+      whitelist_classes = legacy_whitelist_classes
+    end
+
+    if legacy_whitelist_symbols != NOT_GIVEN
+      warn 'warning: Passing whitelist_symbols with the 3rd argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, whitelist_symbols: ...) instead.'
+      whitelist_symbols = legacy_whitelist_symbols
+    end
+
+    if legacy_aliases != NOT_GIVEN
+      warn 'warning: Passing aliases with the 4th argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, aliases: ...) instead.'
+      aliases = legacy_aliases
+    end
+
+    if legacy_filename != NOT_GIVEN
+      warn 'warning: Passing filename with the 5th argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, filename: ...) instead.'
+      filename = legacy_filename
+    end
+
+    result = parse(yaml, filename: filename)
+    return fallback unless result
 
     class_loader = ClassLoader::Restricted.new(whitelist_classes.map(&:to_s),
                                                whitelist_symbols.map(&:to_s))
     scanner      = ScalarScanner.new class_loader
-    if aliases
-      visitor = Visitors::ToRuby.new scanner, class_loader
-    else
-      visitor = Visitors::NoAliasRuby.new scanner, class_loader
-    end
+    visitor = if aliases
+                Visitors::ToRuby.new scanner, class_loader
+              else
+                Visitors::NoAliasRuby.new scanner, class_loader
+              end
     result = visitor.accept result
     symbolize_names!(result) if symbolize_names
     result
@@ -339,28 +368,40 @@ def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases
   #   Psych.parse("---\n - a\n - b") # => #<Psych::Nodes::Document:0x00>
   #
   #   begin
-  #     Psych.parse("--- `", "file.txt")
+  #     Psych.parse("--- `", filename: "file.txt")
   #   rescue Psych::SyntaxError => ex
   #     ex.file    # => 'file.txt'
   #     ex.message # => "(file.txt): found character that cannot start any token"
   #   end
   #
   # See Psych::Nodes for more information about YAML AST.
-  def self.parse yaml, filename = nil, fallback: false
-    parse_stream(yaml, filename) do |node|
+  def self.parse yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: NOT_GIVEN
+    if legacy_filename != NOT_GIVEN
+      warn 'warning: Passing filename with the 2nd argument of Psych.parse is deprecated. Use keyword argument like Psych.parse(yaml, filename: ...) instead.'
+      filename = legacy_filename
+    end
+
+    parse_stream(yaml, filename: filename) do |node|
       return node
     end
-    fallback
+
+    if fallback != NOT_GIVEN
+      warn 'warning: Passing the `fallback` keyword argument of Psych.parse is deprecated.'
+      fallback
+    else
+      false
+    end
   end
 
   ###
   # Parse a file at +filename+. Returns the Psych::Nodes::Document.
   #
   # Raises a Psych::SyntaxError when a YAML syntax error is detected.
-  def self.parse_file filename
-    File.open filename, 'r:bom|utf-8' do |f|
-      parse f, filename
+  def self.parse_file filename, fallback: false
+    result = File.open filename, 'r:bom|utf-8' do |f|
+      parse f, filename: filename
     end
+    result || fallback
   end
 
   ###
@@ -389,14 +430,21 @@ def self.parser
   #   end
   #
   #   begin
-  #     Psych.parse_stream("--- `", "file.txt")
+  #     Psych.parse_stream("--- `", filename: "file.txt")
   #   rescue Psych::SyntaxError => ex
   #     ex.file    # => 'file.txt'
   #     ex.message # => "(file.txt): found character that cannot start any token"
   #   end
   #
+  # Raises a TypeError when NilClass is passed.
+  #
   # See Psych::Nodes for more information about YAML AST.
-  def self.parse_stream yaml, filename = nil, &block
+  def self.parse_stream yaml, legacy_filename = NOT_GIVEN, filename: nil, &block
+    if legacy_filename != NOT_GIVEN
+      warn 'warning: Passing filename with the 2nd argument of Psych.parse_stream is deprecated. Use keyword argument like Psych.parse_stream(yaml, filename: ...) instead.'
+      filename = legacy_filename
+    end
+
     if block_given?
       parser = Psych::Parser.new(Handlers::DocumentStream.new(&block))
       parser.parse yaml, filename
@@ -497,14 +545,21 @@ def self.to_json object
   #   end
   #   list # => ['foo', 'bar']
   #
-  def self.load_stream yaml, filename = nil
-    if block_given?
-      parse_stream(yaml, filename) do |node|
-        yield node.to_ruby
-      end
-    else
-      parse_stream(yaml, filename).children.map { |child| child.to_ruby }
+  def self.load_stream yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false
+    if legacy_filename != NOT_GIVEN
+      warn 'warning: Passing filename with the 2nd argument of Psych.load_stream is deprecated. Use keyword argument like Psych.load_stream(yaml, filename: ...) instead.'
+      filename = legacy_filename
     end
+
+    result = if block_given?
+               parse_stream(yaml, filename: filename) do |node|
+                 yield node.to_ruby
+               end
+             else
+               parse_stream(yaml, filename: filename).children.map(&:to_ruby)
+             end
+
+    result || fallback
   end
 
   ###
@@ -513,7 +568,7 @@ def self.load_stream yaml, filename = nil
   # the specified +fallback+ return value, which defaults to +false+.
   def self.load_file filename, fallback: false
     File.open(filename, 'r:bom|utf-8') { |f|
-      self.load f, filename, fallback: FALLBACK.new(fallback)
+      self.load f, filename: filename, fallback: fallback
     }
   end
 

test/psych/test_exception.rb

@@ -30,7 +30,7 @@ def test_load_takes_file
       assert_nil ex.file
 
       ex = assert_raises(Psych::SyntaxError) do
-        Psych.load '--- `', 'meow'
+        Psych.load '--- `', filename: 'meow'
       end
       assert_equal 'meow', ex.file
     end
@@ -43,7 +43,7 @@ def test_psych_parse_stream_takes_file
       assert_match '(<unknown>)', ex.message
 
       ex = assert_raises(Psych::SyntaxError) do
-        Psych.parse_stream '--- `', 'omg!'
+        Psych.parse_stream '--- `', filename: 'omg!'
       end
       assert_equal 'omg!', ex.file
       assert_match 'omg!', ex.message
@@ -57,7 +57,7 @@ def test_load_stream_takes_file
       assert_match '(<unknown>)', ex.message
 
       ex = assert_raises(Psych::SyntaxError) do
-        Psych.load_stream '--- `', 'omg!'
+        Psych.load_stream '--- `', filename: 'omg!'
       end
       assert_equal 'omg!', ex.file
     end
@@ -94,7 +94,7 @@ def test_psych_parse_takes_file
       assert_nil ex.file
 
       ex = assert_raises(Psych::SyntaxError) do
-        Psych.parse '--- `', 'omg!'
+        Psych.parse '--- `', filename: 'omg!'
       end
       assert_match 'omg!', ex.message
     end

test/psych/test_safe_load.rb

@@ -22,62 +22,69 @@ def test_no_recursion
     def test_explicit_recursion
       x = []
       x << x
-      assert_equal(x, Psych.safe_load(Psych.dump(x), [], [], true))
+      assert_equal(x, Psych.safe_load(Psych.dump(x), whitelist_classes: [], whitelist_symbols: [], aliases: true))
     end
 
     def test_symbol_whitelist
       yml = Psych.dump :foo
       assert_raises(Psych::DisallowedClass) do
         Psych.safe_load yml
       end
-      assert_equal(:foo, Psych.safe_load(yml, [Symbol], [:foo]))
+      assert_equal(
+        :foo,
+        Psych.safe_load(
+          yml,
+          whitelist_classes: [Symbol],
+          whitelist_symbols: [:foo]
+        )
+      )
     end
 
     def test_symbol
       assert_raises(Psych::DisallowedClass) do
         assert_safe_cycle :foo
       end
       assert_raises(Psych::DisallowedClass) do
-        Psych.safe_load '--- !ruby/symbol foo', []
+        Psych.safe_load '--- !ruby/symbol foo', whitelist_classes: []
       end
-      assert_safe_cycle :foo, [Symbol]
-      assert_safe_cycle :foo, %w{ Symbol }
-      assert_equal :foo, Psych.safe_load('--- !ruby/symbol foo', [Symbol])
+      assert_safe_cycle :foo, whitelist_classes: [Symbol]
+      assert_safe_cycle :foo, whitelist_classes: %w{ Symbol }
+      assert_equal :foo, Psych.safe_load('--- !ruby/symbol foo', whitelist_classes: [Symbol])
     end
 
     def test_foo
       assert_raises(Psych::DisallowedClass) do
-        Psych.safe_load '--- !ruby/object:Foo {}', [Foo]
+        Psych.safe_load '--- !ruby/object:Foo {}', whitelist_classes: [Foo]
       end
       assert_raises(Psych::DisallowedClass) do
         assert_safe_cycle Foo.new
       end
-      assert_kind_of(Foo, Psych.safe_load(Psych.dump(Foo.new), [Foo]))
+      assert_kind_of(Foo, Psych.safe_load(Psych.dump(Foo.new), whitelist_classes: [Foo]))
     end
 
     X = Struct.new(:x)
     def test_struct_depends_on_sym
-      assert_safe_cycle(X.new, [X, Symbol])
+      assert_safe_cycle(X.new, whitelist_classes: [X, Symbol])
       assert_raises(Psych::DisallowedClass) do
-        cycle X.new, [X]
+        cycle X.new, whitelist_classes: [X]
       end
     end
 
     def test_anon_struct
-      assert Psych.safe_load(<<-eoyml, [Struct, Symbol])
+      assert Psych.safe_load(<<-eoyml, whitelist_classes: [Struct, Symbol])
 --- !ruby/struct
   foo: bar
                       eoyml
 
       assert_raises(Psych::DisallowedClass) do
-        Psych.safe_load(<<-eoyml, [Struct])
+        Psych.safe_load(<<-eoyml, whitelist_classes: [Struct])
 --- !ruby/struct
   foo: bar
                       eoyml
       end
 
       assert_raises(Psych::DisallowedClass) do
-        Psych.safe_load(<<-eoyml, [Symbol])
+        Psych.safe_load(<<-eoyml, whitelist_classes: [Symbol])
 --- !ruby/struct
   foo: bar
                       eoyml
@@ -86,12 +93,12 @@ def test_anon_struct
 
     private
 
-    def cycle object, whitelist = []
-      Psych.safe_load(Psych.dump(object), whitelist)
+    def cycle object, whitelist_classes: []
+      Psych.safe_load(Psych.dump(object), whitelist_classes: whitelist_classes)
     end
 
-    def assert_safe_cycle object, whitelist = []
-      other = cycle object, whitelist
+    def assert_safe_cycle object, whitelist_classes: []
+      other = cycle object, whitelist_classes: whitelist_classes
       assert_equal object, other
     end
   end