Skip to content

Commit

Permalink
(GH-177) Add auto-align hash rocket feature
Browse files Browse the repository at this point in the history
Previously the onTypeFormatting provider was created and responds to client
settings, turning it on and off appropriately.  This commit actually adds the
on type formatter:

* Only for puppet manifests (.pp)
* Uses puppet-lint lexer to tokenise the document and uses the tokens to
  determine indenting
* Will not operate if the document is over 4KB in size or the client is using
  tabs instead of spaces
* Adds tests for the formatting
  • Loading branch information
glennsarti committed Sep 17, 2019
1 parent db97c1a commit 87fa677
Show file tree
Hide file tree
Showing 5 changed files with 356 additions and 4 deletions.
137 changes: 137 additions & 0 deletions lib/puppet-languageserver/manifest/format_on_type_provider.rb
Original file line number Diff line number Diff line change
@@ -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
26 changes: 25 additions & 1 deletion lib/puppet-languageserver/message_router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
1 change: 1 addition & 0 deletions lib/puppet-languageserver/providers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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) { '>' }
Expand All @@ -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

Expand Down

0 comments on commit 87fa677

Please sign in to comment.