From ad312644ca28a6a3d1fc449bb6d36a35da9c5620 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Thu, 16 May 2019 16:31:25 +0800 Subject: [PATCH 1/2] (GH-137) Load Puppet Custom Types via Puppet Strings Now that there is a puppetstrings feature flag, we can change the loading behaviour of the Custom Types to use the newer loader and Puppet Strings metadata evaluation. This commit; * Adds Custom Types ('type') ability to the retrieve_via_puppet_strings method * Custom Types loading isn't fully implemented like Puppet Functions are, in particular there's no use of the Smart Paths, and the loader still uses the old autoload methods. To work around this a new loader is created, which is based on the the NullLoader. It's purpose is to only support the needed discover_path method, however as it's a NullLoader, it won't actually load anything if it's called by Puppet. This loader calls the old autoloader using the same logic we used previously but surfaced via the loader. * Just like the previous loading mechanism, the whit and component types are ignored. * The Puppet Strings helper is updated to support reading the type data from Puppet Strings. Note that class documentation is now available whereas in the current iteration of the sidecar it is not. * The integration tests were updated to test for the new information and that the data can be cached correctly. --- .../puppet_helper_puppetstrings.rb | 117 +++--------------- .../puppet_monkey_patches_puppetstrings.rb | 51 ++++++++ .../puppet_strings_helper.rb | 42 ++++++- lib/puppet_languageserver_sidecar.rb | 17 ++- .../featureflag_puppetstrings_spec.rb | 28 +++-- 5 files changed, 141 insertions(+), 114 deletions(-) diff --git a/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb b/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb index a54ab690..92467a55 100644 --- a/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb +++ b/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb @@ -5,6 +5,7 @@ module PuppetLanguageServerSidecar module PuppetHelper SIDECAR_PUPPET_ENVIRONMENT = 'sidecarenvironment' + DISCOVERER_LOADER = 'path-discoverer-null-loader' def self.path_has_child?(path, child) # Doesn't matter what the child is, if the path is nil it's true. @@ -36,7 +37,7 @@ def self.get_puppet_resource(typename, title = nil) # Puppet Strings loading def self.available_documentation_types - [:function] + %I[function type] end # Retrieve objects via the Puppet 4 API loaders @@ -50,13 +51,18 @@ def self.retrieve_via_puppet_strings(cache, options = {}) return result if object_types.empty? result[:functions] = PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList.new if object_types.include?(:function) + result[:types] = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new if object_types.include?(:type) current_env = current_environment for_agent = options[:for_agent].nil? ? true : options[:for_agent] loaders = Puppet::Pops::Loaders.new(current_env, for_agent) + # Add any custom loaders + path_discoverer_loader = Puppet::Pops::Loader::PathDiscoveryNullLoader.new(nil, DISCOVERER_LOADER) + loaders.add_loader_by_name(path_discoverer_loader) paths = [] paths.concat(discover_type_paths(:function, loaders)) if object_types.include?(:function) + paths.concat(discover_type_paths(:type, loaders)) if object_types.include?(:type) paths.each do |path| next unless path_has_child?(options[:root_path], path) @@ -66,6 +72,11 @@ def self.retrieve_via_puppet_strings(cache, options = {}) if object_types.include?(:function) # rubocop:disable Style/IfUnlessModifier This reads better file_doc.functions.each { |item| result[:functions] << item } end + if object_types.include?(:type) + file_doc.types.each do |item| + result[:types] << item unless name == 'whit' || name == 'component' # rubocop:disable Style/MultipleComparison + end + end end # Remove Puppet3 functions which have a Puppet4 function already loaded @@ -79,7 +90,10 @@ def self.retrieve_via_puppet_strings(cache, options = {}) end def self.discover_type_paths(type, loaders) - loaders.private_environment_loader.discover_paths(type) + [].concat( + loaders.private_environment_loader.discover_paths(type), + loaders[DISCOVERER_LOADER].discover_paths(type) + ) end # Class and Defined Type loading @@ -113,39 +127,6 @@ def self.retrieve_classes(cache, options = {}) classes end - def self.retrieve_types(cache, options = {}) - PuppetLanguageServerSidecar.log_message(:debug, '[PuppetHelper::retrieve_types] Starting') - - # From https://github.com/puppetlabs/puppet/blob/ebd96213cab43bb2a8071b7ac0206c3ed0be8e58/lib/puppet/metatype/manager.rb#L182-L189 - autoloader = Puppet::Util::Autoload.new(self, 'puppet/type') - current_env = current_environment - types = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new - - # This is an expensive call - if autoloader.method(:files_to_load).arity.zero? - params = [] - else - params = [current_env] - end - autoloader.files_to_load(*params).each do |file| - name = file.gsub(autoloader.path + '/', '') - begin - expanded_name = autoloader.expand(name) - absolute_name = Puppet::Util::Autoload.get_file(expanded_name, current_env) - raise("Could not find absolute path of type #{name}") if absolute_name.nil? - if path_has_child?(options[:root_path], absolute_name) # rubocop:disable Style/IfUnlessModifier Nicer to read like this - types.concat(load_type_file(cache, name, absolute_name, autoloader, current_env)) - end - rescue StandardError => e - PuppetLanguageServerSidecar.log_message(:error, "[PuppetHelper::retrieve_types] Error loading type #{file}: #{e} #{e.backtrace}") - end - end - - PuppetLanguageServerSidecar.log_message(:debug, "[PuppetHelper::retrieve_types] Finished loading #{types.count} type/s") - - types - end - # Private functions def self.prune_resource_parameters(resources) @@ -248,71 +229,5 @@ def self.load_classes_from_manifest(cache, manifest_file) class_info end private_class_method :load_classes_from_manifest - - def self.load_type_file(cache, name, absolute_name, autoloader, env) - types = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new - if cache.active? - cached_result = cache.load(absolute_name, PuppetLanguageServerSidecar::Cache::TYPES_SECTION) - unless cached_result.nil? - begin - types.from_json!(cached_result) - return types - rescue StandardError => e - PuppetLanguageServerSidecar.log_message(:warn, "[PuppetHelper::load_type_file] Error while deserializing #{absolute_name} from cache: #{e}") - types = PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList.new - end - end - end - - # Get the list of currently loaded types - loaded_types = [] - # Due to PUP-8301, if no types have been loaded yet then Puppet::Type.eachtype - # will throw instead of not yielding. - begin - Puppet::Type.eachtype { |item| loaded_types << item.name } - rescue NoMethodError => e - # Detect PUP-8301 - if e.respond_to?(:receiver) - raise unless e.name == :each && e.receiver.nil? - else - raise unless e.name == :each && e.message =~ /nil:NilClass/ - end - end - - unless autoloader.loaded?(name) - # This is an expensive call - unless autoloader.load(name, env) # rubocop:disable Style/IfUnlessModifier Nicer to read like this - PuppetLanguageServerSidecar.log_message(:error, "[PuppetHelper::load_type_file] type #{absolute_name} did not load") - end - end - - # Find the types that were loaded - # Due to PUP-8301, if no types have been loaded yet then Puppet::Type.eachtype - # will throw instead of not yielding. - begin - Puppet::Type.eachtype do |item| - next if loaded_types.include?(item.name) - # Ignore the internal only Puppet Types - next if item.name == :component || item.name == :whit - obj = PuppetLanguageServerSidecar::Protocol::PuppetType.from_puppet(item.name, item) - # TODO: Need to use calling_source in the cache backing store - # Perhaps I should be incrementally adding items to the cache instead of batch mode? - obj.calling_source = absolute_name - types << obj - end - rescue NoMethodError => e - # Detect PUP-8301 - if e.respond_to?(:receiver) - raise unless e.name == :each && e.receiver.nil? - else - raise unless e.name == :each && e.message =~ /nil:NilClass/ - end - end - PuppetLanguageServerSidecar.log_message(:warn, "[PuppetHelper::load_type_file] type #{absolute_name} did not load any types") if types.empty? - cache.save(absolute_name, PuppetLanguageServerSidecar::Cache::TYPES_SECTION, types.to_json) if cache.active? - - types - end - private_class_method :load_type_file end end diff --git a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb index 4b092a17..233656f4 100644 --- a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb +++ b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb @@ -69,6 +69,57 @@ def discover_paths(type, name_authority = Pcore::RUNTIME_NAME_AUTHORITY) end end +# While this is not a monkey patch, but a new class, this class is used purely to +# enumerate the paths of puppet "things" that aren't already covered as part of the +# usual loaders. It is implemented as a null loader as it can't actually _load_ +# anything. +module Puppet + module Pops + module Loader + class PathDiscoveryNullLoader < Puppet::Pops::Loader::NullLoader + def discover_paths(type, name_authority = Pcore::RUNTIME_NAME_AUTHORITY) + result = [] + + if type == :type + autoloader = Puppet::Util::Autoload.new(self, 'puppet/type') + current_env = current_environment + + # This is an expensive call + if autoloader.method(:files_to_load).arity.zero? + params = [] + else + params = [current_env] + end + autoloader.files_to_load(*params).each do |file| + name = file.gsub(autoloader.path + '/', '') + expanded_name = autoloader.expand(name) + absolute_name = Puppet::Util::Autoload.get_file(expanded_name, current_env) + result << absolute_name unless absolute_name.nil? + end + end + + result.concat(super) + result.uniq + end + + private + + def current_environment + begin + env = Puppet.lookup(:environments).get!(Puppet.settings[:environment]) + return env unless env.nil? + rescue Puppet::Environments::EnvironmentNotFound + PuppetLanguageServerSidecar.log_message(:warning, "[Puppet::Pops::Loader::PathDiscoveryNullLoader::current_environment] Unable to load environment #{Puppet.settings[:environment]}") + rescue StandardError => e + PuppetLanguageServerSidecar.log_message(:warning, "[Puppet::Pops::Loader::PathDiscoveryNullLoader::current_environment] Error loading environment #{Puppet.settings[:environment]}: #{e}") + end + Puppet.lookup(:current_environment) + end + end + end + end +end + module Puppet module Pops module Loader diff --git a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb index 79b699dd..f7832ad3 100644 --- a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb +++ b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb @@ -54,6 +54,8 @@ def self.require_puppet_strings require 'puppet-strings' require 'puppet-strings/yard' require 'puppet-strings/json' + + require File.expand_path(File.join(File.dirname(__FILE__), 'puppet_strings_monkey_patches')) @puppet_strings_loaded = true rescue LoadError => e PuppetLanguageServerSidecar.log_message(:error, "[PuppetStringsHelper::require_puppet_strings] Unable to load puppet-strings gem: #{e}") @@ -90,6 +92,7 @@ def populate_from_yard_registry! # Extract all of the information # Ref - https://github.com/puppetlabs/puppet-strings/blob/87a8e10f45bfeb7b6b8e766324bfb126de59f791/lib/puppet-strings/json.rb#L10-L16 populate_functions_from_yard_registry! + populate_types_from_yard_registry! end def populate_from_sidecar_cache!(path, cache) @@ -149,6 +152,38 @@ def populate_functions_from_yard_registry! @cache[source_path].functions << obj end end + + def populate_types_from_yard_registry! + ::YARD::Registry.all(:puppet_type).map(&:to_hash).each do |item| + source_path = item[:file] + type_name = item[:name].to_s + @cache[source_path] = FileDocumentation.new(source_path) if @cache[source_path].nil? + + obj = PuppetLanguageServer::Sidecar::Protocol::PuppetType.new + obj.key = type_name + obj.source = item[:file] + obj.calling_source = obj.source + obj.line = item[:line] + obj.doc = item[:docstring][:text] + + obj.attributes = {} + item[:properties]&.each do |prop| + obj.attributes[prop[:name]] = { + :type => :property, + :doc => prop[:description] + } + end + item[:parameters]&.each do |prop| + obj.attributes[prop[:name]] = { + :type => :param, + :doc => prop[:description], + :isnamevar? => prop[:isnamevar] + } + end + + @cache[source_path].types << obj + end + end end class FileDocumentation @@ -158,16 +193,21 @@ class FileDocumentation # PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList object holding all functions attr_accessor :functions + # PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList object holding all types + attr_accessor :types + def initialize(path = nil) @path = path @functions = PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList.new + @types = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new end # Serialisation def to_h { 'path' => path, - 'functions' => functions + 'functions' => functions, + 'types' => types } end diff --git a/lib/puppet_languageserver_sidecar.rb b/lib/puppet_languageserver_sidecar.rb index 5bcbfdd8..3156005f 100644 --- a/lib/puppet_languageserver_sidecar.rb +++ b/lib/puppet_languageserver_sidecar.rb @@ -272,7 +272,11 @@ def self.execute(options) when 'default_types' cache = options[:disable_cache] ? PuppetLanguageServerSidecar::Cache::Null.new : PuppetLanguageServerSidecar::Cache::FileSystem.new - PuppetLanguageServerSidecar::PuppetHelper.retrieve_types(cache) + if use_puppet_strings + PuppetLanguageServerSidecar::PuppetHelper.retrieve_via_puppet_strings(cache, :object_types => [:type])[:types] + else + PuppetLanguageServerSidecar::PuppetHelper.retrieve_types(cache) + end when 'node_graph' inject_workspace_as_module || inject_workspace_as_environment @@ -322,8 +326,15 @@ def self.execute(options) when 'workspace_types' null_cache = PuppetLanguageServerSidecar::Cache::Null.new return nil unless inject_workspace_as_module || inject_workspace_as_environment - PuppetLanguageServerSidecar::PuppetHelper.retrieve_types(null_cache, - :root_path => PuppetLanguageServerSidecar::Workspace.root_path) + if use_puppet_strings + cache = options[:disable_cache] ? PuppetLanguageServerSidecar::Cache::Null.new : PuppetLanguageServerSidecar::Cache::FileSystem.new + PuppetLanguageServerSidecar::PuppetHelper.retrieve_via_puppet_strings(cache, + :object_types => [:type], + :root_path => PuppetLanguageServerSidecar::Workspace.root_path)[:types] + else + PuppetLanguageServerSidecar::PuppetHelper.retrieve_types(null_cache) + end + else log_message(:error, "Unknown action #{options[:action]}. Expected one of #{ACTION_LIST}") end diff --git a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb index a5cecbaa..9483064f 100644 --- a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb +++ b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb @@ -145,7 +145,9 @@ def expect_same_array_content(a, b) describe 'when running default_types action' do let (:cmd_options) { ['--action', 'default_types'] } - it 'should return a deserializable type list with default types' do + it 'should return a cachable deserializable type list with default types' do + expect_empty_cache + result = run_sidecar(cmd_options) deserial = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new() expect { deserial.from_json!(result) }.to_not raise_error @@ -160,6 +162,16 @@ def expect_same_array_content(a, b) # These are defined in the fixtures/real_agent/cache/lib/puppet/type expect(deserial).to contain_child_with_key(:default_type) + + # Now run using cached information + expect_populated_cache + + result2 = run_sidecar(cmd_options) + deserial2 = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new() + expect { deserial2.from_json!(result2) }.to_not raise_error + + # The second result should be the same as the first + expect_same_array_content(deserial, deserial2) end end @@ -259,11 +271,10 @@ def expect_same_array_content(a, b) expect(obj.attributes.key?(:name)).to be true expect(obj.attributes.key?(:when)).to be true expect(obj.attributes[:name][:type]).to eq(:param) - expect(obj.attributes[:name][:doc]).to eq("name_parameter\n\n") - expect(obj.attributes[:name][:required?]).to be true + expect(obj.attributes[:name][:doc]).to eq("name_parameter") + expect(obj.attributes[:name][:isnamevar?]).to be true expect(obj.attributes[:when][:type]).to eq(:property) - expect(obj.attributes[:when][:doc]).to eq("when_property\n\n") - expect(obj.attributes[:when][:required?]).to be_nil + expect(obj.attributes[:when][:doc]).to eq("when_property") end end end @@ -366,11 +377,10 @@ def expect_same_array_content(a, b) expect(obj.attributes.key?(:name)).to be true expect(obj.attributes.key?(:when)).to be true expect(obj.attributes[:name][:type]).to eq(:param) - expect(obj.attributes[:name][:doc]).to eq("name_env_parameter\n\n") - expect(obj.attributes[:name][:required?]).to be true + expect(obj.attributes[:name][:doc]).to eq("name_env_parameter") + expect(obj.attributes[:name][:isnamevar?]).to be true expect(obj.attributes[:when][:type]).to eq(:property) - expect(obj.attributes[:when][:doc]).to eq("when_env_property\n\n") - expect(obj.attributes[:when][:required?]).to be_nil + expect(obj.attributes[:when][:doc]).to eq("when_env_property") end end end From c0c7431b7d8a41c18fb4265002e76cae2f6cec0b Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Thu, 16 May 2019 16:31:45 +0800 Subject: [PATCH 2/2] (GH-137) Load Puppet Classes and Defined Types via Puppet Strings Now that there is a puppetstrings feature flag, we can change the loading behaviour of Puppet Classes and Defined Types to use the newer loader and Puppet Strings metadata evaluation. This commit; * Uses a "fake" object type called sidecar_manifest to interrogate the loaders for the information. This is because there's no type name in the loaders which suits our needs. By using such a strict name we can be sure that the default loaders will just ignore it. * The PathDiscoveryNullLoader is updated to discover all of the paths to the manifests in the environment. * The Puppet Strings helper is updated to support reading the type data from Puppet Strings. Note that class documentation is now available whereas in the current iteration of the sidecar it is not. * The integration tests were updated to test for the new information and that the data can be cached correctly. --- .../puppet_helper_puppetstrings.rb | 118 ++---------------- .../puppet_monkey_patches_puppetstrings.rb | 6 + .../puppet_strings_helper.rb | 35 ++++++ lib/puppet_languageserver_sidecar.rb | 17 ++- .../defaultmodule/manifests/definedtype.pp | 8 +- .../modules/defaultmodule/manifests/init.pp | 12 +- .../featureflag_puppetstrings_spec.rb | 35 +++++- 7 files changed, 114 insertions(+), 117 deletions(-) diff --git a/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb b/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb index 92467a55..62881139 100644 --- a/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb +++ b/lib/puppet-languageserver-sidecar/puppet_helper_puppetstrings.rb @@ -37,7 +37,7 @@ def self.get_puppet_resource(typename, title = nil) # Puppet Strings loading def self.available_documentation_types - %I[function type] + %I[class function type] end # Retrieve objects via the Puppet 4 API loaders @@ -50,6 +50,7 @@ def self.retrieve_via_puppet_strings(cache, options = {}) result = {} return result if object_types.empty? + result[:classes] = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new if object_types.include?(:class) result[:functions] = PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList.new if object_types.include?(:function) result[:types] = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new if object_types.include?(:type) @@ -61,6 +62,9 @@ def self.retrieve_via_puppet_strings(cache, options = {}) loaders.add_loader_by_name(path_discoverer_loader) paths = [] + # :sidecar_manifest isn't technically a Loadable thing. This is useful because we know that any type + # of loader will just ignore it. + paths.concat(discover_type_paths(:sidecar_manifest, loaders)) if object_types.include?(:class) paths.concat(discover_type_paths(:function, loaders)) if object_types.include?(:function) paths.concat(discover_type_paths(:type, loaders)) if object_types.include?(:type) @@ -69,6 +73,9 @@ def self.retrieve_via_puppet_strings(cache, options = {}) file_doc = PuppetLanguageServerSidecar::PuppetStringsHelper.file_documentation(path, cache) next if file_doc.nil? + if object_types.include?(:class) # rubocop:disable Style/IfUnlessModifier This reads better + file_doc.classes.each { |item| result[:classes] << item } + end if object_types.include?(:function) # rubocop:disable Style/IfUnlessModifier This reads better file_doc.functions.each { |item| result[:functions] << item } end @@ -96,37 +103,6 @@ def self.discover_type_paths(type, loaders) ) end - # Class and Defined Type loading - def self.retrieve_classes(cache, options = {}) - PuppetLanguageServerSidecar.log_message(:debug, '[PuppetHelper::retrieve_classes] Starting') - - # TODO: Can probably do this better, but this works. - current_env = current_environment - module_path_list = current_env - .modules - .select { |mod| Dir.exist?(File.join(mod.path, 'manifests')) } - .map { |mod| mod.path } - manifest_path_list = module_path_list.map { |mod_path| File.join(mod_path, 'manifests') } - PuppetLanguageServerSidecar.log_message(:debug, "[PuppetHelper::retrieve_classes] Loading classes from #{module_path_list}") - - # Find and parse all manifests in the manifest paths - classes = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new - manifest_path_list.each do |manifest_path| - Dir.glob("#{manifest_path}/**/*.pp").each do |manifest_file| - begin - if path_has_child?(options[:root_path], manifest_file) # rubocop:disable Style/IfUnlessModifier Nicer to read like this - classes.concat(load_classes_from_manifest(cache, manifest_file)) - end - rescue StandardError => e - PuppetLanguageServerSidecar.log_message(:error, "[PuppetHelper::retrieve_classes] Error loading manifest #{manifest_file}: #{e} #{e.backtrace}") - end - end - end - - PuppetLanguageServerSidecar.log_message(:debug, "[PuppetHelper::retrieve_classes] Finished loading #{classes.count} classes") - classes - end - # Private functions def self.prune_resource_parameters(resources) @@ -151,83 +127,5 @@ def self.current_environment Puppet.lookup(:current_environment) end private_class_method :current_environment - - def self.load_classes_from_manifest(cache, manifest_file) - class_info = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new - - if cache.active? - cached_result = cache.load(manifest_file, PuppetLanguageServerSidecar::Cache::CLASSES_SECTION) - unless cached_result.nil? - begin - class_info.from_json!(cached_result) - return class_info - rescue StandardError => e - PuppetLanguageServerSidecar.log_message(:warn, "[PuppetHelper::load_classes_from_manifest] Error while deserializing #{manifest_file} from cache: #{e}") - class_info = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new - end - end - end - - file_content = File.open(manifest_file, 'r:UTF-8') { |f| f.read } - - parser = Puppet::Pops::Parser::Parser.new - result = nil - begin - result = parser.parse_string(file_content, '') - rescue Puppet::ParseErrorWithIssue - # Any parsing errors means we can't inspect the document - return class_info - end - - # Enumerate the entire AST looking for classes and defined types - # TODO: Need to learn how to read the help/docs for hover support - if result.model.respond_to? :eAllContents - # Puppet 4 AST - result.model.eAllContents.select do |item| - puppet_class = {} - case item.class.to_s - when 'Puppet::Pops::Model::HostClassDefinition' - puppet_class['type'] = :class - when 'Puppet::Pops::Model::ResourceTypeDefinition' - puppet_class['type'] = :typedefinition - else - next - end - puppet_class['name'] = item.name - puppet_class['doc'] = nil - puppet_class['parameters'] = item.parameters - puppet_class['source'] = manifest_file - puppet_class['line'] = result.locator.line_for_offset(item.offset) - 1 - puppet_class['char'] = result.locator.offset_on_line(item.offset) - - obj = PuppetLanguageServerSidecar::Protocol::PuppetClass.from_puppet(item.name, puppet_class, result.locator) - class_info << obj - end - else - result.model._pcore_all_contents([]) do |item| - puppet_class = {} - case item.class.to_s - when 'Puppet::Pops::Model::HostClassDefinition' - puppet_class['type'] = :class - when 'Puppet::Pops::Model::ResourceTypeDefinition' - puppet_class['type'] = :typedefinition - else - next - end - puppet_class['name'] = item.name - puppet_class['doc'] = nil - puppet_class['parameters'] = item.parameters - puppet_class['source'] = manifest_file - puppet_class['line'] = item.line - puppet_class['char'] = item.pos - obj = PuppetLanguageServerSidecar::Protocol::PuppetClass.from_puppet(item.name, puppet_class, item.locator) - class_info << obj - end - end - cache.save(manifest_file, PuppetLanguageServerSidecar::Cache::CLASSES_SECTION, class_info.to_json) if cache.active? - - class_info - end - private_class_method :load_classes_from_manifest end end diff --git a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb index 233656f4..e1a2b0ff 100644 --- a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb +++ b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb @@ -98,6 +98,12 @@ def discover_paths(type, name_authority = Pcore::RUNTIME_NAME_AUTHORITY) end end + if type == :sidecar_manifest + current_environment.modules.each do |mod| + result.concat(mod.all_manifests) + end + end + result.concat(super) result.uniq end diff --git a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb index f7832ad3..bcbba8fc 100644 --- a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb +++ b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb @@ -91,6 +91,7 @@ def document(path) def populate_from_yard_registry! # Extract all of the information # Ref - https://github.com/puppetlabs/puppet-strings/blob/87a8e10f45bfeb7b6b8e766324bfb126de59f791/lib/puppet-strings/json.rb#L10-L16 + populate_classes_from_yard_registry! populate_functions_from_yard_registry! populate_types_from_yard_registry! end @@ -113,6 +114,35 @@ def save_to_sidecar_cache(path, cache) private + def populate_classes_from_yard_registry! + %I[puppet_class puppet_defined_type].each do |yard_type| + YARD::Registry.all(yard_type).map(&:to_hash).each do |item| + source_path = item[:file] + class_name = item[:name].to_s + @cache[source_path] = FileDocumentation.new(source_path) if @cache[source_path].nil? + + obj = PuppetLanguageServer::Sidecar::Protocol::PuppetClass.new + obj.key = class_name + obj.source = item[:file] + obj.calling_source = obj.source + obj.line = item[:line] + + obj.doc = item[:docstring][:text] + obj.parameters = {} + # Extract the class parameters + item[:docstring][:tags]&.select { |tag| tag[:tag_name] == 'param' }&.each do |tag| + param_name = tag[:name] + obj.parameters[param_name] = { + :doc => tag[:text], + :type => tag[:types].join(', ') + } + end + + @cache[source_path].classes << obj + end + end + end + def populate_functions_from_yard_registry! ::YARD::Registry.all(:puppet_function).map(&:to_hash).each do |item| source_path = item[:file] @@ -190,6 +220,9 @@ class FileDocumentation # The path to file that has been documented attr_accessor :path + # PuppetLanguageServer::Sidecar::Protocol::PuppetClassList object holding all classes + attr_accessor :classes + # PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList object holding all functions attr_accessor :functions @@ -198,6 +231,7 @@ class FileDocumentation def initialize(path = nil) @path = path + @classes = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new @functions = PuppetLanguageServer::Sidecar::Protocol::PuppetFunctionList.new @types = PuppetLanguageServer::Sidecar::Protocol::PuppetTypeList.new end @@ -206,6 +240,7 @@ def initialize(path = nil) def to_h { 'path' => path, + 'classes' => classes, 'functions' => functions, 'types' => types } diff --git a/lib/puppet_languageserver_sidecar.rb b/lib/puppet_languageserver_sidecar.rb index 3156005f..227faf17 100644 --- a/lib/puppet_languageserver_sidecar.rb +++ b/lib/puppet_languageserver_sidecar.rb @@ -260,7 +260,11 @@ def self.execute(options) when 'default_classes' cache = options[:disable_cache] ? PuppetLanguageServerSidecar::Cache::Null.new : PuppetLanguageServerSidecar::Cache::FileSystem.new - PuppetLanguageServerSidecar::PuppetHelper.retrieve_classes(cache) + if use_puppet_strings + PuppetLanguageServerSidecar::PuppetHelper.retrieve_via_puppet_strings(cache, :object_types => [:class])[:classes] + else + PuppetLanguageServerSidecar::PuppetHelper.retrieve_classes(cache) + end when 'default_functions' cache = options[:disable_cache] ? PuppetLanguageServerSidecar::Cache::Null.new : PuppetLanguageServerSidecar::Cache::FileSystem.new @@ -306,8 +310,15 @@ def self.execute(options) when 'workspace_classes' null_cache = PuppetLanguageServerSidecar::Cache::Null.new return nil unless inject_workspace_as_module || inject_workspace_as_environment - PuppetLanguageServerSidecar::PuppetHelper.retrieve_classes(null_cache, - :root_path => PuppetLanguageServerSidecar::Workspace.root_path) + if use_puppet_strings + cache = options[:disable_cache] ? PuppetLanguageServerSidecar::Cache::Null.new : PuppetLanguageServerSidecar::Cache::FileSystem.new + PuppetLanguageServerSidecar::PuppetHelper.retrieve_via_puppet_strings(cache, + :object_types => [:class], + :root_path => PuppetLanguageServerSidecar::Workspace.root_path)[:classes] + else + PuppetLanguageServerSidecar::PuppetHelper.retrieve_classes(null_cache, + :root_path => PuppetLanguageServerSidecar::Workspace.root_path) + end when 'workspace_functions' null_cache = PuppetLanguageServerSidecar::Cache::Null.new 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 3d5dadeb..c27bdc81 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 @@ -1,4 +1,10 @@ +# This is an example of how to document a defined type. +# This typename should be fully qualified. +# +# @param ensure Ensure parameter documentation. +# @param param2 param2 documentation. define defaultdefinedtype( - $ensure = 'UNSET', + $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 fa4644d5..50ecfa26 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 @@ -1,2 +1,12 @@ -class defaultmodule () { +# This is an example of how to document a Puppet class +# +# @example Declaring the class +# include example_class +# +# @param first The first parameter for this class. +# @param second The second parameter for this class. +class defaultmodule ( + String $first = 'firstparam', + Integer $second = 2, +) { } diff --git a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb index 9483064f..e413a67f 100644 --- a/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb +++ b/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/featureflag_puppetstrings_spec.rb @@ -91,7 +91,9 @@ def expect_same_array_content(a, b) describe 'when running default_classes action' do let (:cmd_options) { ['--action', 'default_classes'] } - it 'should return a deserializable class list with default classes' do + it 'should return a cachable deserializable class list with default classes' do + expect_empty_cache + result = run_sidecar(cmd_options) deserial = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new() expect { deserial.from_json!(result) }.to_not raise_error @@ -102,6 +104,35 @@ def expect_same_array_content(a, b) # These are defined in the fixtures/real_agent/environments/testfixtures/modules/defaultmodule expect(deserial).to contain_child_with_key(:defaultdefinedtype) expect(deserial).to contain_child_with_key(:defaultmodule) + + # Make sure the classes have the right properties + obj = child_with_key(deserial, :defaultdefinedtype) + expect(obj.doc).to match(/This is an example of how to document a defined type/) + expect(obj.source).to match(/defaultmodule/) + expect(obj.parameters.count).to eq 2 + expect(obj.parameters['ensure'][:type]).to eq 'Any' + expect(obj.parameters['ensure'][:doc]).to match(/Ensure parameter documentation/) + expect(obj.parameters['param2'][:type]).to eq 'String' + expect(obj.parameters['param2'][:doc]).to match(/param2 documentation/) + + 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['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/) + + # Now run using cached information + expect_populated_cache + + result2 = run_sidecar(cmd_options) + deserial2 = PuppetLanguageServer::Sidecar::Protocol::PuppetClassList.new() + expect { deserial2.from_json!(result2) }.to_not raise_error + + # The second result should be the same as the first + expect_same_array_content(deserial, deserial2) end end @@ -319,7 +350,7 @@ def expect_same_array_content(a, b) # Make sure the class has the right properties obj = child_with_key(deserial, :envdeftype) - expect(obj.doc).to be_nil # We don't yet get documentation for classes or defined types + expect(obj.doc).to_not be_nil expect(obj.parameters['ensure']).to_not be_nil expect(obj.parameters['ensure'][:type]).to eq('String') expect(obj.source).to match(/valid_environment_workspace/)