Skip to content

Commit 9e453e0

Browse files
committed
(puppetlabs#240) Fix output of default values that are expressions
Previously, parameters with a default value that was an expression were not outputted into the documentation correctly. For example, Integer $param = 1 + 1, Would be shown in the documentation to have a default value of “+”. This switches to using `extract_tree_text` instead of `extract_text` to get the text representation of the parsed Puppet code. This also gets rid of the dependency on `Puppet::Pops::Adapters::SourcePosAdapter`, which was [deprecated in 2017](puppetlabs/puppet@68498ad). Unfortunately, it appears that there is a bug in Puppet ([PUP-11632][]) that sometimes returns the incorrect default value. This adds two skipped tests that demonstrate the issue. [PUP-11632]: https://tickets.puppetlabs.com/browse/PUP-11632
1 parent 3991e75 commit 9e453e0

File tree

5 files changed

+83
-15
lines changed

5 files changed

+83
-15
lines changed

lib/puppet-strings/yard/handlers/ruby/data_type_handler.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ def literal_Object(o)
150150

151151
def literal_AccessExpression(o)
152152
# Extract the raw text of the Access Expression
153-
::Puppet::Pops::Adapters::SourcePosAdapter.adapt(o).extract_text
153+
PuppetStrings::Yard::Util.ast_to_text(o)
154154
end
155155

156156
def literal_QualifiedReference(o)
157157
# Extract the raw text of the Qualified Reference
158-
::Puppet::Pops::Adapters::SourcePosAdapter.adapt(o).extract_text
158+
PuppetStrings::Yard::Util.ast_to_text(o)
159159
end
160160

161161
# ----- The following methods are the same as the original Literal_evaluator

lib/puppet-strings/yard/parsers/puppet/statement.rb

+8-13
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ class Statement
2121
def initialize(object, file)
2222
@file = file
2323

24-
adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(object)
25-
@source = adapter.extract_text
26-
@line = adapter.line
24+
@source = PuppetStrings::Yard::Util.ast_to_text(object)
25+
@line = object.line
2726
@comments_range = nil
2827
end
2928

@@ -85,13 +84,11 @@ def initialize(parameter)
8584
@name = parameter.name
8685
# Take the exact text for the type expression
8786
if parameter.type_expr
88-
adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(parameter.type_expr)
89-
@type = adapter.extract_text
87+
@type = PuppetStrings::Yard::Util.ast_to_text(parameter.type_expr)
9088
end
9189
# Take the exact text for the default value expression
9290
if parameter.value
93-
adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(parameter.value)
94-
@value = adapter.extract_text
91+
@value = PuppetStrings::Yard::Util.ast_to_text(parameter.value)
9592
end
9693
end
9794
end
@@ -149,8 +146,7 @@ def initialize(object, file)
149146
if object.respond_to? :return_type
150147
type = object.return_type
151148
if type
152-
adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type)
153-
@type = adapter.extract_text.gsub('>> ', '')
149+
@type = PuppetStrings::Yard::Util.ast_to_text(type).gsub('>> ', '')
154150
end
155151
end
156152
end
@@ -184,12 +180,11 @@ def initialize(object, file)
184180
case type_expr
185181
when Puppet::Pops::Model::AccessExpression
186182
# TODO: I don't like rebuilding the source from the AST, but AccessExpressions don't expose the original source
187-
@alias_of = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type_expr.left_expr).extract_text + '['
188-
@alias_of << type_expr.keys.map { |key| ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(key).extract_text }.join(', ')
183+
@alias_of = PuppetStrings::Yard::Util.ast_to_text(type_expr.left_expr) + '['
184+
@alias_of << type_expr.keys.map { |key| PuppetStrings::Yard::Util.ast_to_text(key) }.join(', ')
189185
@alias_of << ']'
190186
else
191-
adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type_expr)
192-
@alias_of = adapter.extract_text
187+
@alias_of = PuppetStrings::Yard::Util.ast_to_text(type_expr)
193188
end
194189
@name = object.name
195190
end

lib/puppet-strings/yard/util.rb

+7
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,11 @@ def self.docstring_to_hash(docstring, select_tags=nil)
7979

8080
hash
8181
end
82+
83+
# Convert Puppet AST to text.
84+
# @param [Puppet::Pops::Model::PopsObject] ast The Puppet AST to convert to text.
85+
# @return [String] Returns a string of Puppet code.
86+
def self.ast_to_text(ast)
87+
ast.locator.extract_tree_text(ast)
88+
end
8289
end

spec/unit/puppet-strings/yard/parsers/puppet/parser_spec.rb

+59
Original file line numberDiff line numberDiff line change
@@ -250,4 +250,63 @@ class bar {
250250
end
251251
end
252252
end
253+
254+
[
255+
'undef',
256+
'true',
257+
'-1',
258+
'0.34',
259+
'bareword',
260+
"'single quotes'",
261+
'"double quotes"',
262+
'[]',
263+
'[1]',
264+
'{}',
265+
'{ a => 1 }',
266+
'$param1',
267+
'1 + 1',
268+
'func()',
269+
'$param1.foo(1)',
270+
'$param1.foo($param2 + $param3.bar())',
271+
].each do |value|
272+
describe "parsing parameter with #{value} default value" do
273+
let(:source) { <<~PUPPET }
274+
class foo (
275+
$param1 = #{value},
276+
) {}
277+
PUPPET
278+
279+
it 'finds correct value' do
280+
subject.parse
281+
statement = subject.enumerator.first
282+
expect(statement.parameters.size).to eq(1)
283+
expect(statement.parameters[0].value).to eq(value)
284+
end
285+
end
286+
end
287+
288+
[
289+
'$param1.foo()',
290+
"$facts['kernel'] ? {
291+
'Linux' => 'linux',
292+
'Darwin' => 'darwin',
293+
default => $facts['kernel'],
294+
}",
295+
].each do |value|
296+
describe "parsing parameter with #{value} default value" do
297+
let(:source) { <<~PUPPET }
298+
class foo (
299+
$param1 = #{value},
300+
) {}
301+
PUPPET
302+
303+
it 'finds correct value' do
304+
skip('Broken by https://tickets.puppetlabs.com/browse/PUP-11632')
305+
subject.parse
306+
statement = subject.enumerator.first
307+
expect(statement.parameters.size).to eq(1)
308+
expect(statement.parameters[0].value).to eq(value)
309+
end
310+
end
311+
end
253312
end

spec/unit/puppet-strings/yard/util_spec.rb

+7
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,11 @@
4747
expect(subject.github_to_yard_links(str)).to eq('<a href="#label-Module+description"> module-description')
4848
end
4949
end
50+
51+
describe 'ast_to_text' do
52+
it 'converts a simple AST correctly' do
53+
model = Puppet::Pops::Parser::Parser.new.parse_string('class test {}').model
54+
expect(described_class.ast_to_text(model.body)).to eq('class test {}')
55+
end
56+
end
5057
end

0 commit comments

Comments
 (0)