From 6758afaefcde90809a7b2457c23c72fa487a2dd7 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Sun, 20 Jan 2019 10:58:46 +0800 Subject: [PATCH 1/3] (maint) Fix parsing at beginning of a document Previously the parser helper was silently failing to parse documents that had the cursor at line 0, char 0. This was due to the removal based methods trying to remove characters at the beginning of the document. This commit modifies the helper to ignore these parsing attempts if the cursor is at line 0, char 0. --- lib/puppet-languageserver/puppet_parser_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/puppet-languageserver/puppet_parser_helper.rb b/lib/puppet-languageserver/puppet_parser_helper.rb index f10e8981..e5faa519 100644 --- a/lib/puppet-languageserver/puppet_parser_helper.rb +++ b/lib/puppet-languageserver/puppet_parser_helper.rb @@ -72,9 +72,11 @@ def self.object_under_cursor(content, line_num, char_num, multiple_attempts = fa when :noop new_content = content when :remove_char + next if line_num.zero? && char_num.zero? new_content = remove_char_at(content, line_offsets, line_num, char_num) move_offset = -1 when :remove_word + next if line_num.zero? && char_num.zero? next_char = get_char_at(content, line_offsets, line_num, char_num) while /[[:word:]]/ =~ next_char From 538c21ce89763446d994e65aadd9e5cbd5d96360 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 18 Jan 2019 15:58:58 +0800 Subject: [PATCH 2/3] (GH-24) Allow parsing of manifests in tasks mode The Puppet language introduced Puppet Tasks and Plans as part of Puppet 5.4.0 [1]. A special parsing switch `--tasks` was introduced which changes the behaviour of the Puppet parser, that is switching it into "tasks" mode. Unfortunately this switch is global to all Puppet parsing therefore to dynamically switch it like we do in Editor Services, this required a global mutex to be used. Note that this commit does not add Puppet Tasks/Plans intellisense but merely allows the standard manifest providers (completion, validation etc.) to actually function instead of silently failing This commit; * Modifies the Puppet parser, via monkey patching, to implement a "singleton" parsing method that dynamically, and safely, switch the parsing mode on a per request basis * Modifies the Document Store to detect whether a File URI is a Puppet Task/Plan based on the relative path of the file versus the module root. * Modifies the various providers to call the parsing helper in the correct mode * Modifies the Message Router to detect whether a file is a Puppet Task/Plan and then sets the appropriate mode when calling the manifest providers (completion, validation etc.) [1] https://puppet.com/docs/bolt/1.x/writing_plans.html#concept-4485 --- lib/puppet-languageserver/document_store.rb | 16 ++ .../manifest/completion_provider.rb | 12 +- .../manifest/definition_provider.rb | 10 +- .../manifest/document_symbol_provider.rb | 35 +++- .../manifest/hover_provider.rb | 9 +- .../manifest/validation_provider.rb | 19 ++- lib/puppet-languageserver/message_router.rb | 9 +- .../puppet_monkey_patches.rb | 32 ++++ .../puppet_parser_helper.rb | 18 +- lib/puppet-languageserver/uri_helper.rb | 30 +++- lib/puppet-languageserver/validation_queue.rb | 8 +- .../document_store_spec.rb | 30 ++++ .../manifest/completion_provider_spec.rb | 12 ++ .../manifest/definition_provider_spec.rb | 12 ++ .../manifest/hover_provider_spec.rb | 12 ++ .../manifest/validation_provider_spec.rb | 12 ++ .../manifest/document_symbol_provider_spec.rb | 12 ++ .../message_router_spec.rb | 156 +++++++++++++++++- .../puppet-languageserver/uri_helper_spec.rb | 38 ++++- .../validation_queue_spec.rb | 6 +- 20 files changed, 445 insertions(+), 43 deletions(-) diff --git a/lib/puppet-languageserver/document_store.rb b/lib/puppet-languageserver/document_store.rb index cdd2e98a..59d29d4d 100644 --- a/lib/puppet-languageserver/document_store.rb +++ b/lib/puppet-languageserver/document_store.rb @@ -52,6 +52,15 @@ def self.document_type(uri) end end + # Plan files https://puppet.com/docs/bolt/1.x/writing_plans.html#concept-4485 + # exist in modules (requires metadata.json) and are in the `/plans` directory + def self.module_plan_file?(uri) + return false unless store_has_module_metadata? + relative_path = PuppetLanguageServer::UriHelper.relative_uri_path(PuppetLanguageServer::UriHelper.build_file_uri(store_root_path), uri, !windows?) + return false if relative_path.nil? + relative_path.start_with?('/plans/') + end + # Workspace management WORKSPACE_CACHE_TTL_SECONDS = 60 def self.initialize_store(options = {}) @@ -150,5 +159,12 @@ def self.dir_exist?(path) Dir.exist?(path) end private_class_method :dir_exist? + + def self.windows? + # Ruby only sets File::ALT_SEPARATOR on Windows and the Ruby standard + # library uses that to test what platform it's on. + !!File::ALT_SEPARATOR # rubocop:disable Style/DoubleNegation + end + private_class_method :windows? end end diff --git a/lib/puppet-languageserver/manifest/completion_provider.rb b/lib/puppet-languageserver/manifest/completion_provider.rb index ea36709b..8bcc9905 100644 --- a/lib/puppet-languageserver/manifest/completion_provider.rb +++ b/lib/puppet-languageserver/manifest/completion_provider.rb @@ -1,17 +1,23 @@ module PuppetLanguageServer module Manifest module CompletionProvider - def self.complete(content, line_num, char_num) + def self.complete(content, line_num, char_num, options = {}) + options = { + :tasks_mode => false + }.merge(options) items = [] incomplete = false - result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, true, [Puppet::Pops::Model::QualifiedName, Puppet::Pops::Model::BlockExpression]) - + result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, + :multiple_attempts => true, + :disallowed_classes => [Puppet::Pops::Model::QualifiedName, Puppet::Pops::Model::BlockExpression], + :tasks_mode => options[:tasks_mode]) if result.nil? # We are in the root of the document. # Add keywords keywords(%w[class define node application site]) { |x| items << x } + keywords(%w[plan]) { |x| items << x } if options[:tasks_mode] # Add resources all_resources { |x| items << x } diff --git a/lib/puppet-languageserver/manifest/definition_provider.rb b/lib/puppet-languageserver/manifest/definition_provider.rb index 781fdd7c..ca740fed 100644 --- a/lib/puppet-languageserver/manifest/definition_provider.rb +++ b/lib/puppet-languageserver/manifest/definition_provider.rb @@ -1,9 +1,13 @@ module PuppetLanguageServer module Manifest module DefinitionProvider - def self.find_definition(content, line_num, char_num) - result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, false, [Puppet::Pops::Model::BlockExpression]) - + def self.find_definition(content, line_num, char_num, options = {}) + options = { + :tasks_mode => false + }.merge(options) + result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, + :disallowed_classes => [Puppet::Pops::Model::BlockExpression], + :tasks_mode => options[:tasks_mode]) return nil if result.nil? path = result[:path] diff --git a/lib/puppet-languageserver/manifest/document_symbol_provider.rb b/lib/puppet-languageserver/manifest/document_symbol_provider.rb index 9889b9ef..eab38120 100644 --- a/lib/puppet-languageserver/manifest/document_symbol_provider.rb +++ b/lib/puppet-languageserver/manifest/document_symbol_provider.rb @@ -17,7 +17,7 @@ def self.workspace_symbols(query) 'fromline' => item.line, 'fromchar' => 0, # Don't have char pos for types 'toline' => item.line, - 'tochar' => 1024, # Don't have char pos for types + 'tochar' => 1024 # Don't have char pos for types ) ) @@ -30,7 +30,7 @@ def self.workspace_symbols(query) 'fromline' => item.line, 'fromchar' => 0, # Don't have char pos for functions 'toline' => item.line, - 'tochar' => 1024, # Don't have char pos for functions + 'tochar' => 1024 # Don't have char pos for functions ) ) @@ -43,7 +43,7 @@ def self.workspace_symbols(query) 'fromline' => item.line, 'fromchar' => 0, # Don't have char pos for classes 'toline' => item.line, - 'tochar' => 1024, # Don't have char pos for classes + 'tochar' => 1024 # Don't have char pos for classes ) ) end @@ -51,9 +51,12 @@ def self.workspace_symbols(query) result end - def self.extract_document_symbols(content) + def self.extract_document_symbols(content, options = {}) + options = { + :tasks_mode => false + }.merge(options) parser = Puppet::Pops::Parser::Parser.new - result = parser.parse_string(content, '') + result = parser.singleton_parse_string(content, options[:tasks_mode], '') if result.model.respond_to? :eAllContents # We are unable to build a document symbol tree for Puppet 4 AST @@ -187,6 +190,28 @@ def self.recurse_document_symbols(object, path, parentsymbol, symbollist) 'children' => [] ) + # Puppet Plan + when 'Puppet::Pops::Model::PlanDefinition' + this_symbol = LanguageServer::DocumentSymbol.create( + 'name' => object.name, + 'kind' => LanguageServer::SYMBOLKIND_CLASS, + 'detail' => object.name, + 'range' => create_range_array(object.offset, object.length, object.locator), + 'selectionRange' => create_range_array(object.offset, object.length, object.locator), + 'children' => [] + ) + # Load in the class parameters + object.parameters.each do |param| + param_symbol = LanguageServer::DocumentSymbol.create( + 'name' => '$' + param.name, + 'kind' => LanguageServer::SYMBOLKIND_PROPERTY, + 'detail' => '$' + param.name, + 'range' => create_range_array(param.offset, param.length, param.locator), + 'selectionRange' => create_range_array(param.offset, param.length, param.locator), + 'children' => [] + ) + this_symbol['children'].push(param_symbol) + end end object._pcore_contents do |item| diff --git a/lib/puppet-languageserver/manifest/hover_provider.rb b/lib/puppet-languageserver/manifest/hover_provider.rb index 37bd9cf1..d6d548a9 100644 --- a/lib/puppet-languageserver/manifest/hover_provider.rb +++ b/lib/puppet-languageserver/manifest/hover_provider.rb @@ -1,8 +1,13 @@ module PuppetLanguageServer module Manifest module HoverProvider - def self.resolve(content, line_num, char_num) - result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, false, [Puppet::Pops::Model::QualifiedName, Puppet::Pops::Model::BlockExpression]) + def self.resolve(content, line_num, char_num, options = {}) + options = { + :tasks_mode => false + }.merge(options) + result = PuppetLanguageServer::PuppetParserHelper.object_under_cursor(content, line_num, char_num, + :disallowed_classes => [Puppet::Pops::Model::QualifiedName, Puppet::Pops::Model::BlockExpression], + :tasks_mode => options[:tasks_mode]) return LanguageServer::Hover.create_nil_response if result.nil? path = result[:path] diff --git a/lib/puppet-languageserver/manifest/validation_provider.rb b/lib/puppet-languageserver/manifest/validation_provider.rb index 5f9aa559..7f84a05a 100644 --- a/lib/puppet-languageserver/manifest/validation_provider.rb +++ b/lib/puppet-languageserver/manifest/validation_provider.rb @@ -28,7 +28,12 @@ def self.fix_validate_errors(content) [problems_fixed, linter.manifest] end - def self.validate(content, _max_problems = 100) + def self.validate(content, options = {}) + options = { + :max_problems => 100, + :tasks_mode => false + }.merge(options) + result = [] # TODO: Need to implement max_problems problems = 0 @@ -89,8 +94,16 @@ def self.validate(content, _max_problems = 100) Puppet.override({ loaders: loaders }, 'For puppet parser validate') do begin validation_environment = env - validation_environment.check_for_reparse - validation_environment.known_resource_types.clear + $PuppetParserMutex.synchronize do # rubocop:disable Style/GlobalVars + begin + original_taskmode = Puppet[:tasks] if Puppet.tasks_supported? + Puppet[:tasks] = options[:tasks_mode] if Puppet.tasks_supported? + validation_environment.check_for_reparse + validation_environment.known_resource_types.clear + ensure + Puppet[:tasks] = original_taskmode if Puppet.tasks_supported? + end + end rescue StandardError => detail # Sometimes the error is in the cause not the root object itself detail = detail.cause if !detail.respond_to?(:line) && detail.respond_to?(:cause) diff --git a/lib/puppet-languageserver/message_router.rb b/lib/puppet-languageserver/message_router.rb index 5f2dee52..59a7db06 100644 --- a/lib/puppet-languageserver/message_router.rb +++ b/lib/puppet-languageserver/message_router.rb @@ -97,7 +97,7 @@ def receive_request(request) begin case documents.document_type(file_uri) when :manifest - request.reply_result(PuppetLanguageServer::Manifest::CompletionProvider.complete(content, line_num, char_num)) + request.reply_result(PuppetLanguageServer::Manifest::CompletionProvider.complete(content, line_num, char_num, :tasks_mode => PuppetLanguageServer::DocumentStore.module_plan_file?(file_uri))) else raise "Unable to provide completion on #{file_uri}" end @@ -123,7 +123,7 @@ def receive_request(request) begin case documents.document_type(file_uri) when :manifest - request.reply_result(PuppetLanguageServer::Manifest::HoverProvider.resolve(content, line_num, char_num)) + request.reply_result(PuppetLanguageServer::Manifest::HoverProvider.resolve(content, line_num, char_num, :tasks_mode => PuppetLanguageServer::DocumentStore.module_plan_file?(file_uri))) else raise "Unable to provide hover on #{file_uri}" end @@ -140,7 +140,7 @@ def receive_request(request) begin case documents.document_type(file_uri) when :manifest - request.reply_result(PuppetLanguageServer::Manifest::DefinitionProvider.find_definition(content, line_num, char_num)) + request.reply_result(PuppetLanguageServer::Manifest::DefinitionProvider.find_definition(content, line_num, char_num, :tasks_mode => PuppetLanguageServer::DocumentStore.module_plan_file?(file_uri))) else raise "Unable to provide definition on #{file_uri}" end @@ -155,8 +155,7 @@ def receive_request(request) begin case documents.document_type(file_uri) when :manifest - result = PuppetLanguageServer::Manifest::DocumentSymbolProvider.extract_document_symbols(content) - request.reply_result(result) + request.reply_result(PuppetLanguageServer::Manifest::DocumentSymbolProvider.extract_document_symbols(content, :tasks_mode => PuppetLanguageServer::DocumentStore.module_plan_file?(file_uri))) else raise "Unable to provide definition on #{file_uri}" end diff --git a/lib/puppet-languageserver/puppet_monkey_patches.rb b/lib/puppet-languageserver/puppet_monkey_patches.rb index 55048959..ce6b9a0f 100644 --- a/lib/puppet-languageserver/puppet_monkey_patches.rb +++ b/lib/puppet-languageserver/puppet_monkey_patches.rb @@ -1,3 +1,35 @@ +# Monkey Patch the Puppet language parser so we can globally lock any changes to the +# global setting Puppet[:tasks]. We need to manage this so we can switch between +# parsing modes. Unfortunately we can't do this as method parameter, only via the +# global Puppet settings which is not thread safe +$PuppetParserMutex = Mutex.new # rubocop:disable Style/GlobalVars +module Puppet + module Pops + module Parser + class Parser + def singleton_parse_string(code, task_mode = false, path = nil) + $PuppetParserMutex.synchronize do # rubocop:disable Style/GlobalVars + begin + original_taskmode = Puppet[:tasks] if Puppet.tasks_supported? + Puppet[:tasks] = task_mode if Puppet.tasks_supported? + return parse_string(code, path) + ensure + Puppet[:tasks] = original_taskmode if Puppet.tasks_supported? + end + end + end + end + end + end +end + +module Puppet + # Tasks first appeared in Puppet 5.4.0 + def self.tasks_supported? + Gem::Version.new(Puppet.version) >= Gem::Version.new('5.4.0') + end +end + # MUST BE LAST!!!!!! # Suppress any warning messages to STDOUT. It can pollute stdout when running in STDIO mode Puppet::Util::Log.newdesttype :null_logger do diff --git a/lib/puppet-languageserver/puppet_parser_helper.rb b/lib/puppet-languageserver/puppet_parser_helper.rb index e5faa519..36fc8023 100644 --- a/lib/puppet-languageserver/puppet_parser_helper.rb +++ b/lib/puppet-languageserver/puppet_parser_helper.rb @@ -56,13 +56,19 @@ def self.get_line_at(content, line_offsets, line_num) end end - def self.object_under_cursor(content, line_num, char_num, multiple_attempts = false, disallowed_classes = []) + def self.object_under_cursor(content, line_num, char_num, options) + options = { + :multiple_attempts => false, + :disallowed_classes => [], + :tasks_mode => false + }.merge(options) + # Use Puppet to generate the AST parser = Puppet::Pops::Parser::Parser.new # Calculating the line offsets can be expensive and is only required # if we're doing mulitple passes of parsing - line_offsets = line_offsets(content) if multiple_attempts + line_offsets = line_offsets(content) if options[:multiple_attempts] result = nil move_offset = 0 @@ -108,10 +114,10 @@ def self.object_under_cursor(content, line_num, char_num, multiple_attempts = fa next if new_content.nil? begin - result = parser.parse_string(new_content, '') + result = parser.singleton_parse_string(new_content, options[:tasks_mode], '') break rescue Puppet::ParseErrorWithIssue => _exception - next if multiple_attempts + next if options[:multiple_attempts] raise end end @@ -140,14 +146,14 @@ def self.object_under_cursor(content, line_num, char_num, multiple_attempts = fa valid_models = [] if result.model.respond_to? :eAllContents valid_models = result.model.eAllContents.select do |item| - check_for_valid_item(item, abs_offset, disallowed_classes) + check_for_valid_item(item, abs_offset, options[:disallowed_classes]) end valid_models.sort! { |a, b| a.length - b.length } else path = [] result.model._pcore_all_contents(path) do |item| - if check_for_valid_item(item, abs_offset, disallowed_classes) # rubocop:disable Style/IfUnlessModifier Nicer to read like this + if check_for_valid_item(item, abs_offset, options[:disallowed_classes]) # rubocop:disable Style/IfUnlessModifier Nicer to read like this valid_models.push(model_path_struct.new(item, path.dup)) end end diff --git a/lib/puppet-languageserver/uri_helper.rb b/lib/puppet-languageserver/uri_helper.rb index 58e804df..09f87977 100644 --- a/lib/puppet-languageserver/uri_helper.rb +++ b/lib/puppet-languageserver/uri_helper.rb @@ -1,7 +1,35 @@ +require 'uri' +require 'puppet' + module PuppetLanguageServer module UriHelper def self.build_file_uri(path) - path.start_with?('/') ? 'file://' + path : 'file:///' + path + 'file://' + Puppet::Util.uri_encode(path.start_with?('/') ? path : '/' + path) + end + + # Compares two URIs and returns the relative path + # + # @param root_uri [String] The root URI to compare to + # @param uri [String] The URI to compare to the root + # @param case_sensitive [Boolean] Whether the path comparison is case senstive or not. Default is true + # @return [String] Returns the relative path string if the URI is indeed a child of the root, otherwise returns nil + def self.relative_uri_path(root_uri, uri, case_sensitive = true) + actual_root = URI(root_uri) + actual_uri = URI(uri) + return nil unless actual_root.scheme == actual_uri.scheme + + # CGI.unescape doesn't handle space rules properly in uri paths + # URI.unescape does, but returns strings in their original encoding + # Mostly safe here as we're only worried about file based URIs + root_path = URI.unescape(actual_root.path) # rubocop:disable Lint/UriEscapeUnescape + uri_path = URI.unescape(actual_uri.path) # rubocop:disable Lint/UriEscapeUnescape + if case_sensitive + return nil unless uri_path.slice(0, root_path.length) == root_path + else + return nil unless uri_path.slice(0, root_path.length).casecmp(root_path).zero? + end + + uri_path.slice(root_path.length..-1) end end end diff --git a/lib/puppet-languageserver/validation_queue.rb b/lib/puppet-languageserver/validation_queue.rb index 72363cab..e1522db5 100644 --- a/lib/puppet-languageserver/validation_queue.rb +++ b/lib/puppet-languageserver/validation_queue.rb @@ -50,7 +50,7 @@ def self.validate_sync(file_uri, doc_version, connection_object) document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri) content = documents.document(file_uri, doc_version) return nil if content.nil? - result = validate(document_type, content) + result = validate(file_uri, document_type, content) # Send the response connection_object.reply_diagnostics(file_uri, result) @@ -78,11 +78,11 @@ def self.reset_queue(initial_state = []) end # Validate a document - def self.validate(document_type, content) + def self.validate(document_uri, document_type, content) # Perform validation case document_type when :manifest - PuppetLanguageServer::Manifest::ValidationProvider.validate(content) + PuppetLanguageServer::Manifest::ValidationProvider.validate(content, :tasks_mode => PuppetLanguageServer::DocumentStore.module_plan_file?(document_uri)) when :epp PuppetLanguageServer::Epp::ValidationProvider.validate(content) when :puppetfile @@ -117,7 +117,7 @@ def self.worker end # Perform validation - result = validate(document_type, content) + result = validate(file_uri, document_type, content) # Check if the document is still latest version current_version = documents.document_version(file_uri) diff --git a/spec/languageserver/integration/puppet-languageserver/document_store_spec.rb b/spec/languageserver/integration/puppet-languageserver/document_store_spec.rb index f0de596a..4033f43a 100644 --- a/spec/languageserver/integration/puppet-languageserver/document_store_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/document_store_spec.rb @@ -161,6 +161,36 @@ it_should_behave_like 'a metadata.json workspace', expected_root it_should_behave_like 'a cached workspace' it_should_behave_like 'a terminating file finder', 3, 1 + + ['/plans/test.pp', '/plans/a/b/c/something.pp'].each do |testcase| + it "should detect '#{testcase}' as a plan file" do + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path) + testcase + expect(subject.module_plan_file?(file_uri)).to be(true) + end + end + + ['/plan__s/test.pp', 'plans/something.txt', '/plantest.pp', ].each do |testcase| + it "should not detect '#{testcase}' as a plan file" do + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path) + testcase + expect(subject.module_plan_file?(file_uri)).to be(false) + end + end + + it 'should detect plan files as case insensitive on Windows' do + allow(subject).to receive(:windows?).and_return(true) + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path) + '/plans/test.pp' + expect(subject.module_plan_file?(file_uri)).to be(true) + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path.upcase) + '/plans/test.pp' + expect(subject.module_plan_file?(file_uri)).to be(true) + end + + it 'should detect plan files as case sensitive not on Windows' do + allow(subject).to receive(:windows?).and_return(false) + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path) + '/plans/test.pp' + expect(subject.module_plan_file?(file_uri)).to be(true) + file_uri = PuppetLanguageServer::UriHelper.build_file_uri(subject.store_root_path).upcase + '/plans/test.pp' + expect(subject.module_plan_file?(file_uri.upcase)).to be(false) + end end context 'given a workspace option which has a parent directory with metadata.json' do diff --git a/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb b/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb index 08fdbc08..c991029d 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb @@ -73,6 +73,18 @@ def create_ensurable_property end end + context 'Given a Puppet Plan', :if => Puppet.tasks_supported? do + let(:content) { <<-EOT + plan mymodule::my_plan( + ) { + } + EOT + } + it "should not raise an error" do + result = subject.complete(content, 0, 1, { :tasks_mode => true}) + end + end + context "Given a simple valid manifest" do let(:content) { <<-EOT class Alice { diff --git a/spec/languageserver/integration/puppet-languageserver/manifest/definition_provider_spec.rb b/spec/languageserver/integration/puppet-languageserver/manifest/definition_provider_spec.rb index 64a9c85c..18024a94 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/definition_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/definition_provider_spec.rb @@ -51,6 +51,18 @@ def puppetclass_cache_object(key, source) wait_for_puppet_loading end + context 'Given a Puppet Plan', :if => Puppet.tasks_supported? do + let(:content) { <<-EOT + plan mymodule::my_plan( + ) { + } + EOT + } + it "should not raise an error" do + result = subject.find_definition(content, 0, 1, { :tasks_mode => true}) + end + end + context 'When cursor is on a function name' do let(:content) { <<-EOT class Test::NoParams { diff --git a/spec/languageserver/integration/puppet-languageserver/manifest/hover_provider_spec.rb b/spec/languageserver/integration/puppet-languageserver/manifest/hover_provider_spec.rb index d8b00220..18c4d470 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/hover_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/hover_provider_spec.rb @@ -58,6 +58,18 @@ end end + context 'Given a Puppet Plan', :if => Puppet.tasks_supported? do + let(:content) { <<-EOT + plan mymodule::my_plan( + ) { + } + EOT + } + it "should not raise an error" do + result = subject.resolve(content, 0, 1, { :tasks_mode => true}) + end + end + describe 'when cursor is in the root of the document' do let(:line_num) { 5 } let(:char_num) { 3 } diff --git a/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb b/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb index b26e8a6d..7fa4d29e 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb @@ -109,6 +109,18 @@ end end + context 'Given a Puppet Plan', :if => Puppet.tasks_supported? do + let(:manifest) { <<-EOT + plan mymodule::my_plan( + ) { + } + EOT + } + it "should not raise an error" do + result = subject.validate(manifest, { :tasks_mode => true}) + end + end + describe "Given a complete manifest with no validation errors" do let(:manifest) { "user { 'Bob': ensure => 'present' }" } diff --git a/spec/languageserver/unit/puppet-languageserver/manifest/document_symbol_provider_spec.rb b/spec/languageserver/unit/puppet-languageserver/manifest/document_symbol_provider_spec.rb index 3bbf85a8..bbdf9848 100644 --- a/spec/languageserver/unit/puppet-languageserver/manifest/document_symbol_provider_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/manifest/document_symbol_provider_spec.rb @@ -43,6 +43,18 @@ class foo { context 'with Puppet 5.0 and above', :if => Gem::Version.new(Puppet.version) >= Gem::Version.new('5.0.0') do describe '#extract_document_symbols' do + context 'Given a Puppet Plan', :if => Puppet.tasks_supported? do + let(:content) { <<-EOT + plan mymodule::my_plan( + ) { + } + EOT + } + it "should not raise an error" do + result = subject.extract_document_symbols(content, { :tasks_mode => true}) + end + end + it 'should find a class in the document root' do content = "class foo {\n}" result = subject.extract_document_symbols(content) diff --git a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb index 817219fc..b09a0dc0 100644 --- a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb @@ -293,7 +293,7 @@ end end - # textDocument/completion - https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request + # textDocument/completion - https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#completion-request-leftwards_arrow_with_hook context 'given a textDocument/completion request' do let(:request_rpc_method) { 'textDocument/completion' } let(:line_num) { 1 } @@ -327,7 +327,14 @@ context 'for a puppet manifest file' do let(:file_uri) { MANIFEST_FILENAME } it 'should call complete method on the Completion Provider' do - expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object,line_num,char_num).and_return('something') + expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object,line_num,char_num,{:tasks_mode=>false}).and_return('something') + + subject.receive_request(request) + end + + it 'should set tasks_mode option if the file is Puppet plan file' do + expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object,line_num,char_num,{:tasks_mode=>true}).and_return('something') + allow(PuppetLanguageServer::DocumentStore).to receive(:module_plan_file?).and_return true subject.receive_request(request) end @@ -352,7 +359,7 @@ end end - # completionItem/resolve - https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request + # completionItem/resolve - https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#completion-item-resolve-request-leftwards_arrow_with_hook context 'given a completionItem/resolve request' do let(:request_rpc_method) { 'completionItem/resolve' } let(:request_params) {{ @@ -386,7 +393,7 @@ end end - # textDocument/hover - https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textDocument_hover + # textDocument/hover - https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#hover-request-leftwards_arrow_with_hook context 'given a textDocument/hover request' do let(:request_rpc_method) { 'textDocument/hover' } let(:line_num) { 1 } @@ -421,7 +428,14 @@ let(:file_uri) { MANIFEST_FILENAME } it 'should call resolve method on the Hover Provider' do - expect(PuppetLanguageServer::Manifest::HoverProvider).to receive(:resolve).with(Object,line_num,char_num).and_return('something') + expect(PuppetLanguageServer::Manifest::HoverProvider).to receive(:resolve).with(Object,line_num,char_num,{:tasks_mode=>false}).and_return('something') + + subject.receive_request(request) + end + + it 'should set tasks_mode option if the file is Puppet plan file' do + expect(PuppetLanguageServer::Manifest::HoverProvider).to receive(:resolve).with(Object,line_num,char_num,{:tasks_mode=>true}).and_return('something') + allow(PuppetLanguageServer::DocumentStore).to receive(:module_plan_file?).and_return true subject.receive_request(request) end @@ -446,6 +460,138 @@ end end + # textDocument/definition - https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#goto-definition-request-leftwards_arrow_with_hook + context 'given a textDocument/definition request' do + let(:request_rpc_method) { 'textDocument/definition' } + let(:line_num) { 1 } + let(:char_num) { 2 } + let(:request_params) {{ + 'textDocument' => { + 'uri' => file_uri + }, + 'position' => { + 'line' => line_num, + 'character' => char_num, + }, + }} + + context 'for a file the server does not understand' do + let(:file_uri) { UNKNOWN_FILENAME } + + it 'should log an error message' do + expect(PuppetLanguageServer).to receive(:log_message).with(:error,/Unable to provide definition/) + + subject.receive_request(request) + end + + it 'should reply with nil' do + expect(request).to receive(:reply_result).with(nil) + + subject.receive_request(request) + end + end + + context 'for a puppet manifest file' do + let(:file_uri) { MANIFEST_FILENAME } + + it 'should call find_definition method on the Definition Provider' do + expect(PuppetLanguageServer::Manifest::DefinitionProvider).to receive(:find_definition) + .with(Object,line_num,char_num,{:tasks_mode=>false}).and_return('something') + + subject.receive_request(request) + end + + it 'should set tasks_mode option if the file is Puppet plan file' do + expect(PuppetLanguageServer::Manifest::DefinitionProvider).to receive(:find_definition) + .with(Object,line_num,char_num,{:tasks_mode=>true}).and_return('something') + allow(PuppetLanguageServer::DocumentStore).to receive(:module_plan_file?).and_return true + + subject.receive_request(request) + end + + context 'and an error occurs during definition' do + before(:each) do + expect(PuppetLanguageServer::Manifest::DefinitionProvider).to receive(:find_definition).and_raise('MockError') + end + + it 'should log an error message' do + expect(PuppetLanguageServer).to receive(:log_message).with(:error,/MockError/) + + subject.receive_request(request) + end + + it 'should reply with nil' do + expect(request).to receive(:reply_result).with(nil) + + subject.receive_request(request) + end + end + end + end + + # textDocument/documentSymbol - https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#document-symbols-request-leftwards_arrow_with_hook + context 'given a textDocument/documentSymbol request' do + let(:request_rpc_method) { 'textDocument/documentSymbol' } + let(:request_params) {{ + 'textDocument' => { + 'uri' => file_uri + } + }} + + context 'for a file the server does not understand' do + let(:file_uri) { UNKNOWN_FILENAME } + + it 'should log an error message' do + expect(PuppetLanguageServer).to receive(:log_message).with(:error,/Unable to provide definition/) + + subject.receive_request(request) + end + + it 'should reply with nil' do + expect(request).to receive(:reply_result).with(nil) + + subject.receive_request(request) + end + end + + context 'for a puppet manifest file' do + let(:file_uri) { MANIFEST_FILENAME } + + it 'should call extract_document_symbols method on the Document Symbol Provider' do + expect(PuppetLanguageServer::Manifest::DocumentSymbolProvider).to receive(:extract_document_symbols) + .with(Object,{:tasks_mode=>false}).and_return('something') + + subject.receive_request(request) + end + + it 'should set tasks_mode option if the file is Puppet plan file' do + expect(PuppetLanguageServer::Manifest::DocumentSymbolProvider).to receive(:extract_document_symbols) + .with(Object,{:tasks_mode=>true}).and_return('something') + allow(PuppetLanguageServer::DocumentStore).to receive(:module_plan_file?).and_return true + + subject.receive_request(request) + end + + context 'and an error occurs during extraction' do + before(:each) do + expect(PuppetLanguageServer::Manifest::DocumentSymbolProvider).to receive(:extract_document_symbols).and_raise('MockError') + end + + it 'should log an error message' do + expect(PuppetLanguageServer).to receive(:log_message).with(:error,/MockError/) + + subject.receive_request(request) + end + + it 'should reply with nil' do + expect(request).to receive(:reply_result).with(nil) + + subject.receive_request(request) + end + end + end + end + context 'given an unknown request' do let(:request_rpc_method) { 'unknown_request_method' } diff --git a/spec/languageserver/unit/puppet-languageserver/uri_helper_spec.rb b/spec/languageserver/unit/puppet-languageserver/uri_helper_spec.rb index 47cb80a8..01115c97 100644 --- a/spec/languageserver/unit/puppet-languageserver/uri_helper_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/uri_helper_spec.rb @@ -1,14 +1,46 @@ require 'spec_helper' describe 'uri_helper' do + let(:subject) { PuppetLanguageServer::UriHelper } + describe '#build_file_uri' do it 'should return /// without leading slash' do - test = PuppetLanguageServer::UriHelper.build_file_uri('C:\foo.pp') - expect(test).to eq('file:///C:\foo.pp') + test = subject.build_file_uri('C:\foo.pp') + expect(test).to match('^file:///C') + end + it 'should return the uri escaped' do + test = subject.build_file_uri('C:\foo.pp') + expect(test).to eq('file:///C:%5Cfoo.pp') end it 'should return // with a leading slash' do - test = PuppetLanguageServer::UriHelper.build_file_uri('/opt/foo/foo.pp') + test = subject.build_file_uri('/opt/foo/foo.pp') expect(test).to eq('file:///opt/foo/foo.pp') end end + + describe '#relative_uri_path' do + it 'should return nil when the uri schemes differ' do + expect(subject.relative_uri_path('file:///somewhere', 'http:///somewhere')).to be_nil + end + + it 'should return nil if the uri is not a child of the root' do + expect(subject.relative_uri_path('file:///somewhere', 'file:///foo/bar')).to be_nil + end + + it 'should return nil if the uri is not a child of the root, when case sensitive' do + expect(subject.relative_uri_path('file:///Foo/', 'file:///foo/bar')).to be_nil + end + + it 'should unescape URIs when comparing' do + expect(subject.relative_uri_path('file:///%66%6F%6F/', 'file:///foo/b%61r')).to eq('bar') + end + + it 'should return the relative path if the uri is a child of the root' do + expect(subject.relative_uri_path('file:///foo/', 'file:///foo/bar')).to eq('bar') + end + + it 'should return the relative path if the uri is a child of the root and not case-sensitive' do + expect(subject.relative_uri_path('file:///Foo/', 'file:///foo/bar', false)).to eq('bar') + end + end end diff --git a/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb b/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb index c491e2e3..380bafa9 100644 --- a/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb @@ -72,7 +72,7 @@ ]) # We only expect the following results to be returned - expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(file_content2).and_return(validation_result) + expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(file_content2, Hash).and_return(validation_result) expect(PuppetLanguageServer::Epp::ValidationProvider).to receive(:validate).with(file_content1).and_return(validation_result) expect(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).with(file_content1).and_return(validation_result) expect(connection).to receive(:reply_diagnostics).with(MANIFEST_FILENAME, validation_result) @@ -93,7 +93,7 @@ validation_result = [{ 'result' => 'MockResult' }] before(:each) do - expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(FILE_CONTENT).and_return(validation_result) + expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(FILE_CONTENT, Hash).and_return(validation_result) end it_should_behave_like "single document which sends validation results", MANIFEST_FILENAME, FILE_CONTENT, validation_result @@ -155,7 +155,7 @@ validation_result = [{ 'result' => 'MockResult' }] before(:each) do - expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(FILE_CONTENT).and_return(validation_result) + expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(FILE_CONTENT, Hash).and_return(validation_result) end it_should_behave_like "document which sends validation results", MANIFEST_FILENAME, FILE_CONTENT, validation_result From 06c8b01f957c97e74351c913551b574d7c1339f8 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 25 Jan 2019 13:43:42 +0800 Subject: [PATCH 3/3] (maint) Update for rubocop 0.63.1 This commit updates two minor violations that appeared as part of robocop 0.63.1. --- lib/puppet-languageserver-sidecar/puppet_monkey_patches.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/puppet-languageserver-sidecar/puppet_monkey_patches.rb b/lib/puppet-languageserver-sidecar/puppet_monkey_patches.rb index 47967237..bf48e828 100644 --- a/lib/puppet-languageserver-sidecar/puppet_monkey_patches.rb +++ b/lib/puppet-languageserver-sidecar/puppet_monkey_patches.rb @@ -16,7 +16,7 @@ def newfunction(name, options = {}, &block) # Append the caller information result[:source_location] = { :source => caller.absolute_path, - :line => caller.lineno - 1, # Convert to a zero based line number system + :line => caller.lineno - 1 # Convert to a zero based line number system } monkey_append_function_info(name, result) @@ -69,7 +69,7 @@ def newtype(name, options = {}, &block) if block_given? && !block.source_location.nil? result._source_location = { :source => block.source_location[0], - :line => block.source_location[1] - 1, # Convert to a zero based line number system + :line => block.source_location[1] - 1 # Convert to a zero based line number system } end result