diff --git a/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb b/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb index 56c1949b..3e83d7be 100644 --- a/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb +++ b/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb @@ -115,10 +115,14 @@ def check tokens.select { |x| LEGACY_FACTS_VAR_TYPES.include?(x.type) }.each do |token| fact_name = '' - # Get rid of the top scope before we do our work. We don't need to - # preserve it because it won't work with the new structured facts. - if token.value.start_with?('::') - fact_name = token.value.sub(%r{^::}, '') + # This matches legacy facts defined in the fact hash that use the top scope + # fact assignment. + if token.value.start_with?('::facts[') + fact_name = token.value.match(%r{::facts\['(.*)'\]})[1] + + # This matches legacy facts defined in the fact hash. + elsif token.value.start_with?("facts['") + fact_name = token.value.match(%r{facts\['(.*)'\]})[1] # This matches using legacy facts in a the new structured fact. For # example this would match 'uuid' in $facts['uuid'] so it can be converted @@ -126,8 +130,10 @@ def check elsif token.value == 'facts' fact_name = hash_key_for(token) - elsif token.value.start_with?("facts['") - fact_name = token.value.match(%r{facts\['(.*)'\]})[1] + # Now we can get rid of top scopes. We don't need to + # preserve it because it won't work with the new structured facts. + elsif token.value.start_with?('::') + fact_name = token.value.sub(%r{^::}, '') end next unless EASY_FACTS.include?(fact_name) || UNCONVERTIBLE_FACTS.include?(fact_name) || fact_name.match(Regexp.union(REGEX_FACTS)) diff --git a/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb b/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb index 0e43be14..1a726993 100644 --- a/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb +++ b/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb @@ -113,6 +113,14 @@ expect(problems).to have(1).problem end end + + context 'top scoped fact variable using legacy facts hash variable in interpolation' do + let(:code) { "$::facts['osfamily']" } + + it 'detects a single problem' do + expect(problems).to have(1).problem + end + end end context 'with fix enabled' do @@ -165,6 +173,23 @@ end end + context "fact variable using legacy $::facts['osfamily']" do + let(:code) { "$::facts['osfamily']" } + let(:msg) { "legacy fact 'osfamily'" } + + it 'only detects a single problem' do + expect(problems).to have(1).problem + end + + it 'fixes the problem' do + expect(problems).to contain_fixed(msg).on_line(1).in_column(1) + end + + it 'uses the facts hash' do + expect(manifest).to eq("$facts['os']['family']") + end + end + context 'fact variable using legacy $::osfamily' do let(:code) { '$::osfamily' } let(:msg) { "legacy fact 'osfamily'" }