Merge pull request #173 from premailer/grosser/perf

fix perf copps

.rubocop.yml

@@ -42,13 +42,6 @@ Metrics/ModuleLength:
 Metrics/PerceivedComplexity:
   Enabled: false
 
-# TODO: Progressively fix performance offences and remove this setting
-Performance:
-  Enabled: false
-
-Performance/RegexpMatch:
-  Enabled: true
-
 Style/AndOr:
   Enabled: false
 

lib/css_parser.rb

@@ -57,7 +57,7 @@ def self.merge(*rule_sets)
     # in case called like CssParser.merge([rule_set, rule_set])
     rule_sets.flatten! if rule_sets[0].is_a?(Array)
 
-    unless rule_sets.all? { |rs| rs.is_a?(CssParser::RuleSet) }
+    unless rule_sets.all?(CssParser::RuleSet)
       raise ArgumentError, 'all parameters must be CssParser::RuleSets.'
     end
 
@@ -70,7 +70,7 @@ def self.merge(*rule_sets)
       rule_set.expand_shorthand!
 
       specificity = rule_set.specificity
-      specificity ||= rule_set.selectors.map { |s| calculate_specificity(s) }.compact.max || 0
+      specificity ||= rule_set.selectors.filter_map { |s| calculate_specificity(s) }.max || 0
 
       rule_set.each_declaration do |property, value, is_important|
         # Add the property to the list to be folded per http://www.w3.org/TR/CSS21/cascade.html#cascading-order

lib/css_parser/parser.rb

@@ -359,9 +359,9 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
       in_at_media_rule = false
       in_media_block = false
 
-      current_selectors = String.new
-      current_media_query = String.new
-      current_declarations = String.new
+      current_selectors = +''
+      current_media_query = +''
+      current_declarations = +''
 
       # once we are in a rule, we will use this to store where we started if we are capturing offsets
       rule_start = nil
@@ -405,13 +405,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
                 media_types: current_media_queries
               }
               if options[:capture_offsets]
-                add_rule_options.merge!(filename: options[:filename], offset: rule_start..end_offset)
+                add_rule_options[:filename] = options[:filename]
+                add_rule_options[:offset] = rule_start..end_offset
               end
               add_rule!(**add_rule_options)
             end
 
-            current_selectors = String.new
-            current_declarations = String.new
+            current_selectors = +''
+            current_declarations = +''
 
             # restart our search for selectors and declarations
             rule_start = nil if options[:capture_offsets]
@@ -426,14 +427,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
             in_at_media_rule = false
             in_media_block = true
             current_media_queries << CssParser.sanitize_media_query(current_media_query)
-            current_media_query = String.new
+            current_media_query = +''
           elsif token.include?(',')
             # new media query begins
             token.tr!(',', ' ')
             token.strip!
             current_media_query << token << ' '
             current_media_queries << CssParser.sanitize_media_query(current_media_query)
-            current_media_query = String.new
+            current_media_query = +''
           else
             token.strip!
             # special-case the ( and ) tokens to remove inner-whitespace
@@ -478,7 +479,8 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
         media_types: current_media_queries
       }
       if options[:capture_offsets]
-        add_rule_options.merge!(filename: options[:filename], offset: rule_start..end_offset)
+        add_rule_options[:filename] = options[:filename]
+        add_rule_options[:offset] = rule_start..end_offset
       end
       add_rule!(**add_rule_options)
     end
@@ -718,7 +720,7 @@ def css_node_to_h(hash, key, val)
       nodes = {}
       lines.each do |line|
         parts = line.split(':', 2)
-        if /:/.match?(parts[1])
+        if parts[1].include?(':')
           nodes[parts[0]] = css_node_to_h(hash, parts[0], parts[1])
         else
           nodes[parts[0].to_s.strip] = parts[1].to_s.strip

lib/css_parser/rule_set.rb

@@ -11,8 +11,10 @@ class RuleSet
     BACKGROUND_PROPERTIES = ['background-color', 'background-image', 'background-repeat', 'background-position', 'background-size', 'background-attachment'].freeze
     LIST_STYLE_PROPERTIES = ['list-style-type', 'list-style-position', 'list-style-image'].freeze
     FONT_STYLE_PROPERTIES = ['font-style', 'font-variant', 'font-weight', 'font-size', 'line-height', 'font-family'].freeze
+    FONT_WEIGHT_PROPERTIES = ['font-style', 'font-weight', 'font-variant'].freeze
     BORDER_STYLE_PROPERTIES = ['border-width', 'border-style', 'border-color'].freeze
     BORDER_PROPERTIES = ['border', 'border-left', 'border-right', 'border-top', 'border-bottom'].freeze
+    DIMENSION_DIRECTIONS = [:top, :right, :bottom, :left].freeze
 
     NUMBER_OF_DIMENSIONS = 4
 
@@ -196,7 +198,7 @@ def replace_declaration!(property, replacements, preserve_importance: false)
       end
 
       def to_s(options = {})
-        str = declarations.reduce(String.new) do |memo, (prop, value)|
+        str = declarations.reduce(+'') do |memo, (prop, value)|
           importance = options[:force_important] || value.important ? ' !important' : ''
           memo << "#{prop}: #{value.value}#{importance}; "
         end
@@ -456,7 +458,7 @@ def expand_font_shorthand! # :nodoc:
             font_props['font-family'] = m
           end
         elsif /normal|inherit/i.match?(m)
-          ['font-style', 'font-weight', 'font-variant'].each do |font_prop|
+          FONT_WEIGHT_PROPERTIES.each do |font_prop|
             font_props[font_prop] ||= m
           end
         elsif /italic|oblique/i.match?(m)
@@ -554,15 +556,15 @@ def create_background_shorthand! # :nodoc:
     #
     # TODO: this is extremely similar to create_background_shorthand! and should be combined
     def create_border_shorthand! # :nodoc:
-      values = BORDER_STYLE_PROPERTIES.map do |property|
+      values = BORDER_STYLE_PROPERTIES.filter_map do |property|
         next unless (declaration = declarations[property])
         next if declaration.important
         # can't merge if any value contains a space (i.e. has multiple values)
         # we temporarily remove any spaces after commas for the check (inside rgba, etc...)
         next if /\s/.match?(declaration.value.gsub(/,\s/, ',').strip)
 
         declaration.value
-      end.compact
+      end
 
       return if values.size != BORDER_STYLE_PROPERTIES.size
 
@@ -579,7 +581,7 @@ def create_dimensions_shorthand! # :nodoc:
       return if declarations.size < NUMBER_OF_DIMENSIONS
 
       DIMENSIONS.each do |property, dimensions|
-        values = [:top, :right, :bottom, :left].each_with_index.with_object({}) do |(side, index), result|
+        values = DIMENSION_DIRECTIONS.each_with_index.with_object({}) do |(side, index), result|
           next unless (declaration = declarations[dimensions[index]])
 
           result[side] = declaration.value
@@ -602,7 +604,7 @@ def create_dimensions_shorthand! # :nodoc:
     def create_font_shorthand! # :nodoc:
       return unless FONT_STYLE_PROPERTIES.all? { |prop| declarations.key?(prop) }
 
-      new_value = String.new
+      new_value = +''
       ['font-style', 'font-variant', 'font-weight'].each do |property|
         unless declarations[property].value == 'normal'
           new_value << declarations[property].value << ' '
@@ -639,7 +641,7 @@ def compute_dimensions_shorthand(values)
       return [:top] if values.values.uniq.count == 1
 
       # `/* top | right | bottom | left */`
-      return [:top, :right, :bottom, :left] if values[:left] != values[:right]
+      return DIMENSION_DIRECTIONS if values[:left] != values[:right]
 
       # Vertical are the same & horizontal are the same, `/* vertical | horizontal */`
       return [:top, :left] if values[:top] == values[:bottom]

test/test_css_parser_loading.rb

@@ -141,7 +141,7 @@ def test_loading_malformed_content_strings
     file_name = File.expand_path('fixtures/import-malformed.css', __dir__)
     @cp.load_file!(file_name)
     @cp.each_selector do |_sel, dec, _spec|
-      assert_nil dec =~ /wellformed/
+      assert_equal false, dec.include?('wellformed')
     end
   end