Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix perf copps #173

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions lib/css_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
22 changes: 12 additions & 10 deletions lib/css_parser/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 9 additions & 7 deletions lib/css_parser/rule_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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 << ' '
Expand Down Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion test/test_css_parser_loading.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading