From 2ecbc0b2cb5f3ec922201d3f94e3b2d52b0c256d Mon Sep 17 00:00:00 2001 From: Hal Blackburn Date: Tue, 4 Jan 2022 10:42:17 +0000 Subject: [PATCH] (GH-320) Handle @param tags for non-existent params Classes and defined types with docstring @param tags for parameters 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. --- .../puppet_strings_helper.rb | 2 +- .../modules/defaultmodule/manifests/definedtype.pp | 1 + .../testfixtures/modules/defaultmodule/manifests/init.pp | 3 +++ .../puppet-languageserver-sidecar_spec.rb | 4 +++- .../puppet_strings_helper_spec.rb | 9 ++++++--- 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb index 736e0db2..fb967d20 100644 --- a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb +++ b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb @@ -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], diff --git a/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/definedtype.pp b/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/definedtype.pp index c27bdc81..11f95bd5 100644 --- a/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/definedtype.pp +++ b/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/definedtype.pp @@ -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' diff --git a/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/init.pp b/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/init.pp index 50ecfa26..5185aa2b 100644 --- a/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/init.pp +++ b/spec/languageserver-sidecar/fixtures/real_agent/environments/testfixtures/modules/defaultmodule/manifests/init.pp @@ -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', ) { } diff --git a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet-languageserver-sidecar_spec.rb b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet-languageserver-sidecar_spec.rb index c5d1c36d..c5f749ff 100644 --- a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet-languageserver-sidecar_spec.rb +++ b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet-languageserver-sidecar_spec.rb @@ -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 diff --git a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet_strings_helper_spec.rb b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet_strings_helper_spec.rb index 20ab8a0f..da0fd653 100644 --- a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet_strings_helper_spec.rb +++ b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet_strings_helper_spec.rb @@ -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 @@ -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)