From 68d05074f9a41496d9d63dd28bcd6e2308bc22a1 Mon Sep 17 00:00:00 2001 From: Chedli Bourguiba Date: Sat, 15 Jun 2024 02:49:27 +0200 Subject: [PATCH] Fix frozen string literal issue --- .github/workflows/ci.yml | 20 ++++- lib/fog/parsers/base.rb | 2 +- lib/fog/to_hash_document.rb | 12 +-- lib/fog/xml/response.rb | 4 +- spec/fog/to_hash_document_spec.rb | 142 ++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 10 deletions(-) create mode 100644 spec/fog/to_hash_document_spec.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f232d7b..9c4e456 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,5 +12,21 @@ permissions: contents: read jobs: - Shared: - uses: fog/.github/.github/workflows/ci.yml@v1.4.0 + test: + + runs-on: ubuntu-latest + + strategy: + matrix: + ruby-version: ['3.0', '3.1', '3.2', '3.3', 'head'] + continue-on-error: ${{ matrix.ruby-version == 'head' }} + + steps: + - uses: actions/checkout@v4 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true # runs 'bundle install' and caches installed gems automatically + - name: Run tests + run: bundle exec rake RUBYOPT="--enable-frozen-string-literal" diff --git a/lib/fog/parsers/base.rb b/lib/fog/parsers/base.rb index 7e551d2..259d171 100644 --- a/lib/fog/parsers/base.rb +++ b/lib/fog/parsers/base.rb @@ -18,7 +18,7 @@ def reset end def characters(string) - @value ||= '' + @value ||= +'' @value << string end diff --git a/lib/fog/to_hash_document.rb b/lib/fog/to_hash_document.rb index a86c3b5..15df2cf 100644 --- a/lib/fog/to_hash_document.rb +++ b/lib/fog/to_hash_document.rb @@ -5,20 +5,22 @@ def initialize end def characters(string) - @value ||= "" + @value ||= +'' @value << string.strip end def end_element(name) last = @stack.pop - if last.empty? && @value.empty? - @stack.last[name.to_sym] = "" + @stack.push({}) if @stack.empty? + + if last&.empty? && @value.empty? + @stack.last[name.to_sym] = +'' elsif last == { :i_nil => "true" } @stack.last[name.to_sym] = nil elsif !@value.empty? @stack.last[name.to_sym] = @value end - @value = "" + @value = +'' end def body @@ -30,7 +32,7 @@ def response end def start_element(name, attributes = []) - @value = "" + @value = +'' parsed_attributes = {} until attributes.empty? if attributes.first.is_a?(Array) diff --git a/lib/fog/xml/response.rb b/lib/fog/xml/response.rb index 1222abc..f4c9d9b 100644 --- a/lib/fog/xml/response.rb +++ b/lib/fog/xml/response.rb @@ -4,7 +4,7 @@ class Response def initialize(parser) @parser = parser @data_stream = Nokogiri::XML::SAX::PushParser.new(parser) - @response_string = "" + @response_string = +'' end def call(chunk, _remaining, _total) @@ -14,7 +14,7 @@ def call(chunk, _remaining, _total) def rewind @parser.reset - @response_string = "" + @response_string = +'' end def finish diff --git a/spec/fog/to_hash_document_spec.rb b/spec/fog/to_hash_document_spec.rb new file mode 100644 index 0000000..6adeabc --- /dev/null +++ b/spec/fog/to_hash_document_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'minitest_helper' +require 'fog/xml' + +# We expose accessors just for testing purposes +Fog::ToHashDocument.attr_accessor(:value, :stack) + +describe Fog::ToHashDocument do + before do + @document = Fog::ToHashDocument.new + end + + describe '#characters' do + it 'appends characters to @value' do + @document.characters('some text') + _(@document.value).must_equal 'some text' + end + + it 'strips whitespace from characters' do + @document.characters(' some text ') + _(@document.value).must_equal 'some text' + end + end + + describe '#end_element' do + before do + @document.stack << {} + @document.characters('some text') + end + + it 'adds element with text content to the stack' do + @document.end_element('element') + + expected = { element: 'some text' } + _(@document.stack.last).must_equal(expected) + end + + it 'can mutate the new empty value' do + @document.end_element('element') + + _(@document.value).must_equal('') + + # Mutate the new empty value even when frozen string literals are enabled + _(@document.characters('one')) + end + + it 'adds empty string if element is empty and value is empty' do + @document.value = '' + + @document.end_element('element') + + expected = { element: '' } + _(@document.stack.last).must_equal(expected) + end + + it 'adds nil if element has :i_nil attribute' do + @document.stack.last[:i_nil] = 'true' + @document.value = '' + + @document.end_element('element') + + expected = { element: nil } + _(@document.stack.last).must_equal(expected) + end + end + + describe '#body' do + it 'returns the first element of the stack' do + @document.stack << { key: 'value' } + + expected = { key: 'value' } + _(@document.body).must_equal(expected) + end + end + + describe '#response' do + it 'returns the body' do + @document.stack << { key: 'value' } + + expected = { key: 'value' } + _(@document.response).must_equal(expected) + end + end + + describe '#start_element' do + it 'parses attributes correctly' do + @document.start_element('element', [%w[key value]]) + + expected = { key: 'value' } + _(@document.stack.last).must_equal(expected) + end + + it 'handles elements without attributes' do + @document.start_element('element') + + _(@document.stack.last).must_equal({}) + end + + it 'adds nested elements to the stack' do + @document.start_element('parent') + @document.start_element('child') + + _(@document.stack).must_equal([{ child: {} }, { child: {} }, {}]) + end + + it 'adds nested elements with attributes to the stack' do + @document.start_element('parent') + @document.start_element('child', [%w[key value]]) + expected = [ + { child: { key: 'value' } }, + { child: { key: 'value' } }, + { key: 'value' } + ] + + _(@document.stack).must_equal(expected) + end + + it 'handles multiple children elements correctly' do + @document.start_element('parent') + @document.start_element('child1') + @document.end_element('child1') + @document.start_element('child2', [%w[key value]]) + @document.end_element('child2') + expected = { + child1: '', + child2: { key: 'value' } + } + + _(@document.stack.first).must_equal(expected) + end + + it 'handles text content within elements' do + @document.start_element('parent') + @document.characters('some text') + @document.end_element('parent') + + expected = { parent: 'some text' } + _(@document.stack.first).must_equal(expected) + end + end +end