Merge pull request #186 from glennsarti/align-hash-rockets

(GH-177) Add auto-align hash rocket feature

lib/puppet-languageserver/manifest/format_on_type_provider.rb

@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+require 'puppet-lint'
+
+module PuppetLanguageServer
+  module Manifest
+    class FormatOnTypeProvider
+      class << self
+        def instance
+          @instance ||= new
+        end
+      end
+
+      def format(content, line, char, trigger_character, formatting_options)
+        result = []
+        # Abort if the user has pressed something other than `>`
+        return result unless trigger_character == '>'
+        # Abort if the formatting is tab based. Can't do that yet
+        return result unless formatting_options['insertSpaces'] == true
+        # Abort if content is too big
+        return result if content.length > 4096
+
+        lexer = PuppetLint::Lexer.new
+        tokens = lexer.tokenise(content)
+
+        # Find where in the manifest the cursor is
+        cursor_token = find_token_by_location(tokens, line, char)
+        return result if cursor_token.nil?
+        # The cursor should be at the end of a hashrocket, otherwise exit
+        return result unless cursor_token.type == :FARROW
+
+        # Find the start of the hash with respect to the cursor
+        start_brace = cursor_token.prev_token_of(:LBRACE, skip_blocks: true)
+        # Find the end  of the hash with respect to the cursor
+        end_brace = cursor_token.next_token_of(:RBRACE, skip_blocks: true)
+
+        # The line count between the start and end brace needs to be at least 2 lines. Otherwise there's nothing to align to
+        return result if end_brace.nil? || start_brace.nil? || end_brace.line - start_brace.line <= 2
+
+        # Find all hashrockets '=>' between the hash braces, ignoring nested hashes
+        farrows = []
+        farrow_token = start_brace
+        lines = []
+        loop do
+          farrow_token = farrow_token.next_token_of(:FARROW, skip_blocks: true)
+          # if there are no more hashrockets, or we've gone past the end_brace, we can exit the loop
+          break if farrow_token.nil? || farrow_token.line > end_brace.line
+          # if there's a hashrocket AFTER the closing brace (why?) then we can also exit the loop
+          break if farrow_token.line == end_brace.line && farrow_token.character > end_brace.character
+          # Check for multiple hashrockets on the same line. If we find some, then we can't do any automated indentation
+          return result if lines.include?(farrow_token.line)
+          lines << farrow_token.line
+          farrows << { token: farrow_token }
+        end
+
+        # Now we have a list of farrows, time for figure out the indentation marks
+        farrows.each do |item|
+          item.merge!(calculate_indentation_info(item[:token]))
+        end
+
+        # Now we have the list of indentations we can find the biggest
+        max_indent = -1
+        farrows.each do |info|
+          max_indent = info[:indent] if info[:indent] > max_indent
+        end
+        # No valid indentations found
+        return result if max_indent == -1
+
+        # Now we have the indent size, generate all of the required TextEdits
+        farrows.each do |info|
+          # Ignore invalid hashrockets
+          next if info[:indent] == -1
+          end_name_token = info[:name_token].column + info[:name_token].to_manifest.length
+          begin_farrow_token = info[:token].column
+          new_whitespace = max_indent - end_name_token
+          # If the whitespace is already what we want, then ignore it.
+          next if begin_farrow_token - end_name_token == new_whitespace
+
+          # Create the TextEdit
+          result << LSP::TextEdit.new.from_h!(
+            'newText' => ' ' * new_whitespace,
+            'range'   => LSP.create_range(info[:token].line - 1, end_name_token - 1, info[:token].line - 1, begin_farrow_token - 1)
+          )
+        end
+        result
+      end
+
+      private
+
+      VALID_TOKEN_TYPES = %i[NAME STRING SSTRING].freeze
+
+      def find_token_by_location(tokens, line, character)
+        return nil if tokens.empty?
+        # Puppet Lint uses base 1, but LSP is base 0, so adjust accordingly
+        cursor_line = line + 1
+        cursor_column = character + 1
+        idx = -1
+        while idx < tokens.count
+          idx += 1
+          # if the token is on previous lines keep looking...
+          next if tokens[idx].line < cursor_line
+          # return nil if we skipped over the line we need
+          return nil if tokens[idx].line > cursor_line
+          # return nil if we skipped over the character position we need
+          return nil if tokens[idx].column > cursor_column
+          # return the token if it starts on the cursor column we are interested in
+          return tokens[idx] if tokens[idx].column == cursor_column
+          end_column = tokens[idx].column + tokens[idx].to_manifest.length
+          # return the token it the cursor column is within the token string
+          return tokens[idx] if cursor_column <= end_column
+          # otherwise, keep on searching
+        end
+        nil
+      end
+
+      def calculate_indentation_info(farrow_token)
+        result = { indent: -1 }
+        # This is not a valid hashrocket if there's no previous tokens
+        return result if farrow_token.prev_token.nil?
+        if VALID_TOKEN_TYPES.include?(farrow_token.prev_token.type)
+          # Someone forgot the whitespace! e.g. ensure=>
+          result[:indent] = farrow_token.column + 1
+          result[:name_token] = farrow_token.prev_token
+          return result
+        end
+        if farrow_token.prev_token.type == :WHITESPACE
+          # If the whitespace has no previous token (which shouldn't happen) or the thing before the whitespace is not a property name this it not a valid hashrocket
+          return result if farrow_token.prev_token.prev_token.nil?
+          return result unless VALID_TOKEN_TYPES.include?(farrow_token.prev_token.prev_token.type)
+          result[:name_token] = farrow_token.prev_token.prev_token
+          result[:indent] = farrow_token.prev_token.column + 1 # The indent is the whitespace column + 1
+        end
+        result
+      end
+    end
+  end
+end

lib/puppet-languageserver/message_router.rb

@@ -206,7 +206,31 @@ def receive_request(request)
         end
 
       when 'textDocument/onTypeFormatting'
-        request.reply_result(nil)
+        unless client.format_on_type
+          request.reply_result(nil)
+          return
+        end
+        file_uri = request.params['textDocument']['uri']
+        line_num = request.params['position']['line']
+        char_num = request.params['position']['character']
+        content  = documents.document(file_uri)
+        begin
+          case documents.document_type(file_uri)
+          when :manifest
+            request.reply_result(PuppetLanguageServer::Manifest::FormatOnTypeProvider.instance.format(
+                                   content,
+                                   line_num,
+                                   char_num,
+                                   request.params['ch'],
+                                   request.params['options']
+                                 ))
+          else
+            raise "Unable to format on type on #{file_uri}"
+          end
+        rescue StandardError => e
+          PuppetLanguageServer.log_message(:error, "(textDocument/onTypeFormatting) #{e}")
+          request.reply_result(nil)
+        end
 
       when 'textDocument/signatureHelp'
         file_uri = request.params['textDocument']['uri']

lib/puppet-languageserver/providers.rb

@@ -5,6 +5,7 @@
   manifest/completion_provider
   manifest/definition_provider
   manifest/document_symbol_provider
+  manifest/format_on_type_provider
   manifest/signature_provider
   manifest/validation_provider
   manifest/hover_provider

spec/languageserver/unit/puppet-languageserver/manifest/format_on_type_provider_spec.rb

@@ -0,0 +1,123 @@
+require 'spec_helper'
+
+describe 'PuppetLanguageServer::Manifest::FormatOnTypeProvider' do
+  let(:subject) { PuppetLanguageServer::Manifest::FormatOnTypeProvider.new }
+
+  describe '::instance' do
+    it 'should exist' do
+      expect(PuppetLanguageServer::Manifest::FormatOnTypeProvider).to respond_to(:instance)
+    end
+
+    it 'should return the same object' do
+      object1 = PuppetLanguageServer::Manifest::FormatOnTypeProvider.instance
+      object2 = PuppetLanguageServer::Manifest::FormatOnTypeProvider.instance
+      expect(object1).to eq(object2)
+    end
+  end
+
+  describe '#format' do
+    let(:formatting_options) do
+      LSP::FormattingOptions.new.tap do |item|
+        item.tabSize = 2
+        item.insertSpaces = true
+      end.to_h
+    end
+
+    [' ', '=', ','].each do |trigger|
+      context "given a trigger character of '#{trigger}'" do
+        it 'should return an empty array' do
+          result = subject.format("{\n  oneline    =>\n}\n", 1, 1, trigger, formatting_options)
+          expect(result).to eq([])
+        end
+      end
+    end
+
+    context "given a trigger character of greater-than '>'" do
+      let(:trigger_character) { '>' }
+      let(:content) do <<-MANIFEST
+user {
+  ensure=> 'something',
+  password   =>
+  name => {
+    'abc' => '123',
+    'def'    => '789',
+  },
+  name2    => 'correct',
+}
+MANIFEST
+      end
+      let(:valid_cursor) { { line: 2, char: 15 } }
+      let(:inside_cursor) { { line: 5, char: 15 } }
+
+      it 'should return an empty array if the cursor is not on a hashrocket' do
+        result = subject.format(content, 1, 1, trigger_character, formatting_options)
+        expect(result).to eq([])
+      end
+
+      it 'should return an empty array if the formatting options uses tabs' do
+        result = subject.format(content, valid_cursor[:line], valid_cursor[:char], trigger_character, formatting_options.tap { |i| i['insertSpaces'] = false} )
+        expect(result).to eq([])
+      end
+
+      it 'should return an empty array if the document is large' do
+        large_content = content + ' ' * 4096
+        result = subject.format(large_content, valid_cursor[:line], valid_cursor[:char], trigger_character, formatting_options)
+        expect(result).to eq([])
+      end
+
+      # Valid hashrocket key tests
+      [
+        { name: 'bare name',            text: 'barename' },
+        { name: 'single quoted string', text: '\'name\'' },
+        { name: 'double quoted string', text: '"name"' },
+      ].each do |testcase|
+        context "and given a manifest with #{testcase[:name]}" do
+          let(:content) { "{\n  a =>\n  ##TESTCASE## => 'value'\n}\n"}
+
+          it 'should return an empty' do
+            result = subject.format(content.gsub('##TESTCASE##', testcase[:text]), 1, 6, trigger_character, formatting_options)
+            # The expected TextEdit should edit the `a =>`
+            expect(result.count).to eq(1)
+            expect(result[0].range.start.line).to eq(1)
+            expect(result[0].range.start.character).to eq(3)
+            expect(result[0].range.end.line).to eq(1)
+            expect(result[0].range.end.character).to eq(4)
+          end
+        end
+      end
+
+      it 'should have valid text edits in the outer hash' do
+        result = subject.format(content, valid_cursor[:line], valid_cursor[:char], trigger_character, formatting_options)
+
+        expect(result.count).to eq(3)
+        expect(result[0].to_h).to eq({"range"=>{"start"=>{"character"=>8,  "line"=>1}, "end"=>{"character"=>8,  "line"=>1}}, "newText"=>"   "})
+        expect(result[1].to_h).to eq({"range"=>{"start"=>{"character"=>10, "line"=>2}, "end"=>{"character"=>13, "line"=>2}}, "newText"=>" "})
+        expect(result[2].to_h).to eq({"range"=>{"start"=>{"character"=>6,  "line"=>3}, "end"=>{"character"=>7,  "line"=>3}}, "newText"=>"     "})
+      end
+
+      it 'should have valid text edits in the inner hash' do
+        result = subject.format(content, inside_cursor[:line], inside_cursor[:char], trigger_character, formatting_options)
+
+        expect(result.count).to eq(1)
+        expect(result[0].to_h).to eq({"range"=>{"start"=>{"character"=>9, "line"=>5}, "end"=>{"character"=>13, "line"=>5}}, "newText"=>" "})
+      end
+
+      # Invalid scenarios
+      [
+        { name: 'only one line',                      content: "{\n  oneline    =>\n}\n" },
+        { name: 'there is nothing to indent',         content: "{\n  oneline    =>\n  nextline12 => 'value',\n}\n" },
+        { name: 'no starting Left Brace',             content: "\n  oneline    =>\n  nextline12 => 'value',\n}\n" },
+        { name: 'no ending Right Brace',              content: "{\n  oneline    =>\n  nextline12 => 'value',\n\n" },
+        { name: 'hashrockets on the same line',       content: "{\n  oneline    => ,  nextline12 => 'value',\n\n"},
+        { name: 'invalid text before the hashrocket', content: "{\n  String[]   =>\n  nextline => 'value',\n}\n" },
+      ].each do |testcase|
+        context "and given a manifest with #{testcase[:name]}" do
+          it 'should return an empty' do
+            result = subject.format(testcase[:content], 1, 15, trigger_character, formatting_options)
+            expect(result).to eq([])
+          end
+        end
+      end
+    end
+  end
+end

spec/languageserver/unit/puppet-languageserver/message_router_spec.rb

@@ -859,6 +859,7 @@
     context 'given a textDocument/onTypeFormatting request' do
       let(:request_rpc_method) { 'textDocument/onTypeFormatting' }
       let(:file_uri) { MANIFEST_FILENAME }
+      let(:file_content) { "{\n  a =>\n  name => 'value'\n}\n" }
       let(:line_num) { 1 }
       let(:char_num) { 6 }
       let(:trigger_char) { '>' }
@@ -874,11 +875,77 @@
         'ch' => trigger_char,
         'options' => formatting_options
       } }
+      let(:provider) { PuppetLanguageServer::Manifest::FormatOnTypeProvider.new }
 
-      it 'should not log an error message' do
-        expect(PuppetLanguageServer).to_not receive(:log_message).with(:error,"Unknown RPC method #{request_rpc_method}")
+      before(:each) do
+        subject.documents.clear
+        subject.documents.set_document(file_uri,file_content, 0)
+      end
 
-        subject.receive_request(request)
+      context 'with client.format_on_type set to false' do
+        before(:each) do
+          allow(subject.client).to receive(:format_on_type).and_return(false)
+        end
+
+        it 'should reply with nil' do
+          expect(request).to receive(:reply_result).with(nil)
+          subject.receive_request(request)
+        end
+      end
+
+      context 'with client.format_on_type set to true' do
+        before(:each) do
+          allow(subject.client).to receive(:format_on_type).and_return(true)
+        end
+
+        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 format on type on/)
+
+            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 }
+
+          before(:each) do
+            allow(PuppetLanguageServer::Manifest::FormatOnTypeProvider).to receive(:instance).and_return(provider)
+          end
+
+          it 'should call format method on the Format On Type provider' do
+            expect(provider).to receive(:format)
+              .with(file_content, line_num, char_num, trigger_char, formatting_options).and_return('something')
+
+            result = subject.receive_request(request)
+          end
+
+          context 'and an error occurs during formatting' do
+            before(:each) do
+              expect(provider).to receive(:format).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
     end