Unify flow for code flow for adding rule

lib/css_parser/parser.rb

@@ -172,28 +172,36 @@ def add_block!(block, options = {})
     # optional fields for source location for source location
     # +filename+ can be a string or uri pointing to the file or url location.
     # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file.
-    def append_rule!(selectors: nil, block: nil, filename: nil, offset: nil, media_types: :all)
-      rule_set = RuleSet.new(
-        selectors: selectors, block: block,
-        offset: offset, filename: filename
-      )
+    def add_rule!(*args, selectors: nil, block: nil, filename: nil, offset: nil, media_types: :all) # rubocop:disable Metrics/ParameterLists
+      if args.any?
+        media_types = nil
+        if selectors || block || filename || offset || media_types
+          raise ArgumentError, "don't mix positional and keyword arguments arguments"
+        end
 
-      add_rule_set!(rule_set, media_types)
-    rescue ArgumentError => e
-      raise e if @options[:rule_set_exceptions]
-    end
+        warn '[DEPRECATION] `add_rule!` with positional arguments is deprecated. ' \
+             'Please use keyword arguments instead.', uplevel: 1
 
-    # Add a CSS rule by setting the +selectors+, +declarations+ and +media_types+.
-    #
-    # +media_types+ can be a symbol or an array of symbols. default to :all
-    # optional fields for source location for source location
-    # +filename+ can be a string or uri pointing to the file or url location.
-    # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file.
-
-    def add_rule!(selectors, declarations, media_types = :all)
-      warn '[DEPRECATION] `add_rule!` is deprecated. Please use `append_rule!` instead.', uplevel: 1
+        case args.length
+        when 2
+          selectors, block = args
+        when 3
+          selectors, block, media_types = args
+        else
+          raise ArgumentError
+        end
+      end
 
-      append_rule!(selectors: selectors, block: declarations, media_types: media_types)
+      begin
+        rule_set = RuleSet.new(
+          selectors: selectors, block: block,
+          offset: offset, filename: filename
+        )
+
+        add_rule_set!(rule_set, media_types)
+      rescue ArgumentError => e
+        raise e if @options[:rule_set_exceptions]
+      end
     end
 
     # Add a CSS rule by setting the +selectors+, +declarations+, +filename+, +offset+ and +media_types+.
@@ -202,8 +210,8 @@ def add_rule!(selectors, declarations, media_types = :all)
     # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file.
     # +media_types+ can be a symbol or an array of symbols.
     def add_rule_with_offsets!(selectors, declarations, filename, offset, media_types = :all)
-      warn '[DEPRECATION] `add_rule_with_offsets!` is deprecated. Please use `append_rule!` instead.', uplevel: 1
-      append_rule!(
+      warn '[DEPRECATION] `add_rule_with_offsets!` is deprecated. Please use `add_rule!` instead.', uplevel: 1
+      add_rule!(
         selectors: selectors, block: declarations, media_types: media_types,
         filename: filename, offset: offset
       )
@@ -385,11 +393,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
             current_declarations.strip!
 
             unless current_declarations.empty?
+              add_rule_options = {
+                selectors: current_selectors, block: current_declarations,
+                media_types: current_media_queries
+              }
               if options[:capture_offsets]
-                add_rule_with_offsets!(current_selectors, current_declarations, options[:filename], (rule_start..offset.last), current_media_queries)
-              else
-                add_rule!(current_selectors, current_declarations, current_media_queries)
+                add_rule_options.merge!(filename: options[:filename], offset: rule_start..offset.last)
               end
+              add_rule!(**add_rule_options)
             end
 
             current_selectors = String.new
@@ -455,11 +466,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
       # check for unclosed braces
       return unless in_declarations > 0
 
-      unless options[:capture_offsets]
-        return add_rule!(current_selectors, current_declarations, current_media_queries)
+      add_rule_options = {
+        selectors: current_selectors, block: current_declarations,
+        media_types: current_media_queries
+      }
+      if options[:capture_offsets]
+        add_rule_options.merge!(filename: options[:filename], offset: rule_start..offset.last)
       end
-
-      add_rule_with_offsets!(current_selectors, current_declarations, options[:filename], (rule_start..offset.last), current_media_queries)
+      add_rule!(**add_rule_options)
     end
 
     # Load a remote CSS file.

lib/css_parser/rule_set.rb

@@ -269,7 +269,7 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp
       @specificity = specificity
 
       unless offset.nil? == filename.nil?
-        raise ArgumentError, 'require both offset and filename'
+        raise ArgumentError, 'require both offset and filename or no offset and no filename'
       end
 
       @offset = offset

test/test_css_parser_basic.rb

@@ -35,7 +35,7 @@ def test_adding_block_without_closing_brace
   end
 
   def test_adding_a_rule
-    @cp.add_rule!('div', 'color: blue;')
+    @cp.add_rule!(selectors: 'div', block: 'color: blue;')
     assert_equal 'color: blue;', @cp.find_by_selector('div').join(' ')
   end
 

test/test_css_parser_media_types.rb

@@ -135,24 +135,24 @@ def test_adding_block_and_limiting_media_types
   end
 
   def test_adding_rule_set_with_media_type
-    @cp.add_rule!('body', 'color: black;', [:handheld, :tty])
-    @cp.add_rule!('body', 'color: blue;', :screen)
+    @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty])
+    @cp.add_rule!(selectors: 'body', block: 'color: blue;', media_types: :screen)
     assert_equal 'color: black;', @cp.find_by_selector('body', :handheld).join(' ')
   end
 
   def test_adding_rule_set_with_media_query
-    @cp.add_rule!('body', 'color: black;', 'aural and (device-aspect-ratio: 16/9)')
+    @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)')
     assert_equal 'color: black;', @cp.find_by_selector('body', 'aural and (device-aspect-ratio: 16/9)').join(' ')
     assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ')
   end
 
   def test_selecting_with_all_media_types
-    @cp.add_rule!('body', 'color: black;', [:handheld, :tty])
+    @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty])
     assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ')
   end
 
   def test_to_s_includes_media_queries
-    @cp.add_rule!('body', 'color: black;', 'aural and (device-aspect-ratio: 16/9)')
+    @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)')
     assert_equal "@media aural and (device-aspect-ratio: 16/9) {\n  body {\n    color: black;\n  }\n}\n", @cp.to_s
   end
 end

test/test_css_parser_misc.rb

@@ -229,25 +229,45 @@ def test_enumerator_nonempty
 
   def test_catching_argument_exceptions_for_add_rule
     cp_with_exceptions = Parser.new(rule_set_exceptions: true)
-
     assert_raises ArgumentError, 'background-color value is empty' do
-      cp_with_exceptions.add_rule!('body', 'background-color: !important')
+      cp_with_exceptions.add_rule!(selectors: 'body', block: 'background-color: !important')
     end
 
     cp_without_exceptions = Parser.new(rule_set_exceptions: false)
+    cp_without_exceptions.add_rule!(selectors: 'body', block: 'background-color: !important')
+  end
+
+  def test_catching_argument_exceptions_for_add_rule_positional
+    cp_with_exceptions = Parser.new(rule_set_exceptions: true)
 
-    cp_without_exceptions.add_rule!('body', 'background-color: !important')
+    assert_raises ArgumentError, 'background-color value is empty' do
+      _, err = capture_io do
+        cp_with_exceptions.add_rule!('body', 'background-color: !important')
+      end
+      assert_includes err, "DEPRECATION"
+    end
+
+    cp_without_exceptions = Parser.new(rule_set_exceptions: false)
+    _, err = capture_io do
+      cp_without_exceptions.add_rule!('body', 'background-color: !important')
+    end
+    assert_includes err, "DEPRECATION"
   end
 
   def test_catching_argument_exceptions_for_add_rule_with_offsets
     cp_with_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: true)
 
     assert_raises ArgumentError, 'background-color value is empty' do
-      cp_with_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1)
+      _, err = capture_io do
+        cp_with_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1)
+      end
+      assert_includes err, "DEPRECATION"
     end
 
     cp_without_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: false)
-
-    cp_without_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1)
+    _, err = capture_io do
+      cp_without_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1)
+    end
+    assert_includes err, "DEPRECATION"
   end
 end

test/test_rule_set_creating_shorthand.rb

@@ -111,7 +111,7 @@ def test_combining_dimensions_into_shorthand
 
   # Dimensions shorthand, auto property
   def test_combining_dimensions_into_shorthand_with_auto
-    rs = RuleSet.new('#page', "margin: 0; margin-left: auto; margin-right: auto;")
+    rs = RuleSet.new(selectors: '#page', block: "margin: 0; margin-left: auto; margin-right: auto;")
     rs.expand_shorthand!
     assert_equal('auto;', rs['margin-left'])
     rs.create_shorthand!
@@ -201,7 +201,11 @@ def test_combining_list_style_into_shorthand
   end
 
   def test_property_values_in_url
-    rs = RuleSet.new('#header', "background:url(http://example.com/1528/www/top-logo.jpg) no-repeat top right; padding: 79px 0 10px 0;  text-align:left;")
+    rs = RuleSet.new(
+      selectors: '#header',
+      block: "background:url(http://example.com/1528/www/top-logo.jpg) no-repeat top right; " \
+             "padding: 79px 0 10px 0;  text-align:left;"
+    )
     rs.expand_shorthand!
     assert_equal('top right;', rs['background-position'])
     rs.create_shorthand!