From e85801445f278a81d1e3a2901c4093e8f9d3e38a Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Mon, 15 May 2023 21:51:40 +0200 Subject: Fix Performance/StringIdentifierArgument offense --- .rubocop_todo.yml | 5 ----- lib/axlsx/drawing/pic.rb | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..57bf7265 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -135,11 +135,6 @@ Performance/RegexpMatch: - 'lib/axlsx/workbook/workbook.rb' - 'lib/axlsx/workbook/worksheet/cell.rb' -# This cop supports safe autocorrection (--autocorrect). -Performance/StringIdentifierArgument: - Exclude: - - 'lib/axlsx/drawing/pic.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle. # SupportedStyles: separated, grouped diff --git a/lib/axlsx/drawing/pic.rb b/lib/axlsx/drawing/pic.rb index 77a051c3..fa08bd83 100644 --- a/lib/axlsx/drawing/pic.rb +++ b/lib/axlsx/drawing/pic.rb @@ -239,7 +239,7 @@ module Axlsx def swap_anchor(new_anchor) new_anchor.drawing.anchors.delete(new_anchor) @anchor.drawing.anchors[@anchor.drawing.anchors.index(@anchor)] = new_anchor - new_anchor.instance_variable_set "@object", @anchor.object + new_anchor.instance_variable_set :@object, @anchor.object @anchor = new_anchor end end -- cgit v1.2.3 From a0bef85fc877afe91e22558bac5e14e2f7f88dbe Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Mon, 15 May 2023 22:12:05 +0200 Subject: Use `include?` and `find` for performance Fix a couple of performance RuboCop offenses in workbook --- .rubocop_todo.yml | 7 ------- lib/axlsx/workbook/workbook.rb | 4 ++-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..846974f6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -106,11 +106,6 @@ Performance/CollectionLiteralInLoop: - 'lib/axlsx/package.rb' - 'lib/axlsx/workbook/worksheet/page_margins.rb' -# This cop supports unsafe autocorrection (--autocorrect-all). -Performance/Detect: - Exclude: - - 'lib/axlsx/workbook/workbook.rb' - # This cop supports safe autocorrection (--autocorrect). Performance/RedundantBlockCall: Exclude: @@ -121,7 +116,6 @@ Performance/RedundantMatch: Exclude: - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/color.rb' - - 'lib/axlsx/workbook/workbook.rb' # This cop supports safe autocorrection (--autocorrect). Performance/RedundantSplitRegexpArgument: @@ -132,7 +126,6 @@ Performance/RedundantSplitRegexpArgument: 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). diff --git a/lib/axlsx/workbook/workbook.rb b/lib/axlsx/workbook/workbook.rb index 3e4927aa..1a2f0488 100644 --- a/lib/axlsx/workbook/workbook.rb +++ b/lib/axlsx/workbook/workbook.rb @@ -395,8 +395,8 @@ module Axlsx # retrieve the cells from. e.g. range('Sheet1!A1:B2') will return an array of four cells [A1, A2, B1, B2] while range('Sheet1!A1') will return a single Cell. # @return [Cell, Array] def [](cell_def) - sheet_name = cell_def.split('!')[0] if cell_def.match('!') - worksheet = self.worksheets.select { |s| s.name == sheet_name }.first + sheet_name = cell_def.split('!')[0] if cell_def.include?('!') + worksheet = self.worksheets.find { |s| s.name == sheet_name } raise ArgumentError, 'Unknown Sheet' unless sheet_name && worksheet.is_a?(Worksheet) worksheet[cell_def.gsub(/.+!/, "")] -- cgit v1.2.3 From 4627bcce04ade9c17e1d0c169100a6288195f6ac Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 14:21:21 -0700 Subject: Cache col_ref to avoid allocations In cases with lots of rows, each column will ask for its col_ref which will always be the same for the same column_index. We can cache this to avoid lots of small string allocations. Modified `CellSerializer` to use `#col_ref` and `#row_ref` avoiding the string allocation caused by `#col_r` --- lib/axlsx.rb | 30 ++++++++++++++++++------- lib/axlsx/workbook/worksheet/cell_serializer.rb | 4 +++- test/tc_axlsx.rb | 8 ++++++- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 4f15ba23..6e6a317d 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -112,21 +112,35 @@ module Axlsx # @note This follows the standard spreadsheet convention of naming columns A to Z, followed by AA to AZ etc. # @return [String] def self.col_ref(index) - chars = +'' - while index >= 26 do - index, char = index.divmod(26) - chars.prepend((char + 65).chr) - index -= 1 + # Every row will call this for each column / cell and so we can cache result and avoid lots of small object + # allocations. + @col_ref ||= {} + @col_ref[index] ||= begin + i = index + chars = +'' + while i >= 26 + i, char = i.divmod(26) + chars.prepend((char + 65).chr) + i -= 1 + end + chars.prepend((i + 65).chr) + chars.freeze + chars end - chars.prepend((index + 65).chr) - chars + end + + # converts the row index into string values. + # @note The spreadsheet rows are 1-based and the passed in index is 0-based, so we add 1. + # @return [String] + def self.row_ref(index) + (index + 1).to_s end # @return [String] The alpha(column)numeric(row) reference for this sell. # @example Relative Cell Reference # ws.rows.first.cells.first.r #=> "A1" def self.cell_r(c_index, r_index) - col_ref(c_index) << (r_index + 1).to_s + col_ref(c_index) + row_ref(r_index) end # Creates an array of individual cell references based on an excel reference range. diff --git a/lib/axlsx/workbook/worksheet/cell_serializer.rb b/lib/axlsx/workbook/worksheet/cell_serializer.rb index e1bdf728..2baa4271 100644 --- a/lib/axlsx/workbook/worksheet/cell_serializer.rb +++ b/lib/axlsx/workbook/worksheet/cell_serializer.rb @@ -10,7 +10,9 @@ module Axlsx # @param [String] str The string to apend serialization to. # @return [String] def to_xml_string(row_index, column_index, cell, str = +'') - str << '' if cell.value.nil? method = cell.type diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index 95c77d88..a70722ee 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -83,8 +83,14 @@ class TestAxlsx < Test::Unit::TestCase end end + def test_row_ref + assert_equal('1', Axlsx.row_ref(0)) + assert_equal('100', Axlsx.row_ref(99)) + end + def test_cell_r - # todo + assert_equal('A1', Axlsx.cell_r(0, 0)) + assert_equal('Z26', Axlsx.cell_r(25, 25)) end def test_range_to_a -- cgit v1.2.3 From 71b358f0c73c73f88275b4c1c89fdf5372281ada Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 14:51:41 -0700 Subject: Corrected rubocop offenses in lib/axlsx.rb / test/tc_axlsx.rb --- .rubocop_todo.yml | 23 ----------------------- lib/axlsx.rb | 46 ++++++++++++++++++++++----------------------- lib/axlsx/util/constants.rb | 2 ++ test/tc_axlsx.rb | 2 ++ 4 files changed, 27 insertions(+), 46 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..bc046dff 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -20,7 +20,6 @@ Layout/FirstHashElementIndentation: Layout/HashAlignment: Exclude: - 'lib/axlsx/workbook/worksheet/border_creator.rb' - - 'test/tc_axlsx.rb' # This cop supports safe autocorrection (--autocorrect). Lint/AmbiguousOperatorPrecedence: @@ -69,7 +68,6 @@ Lint/NonLocalExitFromIterator: # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/drawing/axes.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' - 'lib/axlsx/workbook/worksheet/sheet_view.rb' @@ -119,7 +117,6 @@ Performance/RedundantBlockCall: # This cop supports safe autocorrection (--autocorrect). Performance/RedundantMatch: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/color.rb' - 'lib/axlsx/workbook/workbook.rb' @@ -266,7 +263,6 @@ Style/GuardClause: # Configuration parameters: AllowSplatArgument. Style/HashConversion: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/workbook/worksheet/cell_serializer.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' @@ -321,7 +317,6 @@ Style/LineEndConcatenation: # Configuration parameters: AllowMethodComparison. 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' @@ -449,7 +444,6 @@ Style/PercentLiteralDelimiters: # This cop supports safe autocorrection (--autocorrect). Style/PerlBackrefs: Exclude: - - 'lib/axlsx.rb' - 'test/workbook/worksheet/tc_sheet_protection.rb' # This cop supports unsafe autocorrection (--autocorrect-all). @@ -471,7 +465,6 @@ Style/QuotedSymbols: # SupportedStyles: compact, exploded Style/RaiseArgs: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/package.rb' - 'lib/axlsx/util/zip_command.rb' - 'lib/axlsx/workbook/worksheet/border_creator.rb' @@ -490,7 +483,6 @@ Style/RedundantCondition: # This cop supports safe autocorrection (--autocorrect). Style/RedundantFileExtensionInRequire: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/content_type/content_type.rb' - 'lib/axlsx/drawing/drawing.rb' - 'lib/axlsx/rels/relationships.rb' @@ -528,7 +520,6 @@ Style/RedundantParentheses: # This cop supports safe autocorrection (--autocorrect). Style/RedundantRegexpEscape: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' - 'lib/axlsx/workbook/worksheet/table.rb' @@ -536,7 +527,6 @@ Style/RedundantRegexpEscape: # Configuration parameters: AllowMultipleReturnValues. Style/RedundantReturn: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/package.rb' - 'lib/axlsx/stylesheet/styles.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' @@ -552,13 +542,6 @@ Style/RegexpLiteral: Exclude: - 'lib/axlsx/workbook/worksheet/cell.rb' -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: implicit, explicit -Style/RescueStandardError: - Exclude: - - 'lib/axlsx.rb' - # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength. # AllowedMethods: present?, blank?, presence, try, try! @@ -578,7 +561,6 @@ Style/SingleLineMethods: # This cop supports unsafe autocorrection (--autocorrect-all). Style/SlicingWithRange: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/drawing/area_chart.rb' - 'lib/axlsx/drawing/line_chart.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' @@ -655,11 +637,6 @@ Style/UnpackFirst: Exclude: - 'lib/axlsx/workbook/worksheet/sheet_protection.rb' -# This cop supports safe autocorrection (--autocorrect). -Style/WhileUntilDo: - Exclude: - - 'lib/axlsx.rb' - # This cop supports safe autocorrection (--autocorrect). Style/WhileUntilModifier: Exclude: diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 6e6a317d..30683458 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -1,29 +1,29 @@ # frozen_string_literal: true require 'htmlentities' -require 'axlsx/version.rb' +require 'axlsx/version' require 'marcel' -require 'axlsx/util/simple_typed_list.rb' -require 'axlsx/util/constants.rb' -require 'axlsx/util/validators.rb' -require 'axlsx/util/accessors.rb' +require 'axlsx/util/simple_typed_list' +require 'axlsx/util/constants' +require 'axlsx/util/validators' +require 'axlsx/util/accessors' require 'axlsx/util/serialized_attributes' require 'axlsx/util/options_parser' require 'axlsx/util/mime_type_utils' require 'axlsx/util/buffered_zip_output_stream' require 'axlsx/util/zip_command' -require 'axlsx/stylesheet/styles.rb' +require 'axlsx/stylesheet/styles' -require 'axlsx/doc_props/app.rb' -require 'axlsx/doc_props/core.rb' -require 'axlsx/content_type/content_type.rb' -require 'axlsx/rels/relationships.rb' +require 'axlsx/doc_props/app' +require 'axlsx/doc_props/core' +require 'axlsx/content_type/content_type' +require 'axlsx/rels/relationships' -require 'axlsx/drawing/drawing.rb' -require 'axlsx/workbook/workbook.rb' -require 'axlsx/package.rb' +require 'axlsx/drawing/drawing' +require 'axlsx/workbook/workbook' +require 'axlsx/package' # required gems require 'nokogiri' require 'zip' @@ -35,9 +35,9 @@ require 'time' begin if Gem.loaded_specs.has_key?("axlsx_styler") - raise StandardError.new("Please remove `axlsx_styler` from your Gemfile, the associated functionality is now built-in to `caxlsx` directly.") + raise StandardError, "Please remove `axlsx_styler` from your Gemfile, the associated functionality is now built-in to `caxlsx` directly." end -rescue +rescue StandardError # Do nothing end @@ -53,7 +53,7 @@ module Axlsx # # Defining as a class method on Axlsx to refrain from monkeypatching Object for all users of this gem. def self.instance_values_for(object) - Hash[object.instance_variables.map { |name| [name.to_s[1..-1], object.instance_variable_get(name)] }] + object.instance_variables.to_h { |name| [name.to_s[1..], object.instance_variable_get(name)] } end # determines the cell range for the items provided @@ -105,7 +105,7 @@ module Axlsx row_index = (numbers_str.to_i - 1) - return [col_index, row_index] + [col_index, row_index] end # converts the column index into alphabetical values. @@ -147,9 +147,9 @@ module Axlsx # @param [String] range A cell range, for example A1:D5 # @return [Array] def self.range_to_a(range) - range.match(/^(\w+?\d+)\:(\w+?\d+)$/) - start_col, start_row = name_to_indices($1) - end_col, end_row = name_to_indices($2) + range =~ /^(\w+?\d+):(\w+?\d+)$/ + start_col, start_row = name_to_indices(::Regexp.last_match(1)) + end_col, end_row = name_to_indices(::Regexp.last_match(2)) (start_row..end_row).to_a.map do |row_num| (start_col..end_col).to_a.map do |col_num| cell_r(col_num, row_num) @@ -163,7 +163,7 @@ module Axlsx def self.camel(s = "", all_caps = true) s = s.to_s s = s.capitalize if all_caps - s.gsub(/_(.)/) { $1.upcase } + s.gsub(/_(.)/) { ::Regexp.last_match(1).upcase } end # returns the provided string with all invalid control charaters @@ -184,7 +184,7 @@ module Axlsx # @param [Object] value The value to process # @return [Object] def self.booleanize(value) - if value == true || value == false + if BOOLEAN_VALUES.include?(value) value ? 1 : 0 else value @@ -195,7 +195,7 @@ module Axlsx # @param [Hash] Hash to merge into # @param [Hash] Hash to be added def self.hash_deep_merge(first_hash, second_hash) - first_hash.merge(second_hash) do |key, this_val, other_val| + first_hash.merge(second_hash) do |_key, this_val, other_val| if this_val.is_a?(Hash) && other_val.is_a?(Hash) Axlsx.hash_deep_merge(this_val, other_val) else diff --git a/lib/axlsx/util/constants.rb b/lib/axlsx/util/constants.rb index 4697bb3a..af8d9702 100644 --- a/lib/axlsx/util/constants.rb +++ b/lib/axlsx/util/constants.rb @@ -413,4 +413,6 @@ module Axlsx # Numeric recognition NUMERIC_REGEX = /\A[+-]?\d+?\Z/.freeze + + BOOLEAN_VALUES = [true, false].freeze end diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index a70722ee..3c4b5f93 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -3,6 +3,7 @@ require 'tc_helper' class TestAxlsx < Test::Unit::TestCase + # rubocop:disable Layout/HashAlignment def setup_wide @wide_test_points = { "A3" => 0, @@ -15,6 +16,7 @@ class TestAxlsx < Test::Unit::TestCase "BZU3" => (2 * (26**2)) + (26 * 26) + 20 } end + # rubocop:enable Layout/HashAlignment def test_cell_range_empty_if_no_cell assert_equal("", Axlsx.cell_range([])) -- cgit v1.2.3 From 57865b2f08ae301f4d6815211fba59fefafcec33 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sun, 14 May 2023 20:01:22 -0700 Subject: Also cache row_ref The `row_ref` method is called once for each column in a row and once at the row level. --- lib/axlsx.rb | 4 ++-- lib/axlsx/workbook/worksheet/row.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 30683458..bf16afbd 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -125,7 +125,6 @@ module Axlsx end chars.prepend((i + 65).chr) chars.freeze - chars end end @@ -133,7 +132,8 @@ module Axlsx # @note The spreadsheet rows are 1-based and the passed in index is 0-based, so we add 1. # @return [String] def self.row_ref(index) - (index + 1).to_s + @row_ref ||= {} + @row_ref[index] ||= (index + 1).to_s.freeze end # @return [String] The alpha(column)numeric(row) reference for this sell. diff --git a/lib/axlsx/workbook/worksheet/row.rb b/lib/axlsx/workbook/worksheet/row.rb index 16116835..dc0320b7 100644 --- a/lib/axlsx/workbook/worksheet/row.rb +++ b/lib/axlsx/workbook/worksheet/row.rb @@ -89,7 +89,7 @@ module Axlsx # @param [String] str The string this rows xml will be appended to. # @return [String] def to_xml_string(r_index, str = +'') - serialized_tag('row', str, :r => r_index + 1) do + serialized_tag('row', str, :r => Axlsx.row_ref(r_index)) do each_with_index { |cell, c_index| cell.to_xml_string(r_index, c_index, str) } end end -- cgit v1.2.3 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 From 6eb2fc56d3ab658edc1477d138b1cf0b3021ab29 Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Tue, 16 May 2023 15:54:08 +0200 Subject: Replace `sub` with `delete_prefix`/`delete_suffix` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby 2.5 introduced `delete_prefix` and `delete_suffix`. Those methods are helpful when serializing formula and array formula values, that are supposed to start and end with given prefixes Also moves formula prefix to constants so they can be used by both `Cell` and `CellSerializer` classes Formula: ``` Ruby version: 3.2.2 Comparison: delete_prefix: 8759353.5 i/s sub: 2607022.4 i/s - 3.36x (± 0.00) slower Comparison: delete_prefix: 40 allocated sub: 160 allocated - 4.00x more ``` Array Formula: ``` Ruby version: 3.2.2 Comparison: delete_prefixes: 4798837.8 i/s sub_sub: 937072.1 i/s - 5.12x (± 0.00) slower Comparison: delete_prefixes: 120 allocated sub_sub: 488 allocated - 4.07x more ``` --- lib/axlsx/util/constants.rb | 10 ++++++++++ lib/axlsx/workbook/worksheet/cell.rb | 14 ++------------ lib/axlsx/workbook/worksheet/cell_serializer.rb | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/axlsx/util/constants.rb b/lib/axlsx/util/constants.rb index 4697bb3a..42a7683c 100644 --- a/lib/axlsx/util/constants.rb +++ b/lib/axlsx/util/constants.rb @@ -413,4 +413,14 @@ module Axlsx # Numeric recognition NUMERIC_REGEX = /\A[+-]?\d+?\Z/.freeze + + # Leading characters that indicate a formula. + # See: https://owasp.org/www-community/attacks/CSV_Injection + FORMULA_PREFIX = '=' + + # Leading characters that indicate an array formula. + ARRAY_FORMULA_PREFIX = '{=' + + # Trailing character that indicates an array formula. + ARRAY_FORMULA_SUFFIX = '}' end diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 929a7c28..f47781f0 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -72,16 +72,6 @@ module Axlsx CELL_TYPES = [:date, :time, :float, :integer, :richtext, :string, :boolean, :iso_8601, :text].freeze - # Leading characters that indicate a formula. - # See: https://owasp.org/www-community/attacks/CSV_Injection - FORMULA_PREFIXES = ['='].freeze - - # Leading characters that indicate an array formula. - ARRAY_FORMULA_PREFIXES = ['{='].freeze - - # Trailing character that indicates an array formula. - ARRAY_FORMULA_SUFFIX = '}' - # The index of the cellXfs item to be applied to this cell. # @return [Integer] # @see Axlsx::Styles @@ -395,14 +385,14 @@ module Axlsx def is_formula? return false if escape_formulas - type == :string && @value.to_s.start_with?(*FORMULA_PREFIXES) + type == :string && @value.to_s.start_with?(FORMULA_PREFIX) end def is_array_formula? return false if escape_formulas type == :string && - @value.to_s.start_with?(*ARRAY_FORMULA_PREFIXES) && + @value.to_s.start_with?(ARRAY_FORMULA_PREFIX) && @value.to_s.end_with?(ARRAY_FORMULA_SUFFIX) end diff --git a/lib/axlsx/workbook/worksheet/cell_serializer.rb b/lib/axlsx/workbook/worksheet/cell_serializer.rb index e1bdf728..9d39add3 100644 --- a/lib/axlsx/workbook/worksheet/cell_serializer.rb +++ b/lib/axlsx/workbook/worksheet/cell_serializer.rb @@ -88,7 +88,7 @@ module Axlsx # @param [String] str The string the serialized content will be appended to. # @return [String] def formula_serialization(cell, str = +'') - str << 't="str">' << cell.clean_value.to_s.sub('=', '') << '' + str << 't="str">' << cell.clean_value.to_s.delete_prefix(FORMULA_PREFIX) << '' str << '' << cell.formula_value.to_s << '' unless cell.formula_value.nil? end @@ -97,7 +97,7 @@ module Axlsx # @param [String] str The string the serialized content will be appended to. # @return [String] def array_formula_serialization(cell, str = +'') - str << 't="str">' << '' << cell.clean_value.to_s.sub('{=', '').sub(/}$/, '') << '' + str << 't="str">' << '' << cell.clean_value.to_s.delete_prefix(ARRAY_FORMULA_PREFIX).delete_suffix(ARRAY_FORMULA_SUFFIX) << '' str << '' << cell.formula_value.to_s << '' unless cell.formula_value.nil? end -- cgit v1.2.3