Skip to content

Commit

Permalink
(puppetlabsGH-320) Handle @param tags for non-existent params
Browse files Browse the repository at this point in the history
Classes and defined types with docstring @param tags for parameters
that are not actually present in the parameter list cause the language
server to crash, due to populate_classes_from_yard_registry assuming
param tags always have a :types array present.

@param tags without types are now ignored. Note that class/defined type
parameters without a specified type automatically have the "Any" type
in their :types array, so they're not excluded by this change.
  • Loading branch information
h4l committed Jan 4, 2022
1 parent 9c1f530 commit f95b2c7
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/puppet-languageserver-sidecar/puppet_strings_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def populate_classes_from_yard_registry!
obj.parameters = {}
# Extract the class parameters
unless item[:docstring][:tags].nil?
item[:docstring][:tags].select { |tag| tag[:tag_name] == 'param' }.each do |tag|
item[:docstring][:tags].select { |tag| tag[:tag_name] == 'param' && tag.key?(:types) }.each do |tag|
param_name = tag[:name]
obj.parameters[param_name] = {
:doc => tag[:text],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#
# @param ensure Ensure parameter documentation.
# @param param2 param2 documentation.
# @param missingparam This param does not exist.
define defaultdefinedtype(
$ensure = 'UNSET',
String $param2 = 'param2default'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
#
# @param first The first parameter for this class.
# @param second The second parameter for this class.
# @param notype This parameter does not specify a type.
# @param missingparam This parameter does not exist.
class defaultmodule (
String $first = 'firstparam',
Integer $second = 2,
$notype = 'three',
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,13 @@ def expect_same_array_content(a, b)
obj = child_with_key(deserial, :defaultmodule)
expect(obj.doc).to match(/This is an example of how to document a Puppet class/)
expect(obj.source).to match(/defaultmodule/)
expect(obj.parameters.count).to eq 2
expect(obj.parameters.count).to eq 3
expect(obj.parameters['first'][:type]).to eq 'String'
expect(obj.parameters['first'][:doc]).to match(/The first parameter for this class/)
expect(obj.parameters['second'][:type]).to eq 'Integer'
expect(obj.parameters['second'][:doc]).to match(/The second parameter for this class/)
expect(obj.parameters['notype'][:type]).to eq 'Any'
expect(obj.parameters['notype'][:doc]).to match(/This parameter does not specify a type/)

# Now run using cached information
expect_populated_cache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,23 @@

# Check base methods
expect(item.key).to eq('defaultmodule')
expect(item.line).to eq(8)
expect(item.line).to eq(10)
expect(item.char).to be_nil
expect(item.length).to be_nil
expect(item.source).to eq(fixture_filepath)
# Check class specific methods
expect(item.doc).to match(/This is an example of how to document a Puppet class/)
# Check the class parameters
expect(item.parameters.count).to eq(2)
expect(item.parameters.count).to eq(3)
param = item.parameters['first']
expect(param[:doc]).to eq('The first parameter for this class.')
expect(param[:type]).to eq('String')
param = item.parameters['second']
expect(param[:doc]).to eq('The second parameter for this class.')
expect(param[:type]).to eq('Integer')
param = item.parameters['notype']
expect(param[:doc]).to eq('This parameter does not specify a type.')
expect(param[:type]).to eq('Any')
end
end

Expand All @@ -119,7 +122,7 @@

# Check base methods
expect(item.key).to eq('defaultdefinedtype')
expect(item.line).to eq(6)
expect(item.line).to eq(7)
expect(item.char).to be_nil
expect(item.length).to be_nil
expect(item.source).to eq(fixture_filepath)
Expand Down

0 comments on commit f95b2c7

Please sign in to comment.