Merge pull request #233 from alexjfisher/fix_duplicated_tokens

Fix `Ensure check methods can't modify tokens array`

Author: LukasAud

.github/workflows/ci.yml

@@ -21,6 +21,7 @@ jobs:
         ruby_version:
           - '3.2'
           - '3.3'
+          - '3.4'
     name: "spec (ruby ${{ matrix.ruby_version }})"
     uses: "puppetlabs/cat-github-actions/.github/workflows/gem_ci.yml@main"
     secrets: "inherit"
@@ -35,6 +36,7 @@ jobs:
         ruby_version:
           - '3.2'
           - '3.3'
+          - '3.4'
     name: "acceptance (ruby ${{ matrix.ruby_version }})"
     needs: "spec"
     uses: "puppetlabs/cat-github-actions/.github/workflows/gem_acceptance.yml@main"

.rubocop_todo.yml

@@ -129,14 +129,15 @@ RSpec/RepeatedExampleGroupDescription:
     - 'spec/unit/puppet-lint/plugins/check_resources/unquoted_file_mode_spec.rb'
     - 'spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb'
 
-# Offense count: 8
+# Offense count: 9
 # Configuration parameters: Include, CustomTransform, IgnoreMethods, IgnoreMetadata.
 # Include: **/*_spec.rb
 RSpec/SpecFilePathFormat:
   Exclude:
     - '**/spec/routing/**/*'
     - 'spec/unit/puppet-lint/bin_spec.rb'
     - 'spec/unit/puppet-lint/checks_spec.rb'
+    - 'spec/unit/puppet-lint/checkplugin_spec.rb'
     - 'spec/unit/puppet-lint/configuration_spec.rb'
     - 'spec/unit/puppet-lint/data_spec.rb'
     - 'spec/unit/puppet-lint/lexer/string_slurper_spec.rb'

lib/puppet-lint/checkplugin.rb

@@ -55,7 +55,9 @@ def fix_problems
   #
   # Returns an Array of PuppetLint::Lexer::Token objects.
   def tokens
-    PuppetLint::Data.tokens
+    # When called from a plugins `check` method, the tokens array returned should be a (shallow) copy
+    called_from_check = (caller_locations(1..1).first.base_label == 'check')
+    PuppetLint::Data.tokens(duplicate: called_from_check)
   end
 
   def add_token(index, token)

lib/puppet-lint/data.rb

@@ -38,37 +38,14 @@ def tokens=(tokens)
       @defaults_indexes = nil
     end
 
-    # @api private
-    def ruby1?
-      @ruby1 = RbConfig::CONFIG['MAJOR'] == '1' if @ruby1.nil?
-      @ruby1
-    end
-
     # Get the tokenised manifest.
     #
+    # @param duplicate [Boolean] if true, returns a duplicate of the token array.
     # @return [Array[PuppetLint::Lexer::Token]]
     #
     # @api public
-    def tokens
-      calling_method = if ruby1?
-                         begin
-                           caller[0][%r{`.*'}][1..-2] # rubocop:disable Performance/Caller
-                         rescue NoMethodError
-                           caller[1][%r{`.*'}][1..-2] # rubocop:disable Performance/Caller
-                         end
-                       else
-                         begin
-                           caller(0..0).first[%r{`.*'}][1..-2]
-                         rescue NoMethodError
-                           caller(1..1).first[%r{`.*'}][1..-2]
-                         end
-                       end
-
-      if calling_method == 'check'
-        @tokens.dup
-      else
-        @tokens
-      end
+    def tokens(duplicate: false)
+      duplicate ? @tokens.dup : @tokens
     end
 
     # Add new token.

spec/unit/puppet-lint/checkplugin_spec.rb

@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+class DummyCheckPlugin < PuppetLint::CheckPlugin
+  def check
+    # Since we're calling `tokens` from a `check` method, we should get our own Array object.
+    # If we add an extra token to it, PuppetLint::Data.tokens should remain unchanged.
+    tokens << :extra_token
+  end
+
+  def fix
+    tokens << :fix_token
+  end
+end
+
+describe PuppetLint::CheckPlugin do
+  before(:each) do
+    PuppetLint::Data.tokens = [:token1, :token2, :token3]
+  end
+
+  it 'returns a duplicate of the token array when called from check' do
+    plugin = DummyCheckPlugin.new
+
+    plugin.check
+
+    # Verify that the global token array remains unchanged.
+    expect(PuppetLint::Data.tokens).to eq([:token1, :token2, :token3])
+  end
+
+  it 'other methods can modify the tokens array' do
+    plugin = DummyCheckPlugin.new
+
+    plugin.fix
+
+    expect(PuppetLint::Data.tokens).to eq([:token1, :token2, :token3, :fix_token])
+  end
+end