From 1413f9be2cef8e0cc77113b94d1d0e4a7afc8ed6 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Tue, 2 May 2023 02:02:16 -0700 Subject: Treat escape_formulas similar to type, style, formula_value This avoid parse_options doing anything which can be expensive if it happens for each cell. --- lib/axlsx/workbook/worksheet/cell.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 929a7c28..74ca7360 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -45,9 +45,10 @@ module Axlsx self.style = val unless val.nil? || val == 0 val = options.delete(:formula_value) self.formula_value = val unless val.nil? + val = options.delete(:escape_formulas) + self.escape_formulas = val.nil? ? row.worksheet.escape_formulas : val parse_options(options) - self.escape_formulas = row.worksheet.escape_formulas if escape_formulas.nil? self.value = value value.cell = self if contains_rich_text? -- cgit v1.2.3 From d18d7f9696211a0ed899153ce4d8ae9cb9c15b6d Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Fri, 12 May 2023 11:08:06 -0700 Subject: Fix safe rubocop offenses in Axslx::Cell There are 2 offenses left but they would be breaking backwards compatibility. --- .rubocop_todo.yml | 13 ------------- lib/axlsx/workbook/worksheet/cell.rb | 20 +++++++++----------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..2739661d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -58,7 +58,6 @@ Lint/DisjunctiveAssignmentInConstructor: # Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. Lint/DuplicateBranch: Exclude: - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/merged_cells.rb' Lint/NonLocalExitFromIterator: @@ -133,7 +132,6 @@ Performance/RegexpMatch: Exclude: - 'lib/axlsx/stylesheet/color.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' # This cop supports safe autocorrection (--autocorrect). Performance/StringIdentifierArgument: @@ -323,7 +321,6 @@ Style/MultipleComparison: Exclude: - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/font.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' - 'test/workbook/tc_workbook_view.rb' @@ -383,7 +380,6 @@ Style/NumericPredicate: - 'lib/axlsx/stylesheet/font.rb' - 'lib/axlsx/util/validators.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/sheet_pr.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' @@ -425,7 +421,6 @@ Style/ParenthesesAroundCondition: Exclude: - 'lib/axlsx/stylesheet/font.rb' - 'lib/axlsx/util/validators.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' # This cop supports safe autocorrection (--autocorrect). @@ -545,13 +540,6 @@ Style/RedundantReturn: Style/RedundantSelf: Enabled: false -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle, AllowInnerSlashes. -# SupportedStyles: slashes, percent_r, mixed -Style/RegexpLiteral: - Exclude: - - 'lib/axlsx/workbook/worksheet/cell.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle. # SupportedStyles: implicit, explicit @@ -620,7 +608,6 @@ Style/SymbolProc: - 'lib/axlsx/drawing/drawing.rb' - 'lib/axlsx/stylesheet/styles.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' - 'lib/axlsx/workbook/worksheet/worksheet_hyperlinks.rb' diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 74ca7360..37b8e205 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -42,7 +42,7 @@ module Axlsx self.type = type unless type == :string val = options.delete(:style) - self.style = val unless val.nil? || val == 0 + self.style = val unless val.nil? || val.zero? val = options.delete(:formula_value) self.formula_value = val unless val.nil? val = options.delete(:escape_formulas) @@ -291,7 +291,7 @@ module Axlsx # @see u def u=(v) - v = :single if (v == true || v == 1 || v == :true || v == 'true') + v = :single if [true, 1, :true, 'true'].include?(v) set_run_style :validate_cell_u, :u, v end @@ -354,7 +354,7 @@ module Axlsx # @example Absolute Cell Reference # ws.rows.first.cells.first.r #=> "$A$1" def r_abs - "$#{r.match(%r{([A-Z]+)([0-9]+)})[1, 2].join('$')}" + "$#{r.match(/([A-Z]+)([0-9]+)/)[1, 2].join('$')}" end # @return [Integer] The cellXfs item index applied to this cell. @@ -379,7 +379,7 @@ module Axlsx start, stop = if target.is_a?(String) [self.r, target] elsif target.is_a?(Cell) - Axlsx.sort_cells([self, target]).map { |c| c.r } + Axlsx.sort_cells([self, target]).map(&:r) end self.row.worksheet.merge_cells "#{start}:#{stop}" unless stop.nil? end @@ -509,13 +509,11 @@ module Axlsx :time elsif v.is_a?(TrueClass) || v.is_a?(FalseClass) :boolean - elsif v.to_s =~ Axlsx::NUMERIC_REGEX && v.respond_to?(:to_i) + elsif v.respond_to?(:to_i) && v.to_s =~ Axlsx::NUMERIC_REGEX :integer - elsif v.to_s =~ Axlsx::SAFE_FLOAT_REGEX && v.respond_to?(:to_f) + elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i))) :float - elsif (matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i) && v.respond_to?(:to_f) - :float - elsif v.to_s =~ Axlsx::ISO_8601_REGEX + elsif Axlsx::ISO_8601_REGEX.match?(v.to_s) :iso_8601 elsif v.is_a? RichText :richtext @@ -533,14 +531,14 @@ module Axlsx case type when :date - self.style = STYLE_DATE if self.style == 0 + self.style = STYLE_DATE if self.style.zero? if !v.is_a?(Date) && v.respond_to?(:to_date) v.to_date else v end when :time - self.style = STYLE_DATE if self.style == 0 + self.style = STYLE_DATE if self.style.zero? if !v.is_a?(Time) && v.respond_to?(:to_time) v.to_time else -- cgit v1.2.3 From e57d67b31d0edbdf3ffe1f2e16f1c262e1b48c4b Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 09:55:49 -0700 Subject: Using `between?` is more efficient than `cover?` Co-authored-by: Geremia Taglialatela --- lib/axlsx/workbook/worksheet/cell.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 37b8e205..10818f5c 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -511,7 +511,7 @@ module Axlsx :boolean elsif v.respond_to?(:to_i) && v.to_s =~ Axlsx::NUMERIC_REGEX :integer - elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i))) + elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && matchdata[:exp].to_i.between?(Float::MIN_10_EXP, Float::MAX_10_EXP))) :float elsif Axlsx::ISO_8601_REGEX.match?(v.to_s) :iso_8601 -- cgit v1.2.3 From af8360755fe21f4e9d30e943ba0b2be3c0128d28 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 10:18:50 -0700 Subject: Improve boolean validation constants (i.e. Axlsx::VALID_BOOLEAN_VALUES) Added VALID_BOOLEAN_TRUE_VALUES and VALID_BOOLEAN_FALSE_VALUES so that those can be re-used in other placed and have the same notion of what a valid boolean value is. For example, we can use the true values in `Cell#u=`. Additionally, since validate_boolean / BOOLEAN_VALIDATOR are invoked so frequently, putting the likely values at the front can actually make a non-trivial difference. Since VALID_BOOLEAN_VALUES is derived from VALID_BOOLEAN_TRUE_VALUES and VALID_BOOLEAN_FALSE_VALUES, we use `Array#zip` to still end up with good order. --- lib/axlsx/util/validators.rb | 6 ++++-- lib/axlsx/workbook/worksheet/cell.rb | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/axlsx/util/validators.rb b/lib/axlsx/util/validators.rb index 0004f51a..752ffc76 100644 --- a/lib/axlsx/util/validators.rb +++ b/lib/axlsx/util/validators.rb @@ -106,8 +106,10 @@ module Axlsx DataTypeValidator.validate :signed_int, Integer, v end - VALID_BOOLEAN_CLASSES = [String, Integer, Symbol, TrueClass, FalseClass].freeze - VALID_BOOLEAN_VALUES = [0, 1, 'true', 'false', :true, :false, true, false, '0', '1'].freeze + VALID_BOOLEAN_CLASSES = [TrueClass, FalseClass, Integer, String, Symbol].freeze + VALID_BOOLEAN_TRUE_VALUES = [true, 1, '1', 'true', :true].freeze + VALID_BOOLEAN_FALSE_VALUES = [false, 0, '0', 'false', :false].freeze + VALID_BOOLEAN_VALUES = VALID_BOOLEAN_TRUE_VALUES.zip(VALID_BOOLEAN_FALSE_VALUES).flatten.freeze BOOLEAN_VALIDATOR = lambda { |arg| VALID_BOOLEAN_VALUES.include?(arg) } # Requires that the value is a form that can be evaluated as a boolean in an xml document. diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 10818f5c..8afacee1 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -291,7 +291,7 @@ module Axlsx # @see u def u=(v) - v = :single if [true, 1, :true, 'true'].include?(v) + v = :single if VALID_BOOLEAN_TRUE_VALUES.include?(v) set_run_style :validate_cell_u, :u, v end -- cgit v1.2.3 From 447b2522bc4a06c8436e708eae40cf7244dc0ec4 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 11:00:38 -0700 Subject: Only define @escape_formulas in cell if it is different from worksheet The benchmarks showed that validate_boolean is called 200_005 times and almost all of those are to validate escape_formulas passed into cell. In this commit the worksheet does not pass in its escape_formulas value, avoiding validate_boolean, and instead the cell asks the worksheet for value when needed. Now validate_boolean is called 5 times in benchmarks. --- lib/axlsx/workbook/worksheet/cell.rb | 6 ++++-- lib/axlsx/workbook/worksheet/worksheet.rb | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 8afacee1..b44656e3 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -46,7 +46,7 @@ module Axlsx val = options.delete(:formula_value) self.formula_value = val unless val.nil? val = options.delete(:escape_formulas) - self.escape_formulas = val.nil? ? row.worksheet.escape_formulas : val + self.escape_formulas = val unless val.nil? parse_options(options) @@ -146,7 +146,9 @@ module Axlsx # Allowing user-generated data to be interpreted as formulas is a security risk. # See https://www.owasp.org/index.php/CSV_Injection for details. # @return [Boolean] - attr_reader :escape_formulas + def escape_formulas + defined?(@escape_formulas) ? @escape_formulas : row.worksheet.escape_formulas + end # Sets whether to treat values starting with an equals sign as formulas or as literal strings. # @param [Boolean] value The value to set. diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb index 960f33b3..21c53409 100644 --- a/lib/axlsx/workbook/worksheet/worksheet.rb +++ b/lib/axlsx/workbook/worksheet/worksheet.rb @@ -429,7 +429,6 @@ module Axlsx # Allowing user generated data to be interpreted as formulas can be dangerous # (see https://www.owasp.org/index.php/CSV_Injection for details). def add_row(values = [], options = {}) - options[:escape_formulas] = escape_formulas if options[:escape_formulas].nil? row = Row.new(self, values, options) update_column_info row, options.delete(:widths) yield row if block_given? -- cgit v1.2.3 From 1201ef7145463a14d263a093d7d53a2f235e48e0 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Mon, 15 May 2023 17:04:59 -0700 Subject: Remove ability to set `u=` to true in favor of :single The `u=` would convert `true` to `:single` for backwards compatibility. However, it is more explicit to set it to `:single` or one of the other underline options instead of relying on the conversion. --- CHANGELOG.md | 1 + lib/axlsx/util/validators.rb | 4 +--- lib/axlsx/workbook/worksheet/cell.rb | 4 +--- test/workbook/worksheet/tc_cell.rb | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19f5d742..92d3396f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG - Drop support for Ruby versions < 2.6 - Added frozen string literals - Fix `SimpleTypedList#to_a` and `SimpleTypedList#to_ary` returning the internal list instance + - Remove ability to set `u=` to true in favor of using :single or one of the other underline options - **April.23.23**: 3.4.1 - [PR #209](https://github.com/caxlsx/caxlsx/pull/209) - Revert characters other than `=` being considered as formulas. diff --git a/lib/axlsx/util/validators.rb b/lib/axlsx/util/validators.rb index 752ffc76..e9cf13b6 100644 --- a/lib/axlsx/util/validators.rb +++ b/lib/axlsx/util/validators.rb @@ -107,9 +107,7 @@ module Axlsx end VALID_BOOLEAN_CLASSES = [TrueClass, FalseClass, Integer, String, Symbol].freeze - VALID_BOOLEAN_TRUE_VALUES = [true, 1, '1', 'true', :true].freeze - VALID_BOOLEAN_FALSE_VALUES = [false, 0, '0', 'false', :false].freeze - VALID_BOOLEAN_VALUES = VALID_BOOLEAN_TRUE_VALUES.zip(VALID_BOOLEAN_FALSE_VALUES).flatten.freeze + VALID_BOOLEAN_VALUES = [true, false, 1, 0, '1', '0', 'true', 'false', :true, :false].freeze BOOLEAN_VALIDATOR = lambda { |arg| VALID_BOOLEAN_VALUES.include?(arg) } # Requires that the value is a form that can be evaluated as a boolean in an xml document. diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index b44656e3..cbf55482 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -285,15 +285,13 @@ module Axlsx def extend=(v) set_run_style :validate_boolean, :extend, v; end # The inline underline property for the cell. - # It must be one of :none, :single, :double, :singleAccounting, :doubleAccounting, true + # It must be one of :none, :single, :double, :singleAccounting, :doubleAccounting # @return [Boolean] # @return [String] - # @note true is for backwards compatability and is reassigned to :single attr_reader :u # @see u def u=(v) - v = :single if VALID_BOOLEAN_TRUE_VALUES.include?(v) set_run_style :validate_cell_u, :u, v end diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb index a534a8dc..fef23ede 100644 --- a/test/workbook/worksheet/tc_cell.rb +++ b/test/workbook/worksheet/tc_cell.rb @@ -567,7 +567,7 @@ class TestCell < Test::Unit::TestCase def test_to_xml # TODO: This could use some much more stringent testing related to the xml content generated! @ws.add_row [Time.now, Date.today, true, 1, 1.0, "text", "=sum(A1:A2)", "2013-01-13T13:31:25.123"] - @ws.rows.last.cells[5].u = true + @ws.rows.last.cells[5].u = :single schema = Nokogiri::XML::Schema(File.open(Axlsx::SML_XSD)) doc = Nokogiri::XML(@ws.to_xml_string) -- cgit v1.2.3