From 897b13ff7c158dad075dd38518c58e32e3b96cde Mon Sep 17 00:00:00 2001 From: Daniel Parks Date: Sun, 25 Sep 2022 02:04:56 -0700 Subject: [PATCH] (#240) Fix output of default values that are expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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](https://github.com/puppetlabs/puppet/commit/68498adfa2560c9cbf28d7a46ebc1aef14c69992). 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 --- .../yard/handlers/ruby/data_type_handler.rb | 4 +- .../yard/parsers/puppet/statement.rb | 21 +++---- lib/puppet-strings/yard/util.rb | 7 +++ .../yard/parsers/puppet/parser_spec.rb | 59 +++++++++++++++++++ spec/unit/puppet-strings/yard/util_spec.rb | 7 +++ 5 files changed, 83 insertions(+), 15 deletions(-) diff --git a/lib/puppet-strings/yard/handlers/ruby/data_type_handler.rb b/lib/puppet-strings/yard/handlers/ruby/data_type_handler.rb index b15a6939c..b36ab4351 100644 --- a/lib/puppet-strings/yard/handlers/ruby/data_type_handler.rb +++ b/lib/puppet-strings/yard/handlers/ruby/data_type_handler.rb @@ -152,12 +152,12 @@ def literal_Object(o) def literal_AccessExpression(o) # Extract the raw text of the Access Expression - ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(o).extract_text + PuppetStrings::Yard::Util.ast_to_text(o) end def literal_QualifiedReference(o) # Extract the raw text of the Qualified Reference - ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(o).extract_text + PuppetStrings::Yard::Util.ast_to_text(o) end # ----- The following methods are the same as the original Literal_evaluator diff --git a/lib/puppet-strings/yard/parsers/puppet/statement.rb b/lib/puppet-strings/yard/parsers/puppet/statement.rb index 54cf7f625..a52479406 100644 --- a/lib/puppet-strings/yard/parsers/puppet/statement.rb +++ b/lib/puppet-strings/yard/parsers/puppet/statement.rb @@ -21,9 +21,8 @@ class Statement def initialize(object, file) @file = file - adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(object) - @source = adapter.extract_text - @line = adapter.line + @source = PuppetStrings::Yard::Util.ast_to_text(object) + @line = object.line @comments_range = nil end @@ -86,13 +85,11 @@ def initialize(parameter) @name = parameter.name # Take the exact text for the type expression if parameter.type_expr - adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(parameter.type_expr) - @type = adapter.extract_text + @type = PuppetStrings::Yard::Util.ast_to_text(parameter.type_expr) end # Take the exact text for the default value expression return unless parameter.value - adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(parameter.value) - @value = adapter.extract_text + @value = PuppetStrings::Yard::Util.ast_to_text(parameter.value) end end @@ -149,8 +146,7 @@ def initialize(object, file) return unless object.respond_to? :return_type type = object.return_type return unless type - adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type) - @type = adapter.extract_text.gsub('>> ', '') + @type = PuppetStrings::Yard::Util.ast_to_text(type).gsub('>> ', '') end end @@ -182,12 +178,11 @@ def initialize(object, file) case type_expr when Puppet::Pops::Model::AccessExpression # TODO: I don't like rebuilding the source from the AST, but AccessExpressions don't expose the original source - @alias_of = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type_expr.left_expr).extract_text + '[' - @alias_of << type_expr.keys.map { |key| ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(key).extract_text }.join(', ') + @alias_of = PuppetStrings::Yard::Util.ast_to_text(type_expr.left_expr) + '[' + @alias_of << type_expr.keys.map { |key| PuppetStrings::Yard::Util.ast_to_text(key) }.join(', ') @alias_of << ']' else - adapter = ::Puppet::Pops::Adapters::SourcePosAdapter.adapt(type_expr) - @alias_of = adapter.extract_text + @alias_of = PuppetStrings::Yard::Util.ast_to_text(type_expr) end @name = object.name end diff --git a/lib/puppet-strings/yard/util.rb b/lib/puppet-strings/yard/util.rb index d51b4f4ee..75c13691a 100644 --- a/lib/puppet-strings/yard/util.rb +++ b/lib/puppet-strings/yard/util.rb @@ -79,4 +79,11 @@ def self.docstring_to_hash(docstring, select_tags = nil) hash end + + # Convert Puppet AST to text. + # @param [Puppet::Pops::Model::PopsObject] ast The Puppet AST to convert to text. + # @return [String] Returns a string of Puppet code. + def self.ast_to_text(ast) + ast.locator.extract_tree_text(ast) + end end diff --git a/spec/unit/puppet-strings/yard/parsers/puppet/parser_spec.rb b/spec/unit/puppet-strings/yard/parsers/puppet/parser_spec.rb index a3baa0492..0a626e890 100644 --- a/spec/unit/puppet-strings/yard/parsers/puppet/parser_spec.rb +++ b/spec/unit/puppet-strings/yard/parsers/puppet/parser_spec.rb @@ -260,4 +260,63 @@ class bar { end end end + + [ + 'undef', + 'true', + '-1', + '0.34', + 'bareword', + "'single quotes'", + '"double quotes"', + '[]', + '[1]', + '{}', + '{ a => 1 }', + '$param1', + '1 + 1', + 'func()', + '$param1.foo(1)', + '$param1.foo($param2 + $param3.bar())', + ].each do |value| + describe "parsing parameter with #{value} default value" do + let(:source) { <<~PUPPET } + class foo ( + $param1 = #{value}, + ) {} + PUPPET + + it 'finds correct value' do + spec_subject.parse + statement = spec_subject.enumerator.first + expect(statement.parameters.size).to eq(1) + expect(statement.parameters[0].value).to eq(value) + end + end + end + + [ + '$param1.foo()', + "$facts['kernel'] ? { + 'Linux' => 'linux', + 'Darwin' => 'darwin', + default => $facts['kernel'], + }", + ].each do |value| + describe "parsing parameter with #{value} default value" do + let(:source) { <<~PUPPET } + class foo ( + $param1 = #{value}, + ) {} + PUPPET + + it 'finds correct value' do + skip('Broken by https://tickets.puppetlabs.com/browse/PUP-11632') + spec_subject.parse + statement = spec_subject.enumerator.first + expect(statement.parameters.size).to eq(1) + expect(statement.parameters[0].value).to eq(value) + end + end + end end diff --git a/spec/unit/puppet-strings/yard/util_spec.rb b/spec/unit/puppet-strings/yard/util_spec.rb index 051644e35..a96b8e68f 100644 --- a/spec/unit/puppet-strings/yard/util_spec.rb +++ b/spec/unit/puppet-strings/yard/util_spec.rb @@ -47,4 +47,11 @@ expect(spec_subject.github_to_yard_links(str)).to eq(' module-description') end end + + describe 'ast_to_text' do + it 'converts a simple AST correctly' do + model = Puppet::Pops::Parser::Parser.new.parse_string('class test {}').model + expect(described_class.ast_to_text(model.body)).to eq('class test {}') + end + end end