Refactor expand_font_shorthand

I used https://www.w3.org/TR/CSS21/fonts.html#propdef-font to create the scanner

.rubocop.yml

@@ -41,6 +41,9 @@ Metrics/ModuleLength:
 Metrics/PerceivedComplexity:
   Enabled: false
 
+Style/AccessModifierDeclarations:
+  Enabled: false
+
 Style/AndOr:
   Enabled: false
 

lib/css_parser/regexps.rb

@@ -45,7 +45,6 @@ def self.regex_possible_values(*values)
   RE_SINGLE_BACKGROUND_SIZE = /#{RE_LENGTH_OR_PERCENTAGE}|auto|cover|contain|initial|inherit/i.freeze
   RE_BACKGROUND_POSITION = /#{RE_SINGLE_BACKGROUND_POSITION}\s+#{RE_SINGLE_BACKGROUND_POSITION}|#{RE_SINGLE_BACKGROUND_POSITION}/.freeze
   RE_BACKGROUND_SIZE = %r{\s*/\s*(#{RE_SINGLE_BACKGROUND_SIZE}\s+#{RE_SINGLE_BACKGROUND_SIZE}|#{RE_SINGLE_BACKGROUND_SIZE})}.freeze
-  FONT_UNITS_RX = /((x+-)*small|medium|larger*|auto|inherit|([0-9]+|[0-9]*\.[0-9]+)(e[mx]+|px|[cm]+m|p[tc+]|in|%)*)/i.freeze
   RE_BORDER_STYLE = /(\s*^)?(none|hidden|dotted|dashed|solid|double|dot-dash|dot-dot-dash|wave|groove|ridge|inset|outset)(\s*$)?/imx.freeze
   RE_BORDER_UNITS = Regexp.union(BOX_MODEL_UNITS_RX, /(thin|medium|thick)/i)
 

lib/css_parser/rule_set.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'forwardable'
+require 'set'
 
 module CssParser
   class RuleSet
@@ -203,6 +204,85 @@ def expand_dimensions_shorthand! # :nodoc:
       end
     end
 
+    class FontScanner
+      FONT_STYLES = Set.new(['normal', 'italic', 'oblique', 'inherit'])
+      FONT_VARIANTS = Set.new(['normal', 'small-caps', 'inherit'])
+      FONT_WEIGHTS = Set.new(
+        [
+          'normal', 'bold', 'bolder', 'lighter',
+          '100', '200', '300', '400', '500', '600', '700', '800', '900',
+          'inherit'
+        ]
+      )
+      ABSOLUTE_SIZES = Set.new(
+        ['xx-small', 'x-small', 'small', 'medium', 'large', 'x-large', 'xx-large']
+      )
+      RELATIVE_SIZES = Set.new(['smaller', 'larger'])
+
+      attr_reader :current, :pos, :tokens
+
+      def initialize(tokens)
+        @token_scanner = Crass::TokenScanner.new(tokens)
+      end
+
+      def peek = @token_scanner.peek
+      def consume = @token_scanner.consume
+      def collect(&block) = @token_scanner.collect(&block)
+
+      private def consume_iden_str(value)
+        consume if peek[:node] == :ident && peek[:value] == value
+      end
+
+      private def consume_iden_set(set)
+        consume if peek[:node] == :ident && set.member?(peek[:value])
+      end
+
+      private def consume_type(type)
+        consume if peek[:node] == type
+      end
+
+      def consume_font_style = consume_iden_set(FONT_STYLES)
+      def consume_font_variant = consume_iden_set(FONT_VARIANTS)
+      def consume_font_weight = consume_iden_set(FONT_WEIGHTS) || consume_type(:number)
+      def consume_absulute_size = consume_iden_set(ABSOLUTE_SIZES)
+      def consume_relative_size = consume_iden_set(RELATIVE_SIZES)
+      def consume_length = consume_type(:dimension)
+      def consume_percentage = consume_type(:percentage)
+      def consume_number = consume_type(:percentage)
+      def consume_inherit = consume_iden_str('inherit')
+      def consume_normal = consume_iden_str('normal')
+
+      def consume_font_style_variant_weight
+        consume_font_style || consume_font_variant || consume_font_weight
+      end
+
+      def consume_font_size
+        consume_absulute_size ||
+          consume_relative_size ||
+          consume_length ||
+          consume_percentage ||
+          consume_inherit
+      end
+
+      def consume_line_height
+        consume_normal ||
+          consume_number ||
+          consume_length ||
+          consume_percentage ||
+          consume_inherit
+      end
+
+      def consume_system_fonts
+        consume_iden_str('caption') ||
+          consume_iden_str('icon') ||
+          consume_iden_str('menu') ||
+          consume_iden_str('message-box') ||
+          consume_iden_str('small-caption') ||
+          consume_iden_str('status-bar') ||
+          consume_inherit
+      end
+    end
+
     # Convert shorthand font declarations (e.g. <tt>font: 300 italic 11px/14px verdana, helvetica, sans-serif;</tt>)
     # into their constituent parts.
     def expand_font_shorthand! # :nodoc:
@@ -216,43 +296,47 @@ def expand_font_shorthand! # :nodoc:
         'font-size' => 'normal',
         'line-height' => 'normal'
       }
+      tokens = Crass::Tokenizer
+               .tokenize(declaration.value.dup)
+               .reject { _1[:node] == :whitespace }
+      scanner = FontScanner.new(tokens)
+
+      if scanner.consume_system_fonts
+        # nothing we can do with system fonts
+        return
+      end
 
-      value = declaration.value.dup
-      value.gsub!(%r{/\s+}, '/') # handle spaces between font size and height shorthand (e.g. 14px/ 16px)
-
-      in_fonts = false
-
-      matches = value.scan(/"(?:.*[^"])"|'(?:.*[^'])'|(?:\w[^ ,]+)/)
-      matches.each do |m|
-        m.strip!
-        m.gsub!(/;$/, '')
-
-        if in_fonts
-          if font_props.key?('font-family')
-            font_props['font-family'] += ", #{m}"
-          else
-            font_props['font-family'] = m
-          end
-        elsif m =~ /normal|inherit/i
-          ['font-style', 'font-weight', 'font-variant'].each do |font_prop|
-            font_props[font_prop] ||= m
-          end
-        elsif m =~ /italic|oblique/i
-          font_props['font-style'] = m
-        elsif m =~ /small-caps/i
-          font_props['font-variant'] = m
-        elsif m =~ /[1-9]00$|bold|bolder|lighter/i
-          font_props['font-weight'] = m
-        elsif m =~ CssParser::FONT_UNITS_RX
-          if m.include?('/')
-            font_props['font-size'], font_props['line-height'] = m.split('/', 2)
-          else
-            font_props['font-size'] = m
-          end
-          in_fonts = true
+      while (token = scanner.consume_font_style_variant_weight)
+        if FontScanner::FONT_STYLES.member?(token[:value])
+          font_props['font-style'] = token[:value]
+        end
+        if FontScanner::FONT_VARIANTS.member?(token[:value])
+          font_props['font-variant'] = token[:value]
+        end
+        # we use raw from font wights since it include numbers
+        if FontScanner::FONT_WEIGHTS.member?(token[:raw])
+          font_props['font-weight'] = token[:raw]
         end
       end
 
+      font_size = scanner.consume_font_size
+      font_props['font-size'] = font_size[:raw]
+
+      if scanner.peek[:node] == :delim && scanner.peek[:value] == '/'
+        scanner.consume
+        line_height = scanner.consume_line_height
+        font_props['line-height'] = line_height[:raw]
+      end
+
+      rest = scanner.collect do
+        while scanner.consume
+          # nothing, just collect the rest
+        end
+      end
+      if rest.any?
+        font_props['font-family'] = Crass::Parser.stringify(rest)
+      end
+
       declarations.replace_declaration!('font', font_props, preserve_importance: true)
     end
 

test/test_rule_set_expanding_shorthand.rb

@@ -71,20 +71,26 @@ def test_getting_font_size_from_shorthand
     ['em', 'ex', 'in', 'px', 'pt', 'pc', '%'].each do |unit|
       shorthand = "font: 300 italic 11.25#{unit}/14px verdana, helvetica, sans-serif;"
       declarations = expand_declarations(shorthand)
-      assert_equal("11.25#{unit}", declarations['font-size'])
+      assert_equal("11.25#{unit}", declarations['font-size'], shorthand)
     end
 
     ['smaller', 'small', 'medium', 'large', 'x-large'].each do |unit|
       shorthand = "font: 300 italic #{unit}/14px verdana, helvetica, sans-serif;"
       declarations = expand_declarations(shorthand)
-      assert_equal(unit, declarations['font-size'])
+      assert_equal(unit, declarations['font-size'], shorthand)
     end
   end
 
+  def test_font_with_comments_and_spaces
+    shorthand = "font: 300 /* HI */   italic  \t\t   12px  sans-serif;"
+    declarations = expand_declarations(shorthand)
+    assert_equal("12px", declarations['font-size'])
+  end
+
   def test_getting_font_families_from_shorthand
     shorthand = "font: 300 italic 12px/14px \"Helvetica-Neue-Light 45\", 'verdana', helvetica, sans-serif;"
     declarations = expand_declarations(shorthand)
-    assert_equal("\"Helvetica-Neue-Light 45\", 'verdana', helvetica, sans-serif", declarations['font-family'])
+    assert_equal("\"Helvetica-Neue-Light 45\",'verdana',helvetica,sans-serif", declarations['font-family'])
   end
 
   def test_getting_font_weight_from_shorthand
@@ -95,8 +101,10 @@ def test_getting_font_weight_from_shorthand
     end
 
     # ensure normal is the default state
-    ['font: normal italic 12px sans-serif;', 'font: italic 12px sans-serif;',
-     'font: small-caps normal 12px sans-serif;', 'font: 12px/16px sans-serif;'].each do |shorthand|
+    ['font: normal italic 12px sans-serif;',
+     'font: italic 12px sans-serif;',
+     'font: small-caps normal 12px sans-serif;',
+     'font: 12px/16px sans-serif;'].each do |shorthand|
       declarations = expand_declarations(shorthand)
       assert_equal('normal', declarations['font-weight'], shorthand)
     end
@@ -110,8 +118,11 @@ def test_getting_font_variant_from_shorthand
 
   def test_getting_font_variant_from_shorthand_ensure_normal_is_the_default_state
     [
-      'font: normal italic 12px sans-serif;', 'font: italic 12px sans-serif;',
-      'font: normal 12px sans-serif;', 'font: 12px/16px sans-serif;'
+      'font: normal large sans-serif;',
+      'font: normal italic 12px sans-serif;',
+      'font: italic 12px sans-serif;',
+      'font: normal 12px sans-serif;',
+      'font: 12px/16px sans-serif;'
     ].each do |shorthand|
       declarations = expand_declarations(shorthand)
       assert_equal('normal', declarations['font-variant'], shorthand)
@@ -126,8 +137,10 @@ def test_getting_font_style_from_shorthand
     end
 
     # ensure normal is the default state
-    ['font: normal bold 12px sans-serif;', 'font: small-caps 12px sans-serif;',
-     'font: normal 12px sans-serif;', 'font: 12px/16px sans-serif;'].each do |shorthand|
+    ['font: normal bold 12px sans-serif;',
+     'font: small-caps 12px sans-serif;',
+     'font: normal 12px sans-serif;',
+     'font: 12px/16px sans-serif;'].each do |shorthand|
       declarations = expand_declarations(shorthand)
       assert_equal('normal', declarations['font-style'], shorthand)
     end
@@ -141,8 +154,10 @@ def test_getting_line_height_from_shorthand
     end
 
     # ensure normal is the default state
-    ['font: normal bold 12px sans-serif;', 'font: small-caps 12px sans-serif;',
-     'font: normal 12px sans-serif;', 'font: 12px sans-serif;'].each do |shorthand|
+    ['font: normal bold 12px sans-serif;',
+     'font: small-caps 12px sans-serif;',
+     'font: normal 12px sans-serif;',
+     'font: 12px sans-serif;'].each do |shorthand|
       declarations = expand_declarations(shorthand)
       assert_equal('normal', declarations['line-height'], shorthand)
     end
@@ -156,6 +171,15 @@ def test_getting_line_height_from_shorthand_with_spaces
     end
   end
 
+  def test_expands_nothing_using_system_fonts
+    %w[caption icon menu message-box small-caption status-bar].each do |system_font|
+      shorthand = "font: #{system_font}"
+      declarations = expand_declarations(shorthand)
+      assert_equal(["font"], declarations.keys)
+      assert_equal(system_font, declarations['font'])
+    end
+  end
+
   # Background shorthand
   def test_getting_background_properties_from_shorthand
     expected = {