From 64d5d1eddfb52e2e84ca37e89becc4f22fa7f147 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 27 Nov 2019 15:41:01 +0800 Subject: [PATCH 1/2] (GH-139) Provide completions for defined types Previously the manifest completion provider would not provide results if the cursor was in a defined type. This commit adds the ResourceTypeDefinition much like the HostClassDefinition. --- .../manifest/completion_provider.rb | 4 +-- .../manifest/completion_provider_spec.rb | 31 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/puppet-languageserver/manifest/completion_provider.rb b/lib/puppet-languageserver/manifest/completion_provider.rb index 7d930bbe..2d74cba7 100644 --- a/lib/puppet-languageserver/manifest/completion_provider.rb +++ b/lib/puppet-languageserver/manifest/completion_provider.rb @@ -41,8 +41,8 @@ def self.complete(content, line_num, char_num, options = {}) # Complete for `$facts[...` all_facts { |x| items << x } if expr == 'facts' - when 'Puppet::Pops::Model::HostClassDefinition' - # We are in the root of a `class` statement + when 'Puppet::Pops::Model::HostClassDefinition', 'Puppet::Pops::Model::ResourceTypeDefinition' + # We are in the root of a `class` or `define` statement # Add keywords keywords(%w[require contain]) { |x| items << x } 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 44aa19f7..efb2134d 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb @@ -131,6 +131,11 @@ class Alice { ensure => 'present', name => 'name', } + +define delta ( +) { + +} EOT } @@ -153,20 +158,24 @@ class Alice { end end - describe "When inside the root of a class" do - let(:line_num) { 1 } - let(:char_num) { 0 } - let(:expected_types) { ['keyword','resource_type','resource_class'] } + [ + { :name => 'class', :line_num => 1 }, + { :name => 'defined type', :line_num => 18 }, + ].each do |testcase| + describe "When inside the root of a #{testcase[:name]}" do + let(:char_num) { 0 } + let(:expected_types) { ['keyword','resource_type','resource_class'] } - it 'should return a list of keyword, resource_type, resource_class' do - result = subject.complete(content, line_num, char_num) + it 'should return a list of keyword, resource_type, resource_class' do + result = subject.complete(content, testcase[:line_num], char_num) - result.items.each do |item| - expect(item).to be_completion_item_with_type(expected_types) - end + result.items.each do |item| + expect(item).to be_completion_item_with_type(expected_types) + end - expected_types.each do |typename| - expect(number_of_completion_item_with_type(result,typename)).to be > 0 + expected_types.each do |typename| + expect(number_of_completion_item_with_type(result,typename)).to be > 0 + end end end end From 13b8645f1050bedef3539f21d4b9aff3fc543e25 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 27 Nov 2019 15:57:39 +0800 Subject: [PATCH 2/2] (GH-139) Add more intelligence to completion request Previously the completion provider blindly moved the cursor one place back which worked fine if completions were triggered by a character instead of manual. Now that the LSP has provisions for the context of the completion request we can intelligently figure out when to remove the trigger character. This commit updates the mesasge handler to pass through the CompletionContext object and the completion provider can then determine how to use the context accordingly. This commit also updates the tests for this behaviour. --- .../manifest/completion_provider.rb | 11 +++++---- lib/puppet-languageserver/message_handler.rb | 3 ++- .../manifest/completion_provider_spec.rb | 7 +++--- .../message_handler_spec.rb | 24 +++++++++++++++++-- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/lib/puppet-languageserver/manifest/completion_provider.rb b/lib/puppet-languageserver/manifest/completion_provider.rb index 2d74cba7..5082d116 100644 --- a/lib/puppet-languageserver/manifest/completion_provider.rb +++ b/lib/puppet-languageserver/manifest/completion_provider.rb @@ -5,15 +5,18 @@ module Manifest module CompletionProvider def self.complete(content, line_num, char_num, options = {}) options = { - :tasks_mode => false + :tasks_mode => false, + :context => nil # LSP::CompletionContext object }.merge(options) items = [] incomplete = false + is_trigger_char = !options[:context].nil? && options[:context].triggerKind == LSP::CompletionTriggerKind::TRIGGERCHARACTER 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]) + :multiple_attempts => true, + :disallowed_classes => [Puppet::Pops::Model::QualifiedName, Puppet::Pops::Model::BlockExpression], + :tasks_mode => options[:tasks_mode], + :remove_trigger_char => is_trigger_char) if result.nil? # We are in the root of the document. diff --git a/lib/puppet-languageserver/message_handler.rb b/lib/puppet-languageserver/message_handler.rb index cc7c750a..99ee15b3 100644 --- a/lib/puppet-languageserver/message_handler.rb +++ b/lib/puppet-languageserver/message_handler.rb @@ -105,10 +105,11 @@ def request_textdocument_completion(_, json_rpc_message) line_num = json_rpc_message.params['position']['line'] char_num = json_rpc_message.params['position']['character'] content = documents.document(file_uri) + context = json_rpc_message.params['context'].nil? ? nil : LSP::CompletionContext.new(json_rpc_message.params['context']) case documents.document_type(file_uri) when :manifest - return PuppetLanguageServer::Manifest::CompletionProvider.complete(content, line_num, char_num, :tasks_mode => PuppetLanguageServer::DocumentStore.plan_file?(file_uri)) + return PuppetLanguageServer::Manifest::CompletionProvider.complete(content, line_num, char_num, :context => context, :tasks_mode => PuppetLanguageServer::DocumentStore.plan_file?(file_uri)) else raise "Unable to provide completion on #{file_uri}" end 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 efb2134d..41650002 100644 --- a/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/manifest/completion_provider_spec.rb @@ -116,6 +116,7 @@ def create_ensurable_property context "Given a simple valid manifest" do let(:content) { <<-EOT + class Alice { user { 'Bob': @@ -143,7 +144,7 @@ class Alice { let(:char_num) { 0 } let(:expected_types) { ['keyword','resource_type','function','resource_class'] } - [0, 8].each do |line_num| + [0, 9].each do |line_num| it "should return a list of keyword, resource_type, function, resource_class regardless of cursor location (Testing line #{line_num})" do result = subject.complete(content, line_num, char_num) @@ -160,7 +161,7 @@ class Alice { [ { :name => 'class', :line_num => 1 }, - { :name => 'defined type', :line_num => 18 }, + { :name => 'defined type', :line_num => 19 }, ].each do |testcase| describe "When inside the root of a #{testcase[:name]}" do let(:char_num) { 0 } @@ -181,7 +182,7 @@ class Alice { end describe "When inside the root of a resource" do - let(:line_num) { 11 } + let(:line_num) { 12 } let(:char_num) { 0 } let(:expected_types) { ['resource_parameter','resource_property'] } diff --git a/spec/languageserver/unit/puppet-languageserver/message_handler_spec.rb b/spec/languageserver/unit/puppet-languageserver/message_handler_spec.rb index 4f4646ce..48760164 100644 --- a/spec/languageserver/unit/puppet-languageserver/message_handler_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/message_handler_spec.rb @@ -488,16 +488,36 @@ 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,{:tasks_mode=>false}).and_return('something') + expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object, line_num, char_num, { :tasks_mode => false, :context => nil }).and_return('something') subject.request_textdocument_completion(connection_id, request_message) 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') + expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object, line_num, char_num, { :tasks_mode => true, :context => nil }).and_return('something') allow(PuppetLanguageServer::DocumentStore).to receive(:plan_file?).and_return true subject.request_textdocument_completion(connection_id, request_message) end + context 'with a completion context' do + let(:request_params) {{ + 'textDocument' => { + 'uri' => file_uri + }, + 'position' => { + 'line' => line_num, + 'character' => char_num, + }, + 'context' => { + 'triggerKind' => 1 + } + }} + + it 'should pass the context' do + expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).with(Object, line_num, char_num, { :tasks_mode => false, :context => LSP::CompletionContext}).and_return('something') + subject.request_textdocument_completion(connection_id, request_message) + end + end + context 'and an error occurs during completion' do before(:each) do expect(PuppetLanguageServer::Manifest::CompletionProvider).to receive(:complete).and_raise('MockError')