From db97c1aab2cb07b747bfa926c4b4ef4789ee3575 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Tue, 17 Sep 2019 17:53:10 +0800 Subject: [PATCH] (GH-177) Add registrations and settings for on type formatting Now that the Language Client can process client settings, and dynamic registrations it is now possible to setup the server to register to receive the onTypeFormatting request. This commit: * Responds to the 'puppet.editorService.formatOnType.enable' client setting * Dynamically registers the provider if dynamic registration is possible otherwise uses static registration * Adds a format_on_type method on the Language Client to track whether this feature is enabled * Adds dummy response to the `textDocument/onTypeFormatting` request * Adds tests for the client interacting with the settings --- lib/puppet-languageserver/language_client.rb | 34 ++++- lib/puppet-languageserver/message_router.rb | 9 +- .../server_capabilities.rb | 12 +- .../language_client_spec.rb | 122 +++++++++++++++++- .../message_router_spec.rb | 84 +++++++++++- 5 files changed, 249 insertions(+), 12 deletions(-) diff --git a/lib/puppet-languageserver/language_client.rb b/lib/puppet-languageserver/language_client.rb index 22351292..8d8329bb 100644 --- a/lib/puppet-languageserver/language_client.rb +++ b/lib/puppet-languageserver/language_client.rb @@ -4,6 +4,9 @@ module PuppetLanguageServer class LanguageClient attr_reader :message_router + # Client settings + attr_reader :format_on_type + def initialize(message_router) @message_router = message_router @client_capabilites = {} @@ -17,6 +20,9 @@ def initialize(message_router) # } # ] @registrations = {} + + # Default settings + @format_on_type = false end def client_capability(*names) @@ -35,12 +41,20 @@ def parse_lsp_initialize!(initialize_params = {}) @client_capabilites = initialize_params['capabilities'] end - # Settings could be a hash or an array of hash - def parse_lsp_configuration_settings!(settings = [{}]) - # TODO: Future use. Actually do something with the settings - # settings = [settings] unless settings.is_a?(Hash) - # settings.each do |hash| - # end + def parse_lsp_configuration_settings!(settings = {}) + # format on type + value = safe_hash_traverse(settings, 'puppet', 'editorService', 'formatOnType', 'enable') + unless value.nil? || to_boolean(value) == @format_on_type # rubocop:disable Style/GuardClause Ummm no. + # Is dynamic registration available? + if client_capability('textDocument', 'onTypeFormatting', 'dynamicRegistration') == true + if value + register_capability('textDocument/onTypeFormatting', PuppetLanguageServer::ServerCapabilites.document_on_type_formatting_options) + else + unregister_capability('textDocument/onTypeFormatting') + end + end + @format_on_type = value + end end def capability_registrations(method) @@ -148,12 +162,18 @@ def parse_unregister_capability_response!(response, original_request) private + def to_boolean(value) + return false if value.nil? || value == false + return true if value == true + value.to_s =~ %r{^(true|t|yes|y|1)$/i} + end + def new_request_id SecureRandom.uuid end def safe_hash_traverse(hash, *names) - return nil if names.empty? + return nil if names.empty? || hash.nil? || hash.empty? item = nil loop do name = names.shift diff --git a/lib/puppet-languageserver/message_router.rb b/lib/puppet-languageserver/message_router.rb index a53ec309..d72e5dc2 100644 --- a/lib/puppet-languageserver/message_router.rb +++ b/lib/puppet-languageserver/message_router.rb @@ -46,7 +46,11 @@ def receive_request(request) when 'initialize' PuppetLanguageServer.log_message(:debug, 'Received initialize method') client.parse_lsp_initialize!(request.params) - request.reply_result('capabilities' => PuppetLanguageServer::ServerCapabilites.capabilities) + # Setup static registrations if dynamic registration is not available + info = { + :documentOnTypeFormattingProvider => !client.client_capability('textDocument', 'onTypeFormatting', 'dynamicRegistration') + } + request.reply_result('capabilities' => PuppetLanguageServer::ServerCapabilites.capabilities(info)) when 'shutdown' PuppetLanguageServer.log_message(:debug, 'Received shutdown method') @@ -201,6 +205,9 @@ def receive_request(request) request.reply_result(nil) end + when 'textDocument/onTypeFormatting' + request.reply_result(nil) + when 'textDocument/signatureHelp' file_uri = request.params['textDocument']['uri'] line_num = request.params['position']['line'] diff --git a/lib/puppet-languageserver/server_capabilities.rb b/lib/puppet-languageserver/server_capabilities.rb index 3ada953d..f42e937e 100644 --- a/lib/puppet-languageserver/server_capabilities.rb +++ b/lib/puppet-languageserver/server_capabilities.rb @@ -2,10 +2,10 @@ module PuppetLanguageServer module ServerCapabilites - def self.capabilities + def self.capabilities(options = {}) # https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#initialize-request - { + value = { 'textDocumentSync' => LSP::TextDocumentSyncKind::FULL, 'hoverProvider' => true, 'completionProvider' => { @@ -19,6 +19,14 @@ def self.capabilities 'triggerCharacters' => ['(', ','] } } + value['documentOnTypeFormattingProvider'] = document_on_type_formatting_options if options[:documentOnTypeFormattingProvider] + value + end + + def self.document_on_type_formatting_options + { + 'firstTriggerCharacter' => '>' + } end def self.no_capabilities diff --git a/spec/languageserver/unit/puppet-languageserver/language_client_spec.rb b/spec/languageserver/unit/puppet-languageserver/language_client_spec.rb index 22a48eca..0057a2ba 100644 --- a/spec/languageserver/unit/puppet-languageserver/language_client_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/language_client_spec.rb @@ -1,5 +1,101 @@ require 'spec_helper' +def pretty_value(value) + value.nil? ? 'nil' : value.to_s +end + +# Requires +# :settings : A hashtable of the inbound settings +# :setting_value : The value that will be set to +RSpec.shared_examples "a client setting" do |method_name| + [ + { :from => false, :setting => nil, :expected_setting => false }, + { :from => false, :setting => false, :expected_setting => false }, + { :from => false, :setting => true, :expected_setting => true }, + { :from => true, :setting => nil, :expected_setting => true }, + { :from => true, :setting => false, :expected_setting => false }, + { :from => true, :setting => true, :expected_setting => true }, + ].each do |testcase| + context "When it transitions from #{pretty_value(testcase[:from])} with a setting value of #{pretty_value(testcase[:setting])}" do + let(:setting_value) { testcase[:setting] } + + before(:each) do + subject.instance_variable_set("@#{method_name}".intern, testcase[:from]) + end + + it "should have a cached value to #{testcase[:expected_setting]}" do + expect(subject.send(method_name)).to eq(testcase[:from]) + + subject.parse_lsp_configuration_settings!(settings) + expect(subject.send(method_name)).to eq(testcase[:expected_setting]) + end + end + end +end + +# Requires +# :settings : A hashtable of the inbound settings +# :setting_value : The value that will be set to +RSpec.shared_examples "a setting with dynamic registrations" do |method_name, dynamic_reg, registration_method| + [ + { :from => false, :setting => nil, :noop => true }, + { :from => false, :setting => false, :noop => true }, + { :from => false, :setting => true, :register => true }, + { :from => true, :setting => nil, :noop => true }, + { :from => true, :setting => false, :unregister => true }, + { :from => true, :setting => true, :noop => true }, + ].each do |testcase| + context "When it transitions from #{pretty_value(testcase[:from])} with a setting value of #{pretty_value(testcase[:setting])}" do + let(:setting_value) { testcase[:setting] } + + before(:each) do + subject.instance_variable_set("@#{method_name}".intern, testcase[:from]) + end + + it 'should not call any capabilities', :if => testcase[:noop] do + expect(subject).to receive(:client_capability).exactly(0).times + expect(subject).to receive(:register_capability).exactly(0).times + expect(subject).to receive(:unregister_capability).exactly(0).times + + subject.parse_lsp_configuration_settings!(settings) + end + + context "when dynamic registration is not supported", :unless => testcase[:noop] do + before(:each) do + expect(subject).to receive(:client_capability).with(*dynamic_reg).and_return(false) + end + + it 'should not call any registration or unregistrations' do + expect(subject).to receive(:register_capability).exactly(0).times + expect(subject).to receive(:unregister_capability).exactly(0).times + + subject.parse_lsp_configuration_settings!(settings) + end + end + + context "when dynamic registration is supported", :unless => testcase[:noop] do + before(:each) do + expect(subject).to receive(:client_capability).with(*dynamic_reg).and_return(true) + end + + it "should register #{registration_method}", :if => testcase[:register] do + expect(subject).to receive(:register_capability).with(registration_method, Object) + expect(subject).to receive(:unregister_capability).exactly(0).times + + subject.parse_lsp_configuration_settings!(settings) + end + + it "should unregister #{registration_method}", :if => testcase[:unregister] do + expect(subject).to receive(:unregister_capability).with(registration_method) + expect(subject).to receive(:register_capability).exactly(0).times + + subject.parse_lsp_configuration_settings!(settings) + end + end + end + end +end + describe 'PuppetLanguageServer::LanguageClient' do let(:json_rpc_handler) { MockJSONRPCHandler.new } let(:message_router) { MockMessageRouter.new.tap { |i| i.json_rpc_handler = json_rpc_handler } } @@ -146,6 +242,12 @@ allow(PuppetLanguageServer).to receive(:log_message) end + describe '#format_on_type' do + it 'should be false by default' do + expect(subject.format_on_type).to eq(false) + end + end + describe '#client_capability' do before(:each) do subject.parse_lsp_initialize!(initialize_params) @@ -181,7 +283,25 @@ # end describe '#parse_lsp_configuration_settings!' do - # TODO: Future use. + describe 'puppet.editorService.formatOnType.enable' do + let(:settings) do + { 'puppet' => { + 'editorService' => { + 'formatOnType' => { + 'enable' => setting_value + } + } + } + } + end + + it_behaves_like 'a client setting', :format_on_type + + it_behaves_like 'a setting with dynamic registrations', + :format_on_type, + ['textDocument', 'onTypeFormatting', 'dynamicRegistration'], + 'textDocument/onTypeFormatting' + end end describe '#capability_registrations' do diff --git a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb index 406303f9..090039b1 100644 --- a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb @@ -18,6 +18,12 @@ match { |actual| !actual.send(method_name).nil? } end + RSpec::Matchers.define :server_capability do |name| + match do |actual| + actual['capabilities'] && actual['capabilities'][name] + end + end + describe '#documents' do it 'should respond to documents method' do expect(subject).to respond_to(:documents) @@ -67,7 +73,7 @@ # initialize - https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#initialize context 'given an initialize request' do let(:request_rpc_method) { 'initialize' } - let(:request_params) { { 'cap1' => 'value1' } } + let(:request_params) { { 'capabilities' => { 'cap1' => 'value1' } } } it 'should reply with capabilites' do expect(request).to receive(:reply_result).with(hash_including('capabilities')) @@ -79,6 +85,55 @@ subject.receive_request(request) end + + context 'when onTypeFormatting does support dynamic registration' do + let(:request_params) do + { 'capabilities' => { + 'textDocument' => { + 'onTypeFormatting' => { + 'dynamicRegistration' => true + } + } + } + } + end + + it 'should statically register a documentOnTypeFormattingProvider' do + expect(request).to_not receive(:reply_result).with(server_capability('documentOnTypeFormattingProvider')) + allow(request).to receive(:reply_result) + + subject.receive_request(request) + end + end + + context 'when onTypeFormatting does not support dynamic registration' do + let(:request_params) do + { 'capabilities' => { + 'textDocument' => { + 'onTypeFormatting' => { + 'dynamicRegistration' => false + } + } + } + } + end + + it 'should statically register a documentOnTypeFormattingProvider' do + expect(request).to receive(:reply_result).with(server_capability('documentOnTypeFormattingProvider')) + + subject.receive_request(request) + end + end + + context 'when onTypeFormatting does not specify dynamic registration' do + let(:request_params) { {} } + + it 'should statically register a documentOnTypeFormattingProvider' do + expect(request).to receive(:reply_result).with(server_capability('documentOnTypeFormattingProvider')) + + subject.receive_request(request) + end + end end # shutdown - https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#shutdown @@ -800,6 +855,33 @@ end end + # textDocument/onTypeFormatting - https://microsoft.github.io/language-server-protocol/specification#textDocument_onTypeFormatting + context 'given a textDocument/onTypeFormatting request' do + let(:request_rpc_method) { 'textDocument/onTypeFormatting' } + let(:file_uri) { MANIFEST_FILENAME } + let(:line_num) { 1 } + let(:char_num) { 6 } + let(:trigger_char) { '>' } + let(:formatting_options) { { 'tabSize' => 2, 'insertSpaces' => true} } + let(:request_params) { { + 'textDocument' => { + 'uri' => file_uri + }, + 'position' => { + 'line' => line_num, + 'character' => char_num, + }, + 'ch' => trigger_char, + 'options' => formatting_options + } } + + it 'should not log an error message' do + expect(PuppetLanguageServer).to_not receive(:log_message).with(:error,"Unknown RPC method #{request_rpc_method}") + + subject.receive_request(request) + end + end + context 'given an unknown request' do let(:request_rpc_method) { 'unknown_request_method' }