Skip to content

Commit

Permalink
(GH-50) Add document formatter for puppet-lint
Browse files Browse the repository at this point in the history
Previously the vscode extension ran puppet-lint locally for fix linting issues,
however with the language server this feature was lost.  This commit adds the
document formatting feature back into the vscode extension;

* Uses the puppet-lint --fix feature to actually apply the fixes
* Adds a custom command to the language protocol puppet/fixDiagnosticErrors
  which will return a 'fixed' document and the number of fixes applied
* Adds an additional configuration option puppet.format.enable which will
  disable document formatting.
  • Loading branch information
glennsarti committed Jan 26, 2018
1 parent a8f72de commit d2ecd7d
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/languageserver/languageserver.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%w[constants diagnostic completion_list completion_item hover location puppet_version puppet_compilation].each do |lib|
%w[constants diagnostic completion_list completion_item hover location puppet_version puppet_compilation puppet_fix_diagnostic_errors].each do |lib|
begin
require "languageserver/#{lib}"
rescue LoadError
Expand Down
39 changes: 39 additions & 0 deletions lib/languageserver/puppet_fix_diagnostic_errors.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
module LanguageServer
module PuppetFixDiagnosticErrorsRequest
# export interface PuppetFixDiagnosticErrorsRequestParams {
# documentUri: string;
# alwaysReturnContent: boolean;
# }

def self.create(options)
result = {
'alwaysReturnContent' => false
}
raise('documentUri is a required field for PuppetFixDiagnosticErrorsRequest') if options['documentUri'].nil?

result['documentUri'] = options['documentUri']
result['alwaysReturnContent'] = options['alwaysReturnContent'] unless options['alwaysReturnContent'].nil?
result
end
end

module PuppetFixDiagnosticErrorsResponse
# export interface PuppetFixDiagnosticErrorsResponse {
# documentUri: string;
# fixesApplied: number;
# newContent?: string;
# }

def self.create(options)
result = {}
raise('documentUri is a required field for PuppetFixDiagnosticErrorsResponse') if options['documentUri'].nil?
raise('fixesApplied is a required field for PuppetFixDiagnosticErrorsResponse') if options['fixesApplied'].nil?

result['documentUri'] = options['documentUri']
result['fixesApplied'] = options['fixesApplied']
result['newContent'] = options['newContent'] unless options['newContent'].nil?

result
end
end
end
27 changes: 27 additions & 0 deletions lib/puppet-languageserver/document_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,33 @@ def self.find_module_root_from_path(path)
module_root
end

# Similar to 'validate' this will run puppet-lint and returns
# the manifest with any fixes applied
#
# Returns:
# [ <Int> Number of problems fixed,
# <String> New Content
# ]
def self.fix_validate_errors(content, workspace)
# Find module root and attempt to build PuppetLint options
module_root = find_module_root_from_path(workspace)
linter_options = nil
if module_root.nil?
linter_options = PuppetLint::OptParser.build
else
Dir.chdir(module_root.to_s) { linter_options = PuppetLint::OptParser.build }
end
linter_options.parse!(['--fix'])

linter = PuppetLint::Checks.new
linter.load_data(nil, content)

problems = linter.run(nil, content)
problems_fixed = problems.nil? ? 0 : problems.count { |item| item[:kind] == :fixed }

[problems_fixed, linter.manifest]
end

def self.validate(content, workspace, _max_problems = 100)
result = []
# TODO: Need to implement max_problems
Expand Down
24 changes: 24 additions & 0 deletions lib/puppet-languageserver/message_router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,30 @@ def receive_request(request)
request.reply_result(LanguageServer::PuppetCompilation.create('dotContent' => dot_content,
'error' => error_content))

when 'puppet/fixDiagnosticErrors'
begin
formatted_request = LanguageServer::PuppetFixDiagnosticErrorsRequest.create(request.params)
file_uri = formatted_request['documentUri']
content = documents.document(file_uri)

changes, new_content = PuppetLanguageServer::DocumentValidator.fix_validate_errors(content, @workspace)

request.reply_result(LanguageServer::PuppetFixDiagnosticErrorsResponse.create(
'documentUri' => formatted_request['documentUri'],
'fixesApplied' => changes,
'newContent' => changes > 0 || formatted_request['alwaysReturnContent'] ? new_content : nil
))
rescue StandardError => exception
PuppetLanguageServer.log_message(:error, "(puppet/fixDiagnosticErrors) #{exception}")
unless formatted_request.nil?
request.reply_result(LanguageServer::PuppetFixDiagnosticErrorsResponse.create(
'documentUri' => formatted_request['documentUri'],
'fixesApplied' => 0,
'newContent' => formatted_request['alwaysReturnContent'] ? content : nil # rubocop:disable Metrics/BlockNesting
))
end
end

when 'textDocument/completion'
file_uri = request.params['textDocument']['uri']
line_num = request.params['position']['line']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,102 @@
describe 'document_validator' do
let(:subject) { PuppetLanguageServer::DocumentValidator }

describe '#fix_validate_errors' do
describe "Given an incomplete manifest which has syntax errors but no lint errors" do
let(:manifest) { 'user { \'Bob\'' }

it "should return no changes" do
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(0)
expect(new_content).to eq(manifest)
end
end

describe "Given a complete manifest which has a single fixable lint errors" do
let(:manifest) { "
user { \"Bob\":
ensure => 'present'
}"
}
let(:new_manifest) { "
user { 'Bob':
ensure => 'present'
}"
}

it "should return changes" do
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(1)
expect(new_content).to eq(new_manifest)
end
end

describe "Given a complete manifest which has multiple fixable lint errors" do
let(:manifest) { "
// bad comment
user { \"Bob\":
name => 'username',
ensure => 'present'
}"
}
let(:new_manifest) { "
# bad comment
user { 'Bob':
name => 'username',
ensure => 'present'
}"
}

it "should return changes" do
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(3)
expect(new_content).to eq(new_manifest)
end
end


describe "Given a complete manifest which has unfixable lint errors" do
let(:manifest) { "
user { 'Bob':
name => 'name',
ensure => 'present'
}"
}

it "should return no changes" do
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(0)
expect(new_content).to eq(manifest)
end
end

describe "Given a complete manifest with CRLF which has fixable lint errors" do
let(:manifest) { "user { \"Bob\":\r\nensure => 'present'\r\n}" }
let(:new_manifest) { "user { 'Bob':\r\nensure => 'present'\r\n}" }

it "should preserve CRLF" do
pending('Release of https://github.com/rodjek/puppet-lint/commit/2a850ab3fd3694a4dd0c4d2f22a1e60b9ca0a495')
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(1)
expect(new_content).to eq(new_manifest)
end
end

describe "Given a complete manifest which has disabed fixable lint errors" do
let(:manifest) { "
user { \"Bob\": # lint:ignore:double_quoted_strings
ensure => 'present'
}"
}

it "should return no changes" do
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
expect(problems_fixed).to eq(0)
expect(new_content).to eq(manifest)
end
end
end

describe '#validate' do
describe "Given an incomplete manifest which has syntax errors" do
let(:manifest) { 'user { "Bob"' }
Expand Down

0 comments on commit d2ecd7d

Please sign in to comment.