Skip to content

Commit a3b7241

Browse files
committed
(CONT-675) Fix fact detection
Prior to this change the legacy facts check would not detect legacy facts defined as follows: ``` $::facts['osfamily'] ```` This changes updates the `check` logic to evaluate tokens that may start with `::` and extract the fact name as appropriate. `
1 parent b029765 commit a3b7241

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,19 +115,25 @@ def check
115115
tokens.select { |x| LEGACY_FACTS_VAR_TYPES.include?(x.type) }.each do |token|
116116
fact_name = ''
117117

118-
# Get rid of the top scope before we do our work. We don't need to
119-
# preserve it because it won't work with the new structured facts.
120-
if token.value.start_with?('::')
121-
fact_name = token.value.sub(%r{^::}, '')
118+
# This matches legacy facts defined in the fact hash that use the top scope
119+
# fact assignment.
120+
if token.value.start_with?('::facts[')
121+
fact_name = token.value.match(%r{::facts\['(.*)'\]})[1]
122+
123+
# This matches legacy facts defined in the fact hash.
124+
elsif token.value.start_with?("facts['")
125+
fact_name = token.value.match(%r{facts\['(.*)'\]})[1]
122126

123127
# This matches using legacy facts in a the new structured fact. For
124128
# example this would match 'uuid' in $facts['uuid'] so it can be converted
125129
# to facts['dmi']['product']['uuid']"
126130
elsif token.value == 'facts'
127131
fact_name = hash_key_for(token)
128132

129-
elsif token.value.start_with?("facts['")
130-
fact_name = token.value.match(%r{facts\['(.*)'\]})[1]
133+
# Now we can get rid of top scopes. We don't need to
134+
# preserve it because it won't work with the new structured facts.
135+
elsif token.value.start_with?('::')
136+
fact_name = token.value.sub(%r{^::}, '')
131137
end
132138

133139
next unless EASY_FACTS.include?(fact_name) || UNCONVERTIBLE_FACTS.include?(fact_name) || fact_name.match(Regexp.union(REGEX_FACTS))

spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@
113113
expect(problems).to have(1).problem
114114
end
115115
end
116+
117+
context 'top scoped fact variable using legacy facts hash variable in interpolation' do
118+
let(:code) { "$::facts['osfamily']" }
119+
120+
it 'detects a single problem' do
121+
expect(problems).to have(1).problem
122+
end
123+
end
116124
end
117125

118126
context 'with fix enabled' do
@@ -165,6 +173,31 @@
165173
end
166174
end
167175

176+
context 'fact variable using top scope $::facts hash' do
177+
let(:code) { "$::facts['os']['family']" }
178+
179+
it 'does not detect any problems' do
180+
expect(problems).to have(0).problem
181+
end
182+
end
183+
184+
context "fact variable using legacy top scope $::facts['osfamily']" do
185+
let(:code) { "$::facts['osfamily']" }
186+
let(:msg) { "legacy fact 'osfamily'" }
187+
188+
it 'only detects a single problem' do
189+
expect(problems).to have(1).problem
190+
end
191+
192+
it 'fixes the problem' do
193+
expect(problems).to contain_fixed(msg).on_line(1).in_column(1)
194+
end
195+
196+
it 'uses the facts hash' do
197+
expect(manifest).to eq("$facts['os']['family']")
198+
end
199+
end
200+
168201
context 'fact variable using legacy $::osfamily' do
169202
let(:code) { '$::osfamily' }
170203
let(:msg) { "legacy fact 'osfamily'" }

0 commit comments

Comments
 (0)