Skip to content

Commit d2ecd7d

Browse files
committed
(GH-50) Add document formatter for puppet-lint
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.
1 parent a8f72de commit d2ecd7d

File tree

5 files changed

+187
-1
lines changed

5 files changed

+187
-1
lines changed

lib/languageserver/languageserver.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
%w[constants diagnostic completion_list completion_item hover location puppet_version puppet_compilation].each do |lib|
1+
%w[constants diagnostic completion_list completion_item hover location puppet_version puppet_compilation puppet_fix_diagnostic_errors].each do |lib|
22
begin
33
require "languageserver/#{lib}"
44
rescue LoadError
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
module LanguageServer
2+
module PuppetFixDiagnosticErrorsRequest
3+
# export interface PuppetFixDiagnosticErrorsRequestParams {
4+
# documentUri: string;
5+
# alwaysReturnContent: boolean;
6+
# }
7+
8+
def self.create(options)
9+
result = {
10+
'alwaysReturnContent' => false
11+
}
12+
raise('documentUri is a required field for PuppetFixDiagnosticErrorsRequest') if options['documentUri'].nil?
13+
14+
result['documentUri'] = options['documentUri']
15+
result['alwaysReturnContent'] = options['alwaysReturnContent'] unless options['alwaysReturnContent'].nil?
16+
result
17+
end
18+
end
19+
20+
module PuppetFixDiagnosticErrorsResponse
21+
# export interface PuppetFixDiagnosticErrorsResponse {
22+
# documentUri: string;
23+
# fixesApplied: number;
24+
# newContent?: string;
25+
# }
26+
27+
def self.create(options)
28+
result = {}
29+
raise('documentUri is a required field for PuppetFixDiagnosticErrorsResponse') if options['documentUri'].nil?
30+
raise('fixesApplied is a required field for PuppetFixDiagnosticErrorsResponse') if options['fixesApplied'].nil?
31+
32+
result['documentUri'] = options['documentUri']
33+
result['fixesApplied'] = options['fixesApplied']
34+
result['newContent'] = options['newContent'] unless options['newContent'].nil?
35+
36+
result
37+
end
38+
end
39+
end

lib/puppet-languageserver/document_validator.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,33 @@ def self.find_module_root_from_path(path)
2424
module_root
2525
end
2626

27+
# Similar to 'validate' this will run puppet-lint and returns
28+
# the manifest with any fixes applied
29+
#
30+
# Returns:
31+
# [ <Int> Number of problems fixed,
32+
# <String> New Content
33+
# ]
34+
def self.fix_validate_errors(content, workspace)
35+
# Find module root and attempt to build PuppetLint options
36+
module_root = find_module_root_from_path(workspace)
37+
linter_options = nil
38+
if module_root.nil?
39+
linter_options = PuppetLint::OptParser.build
40+
else
41+
Dir.chdir(module_root.to_s) { linter_options = PuppetLint::OptParser.build }
42+
end
43+
linter_options.parse!(['--fix'])
44+
45+
linter = PuppetLint::Checks.new
46+
linter.load_data(nil, content)
47+
48+
problems = linter.run(nil, content)
49+
problems_fixed = problems.nil? ? 0 : problems.count { |item| item[:kind] == :fixed }
50+
51+
[problems_fixed, linter.manifest]
52+
end
53+
2754
def self.validate(content, workspace, _max_problems = 100)
2855
result = []
2956
# TODO: Need to implement max_problems

lib/puppet-languageserver/message_router.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,30 @@ def receive_request(request)
153153
request.reply_result(LanguageServer::PuppetCompilation.create('dotContent' => dot_content,
154154
'error' => error_content))
155155

156+
when 'puppet/fixDiagnosticErrors'
157+
begin
158+
formatted_request = LanguageServer::PuppetFixDiagnosticErrorsRequest.create(request.params)
159+
file_uri = formatted_request['documentUri']
160+
content = documents.document(file_uri)
161+
162+
changes, new_content = PuppetLanguageServer::DocumentValidator.fix_validate_errors(content, @workspace)
163+
164+
request.reply_result(LanguageServer::PuppetFixDiagnosticErrorsResponse.create(
165+
'documentUri' => formatted_request['documentUri'],
166+
'fixesApplied' => changes,
167+
'newContent' => changes > 0 || formatted_request['alwaysReturnContent'] ? new_content : nil
168+
))
169+
rescue StandardError => exception
170+
PuppetLanguageServer.log_message(:error, "(puppet/fixDiagnosticErrors) #{exception}")
171+
unless formatted_request.nil?
172+
request.reply_result(LanguageServer::PuppetFixDiagnosticErrorsResponse.create(
173+
'documentUri' => formatted_request['documentUri'],
174+
'fixesApplied' => 0,
175+
'newContent' => formatted_request['alwaysReturnContent'] ? content : nil # rubocop:disable Metrics/BlockNesting
176+
))
177+
end
178+
end
179+
156180
when 'textDocument/completion'
157181
file_uri = request.params['textDocument']['uri']
158182
line_num = request.params['position']['line']

spec/languageserver/integration/puppet-languageserver/document_validator_spec.rb

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,102 @@
33
describe 'document_validator' do
44
let(:subject) { PuppetLanguageServer::DocumentValidator }
55

6+
describe '#fix_validate_errors' do
7+
describe "Given an incomplete manifest which has syntax errors but no lint errors" do
8+
let(:manifest) { 'user { \'Bob\'' }
9+
10+
it "should return no changes" do
11+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
12+
expect(problems_fixed).to eq(0)
13+
expect(new_content).to eq(manifest)
14+
end
15+
end
16+
17+
describe "Given a complete manifest which has a single fixable lint errors" do
18+
let(:manifest) { "
19+
user { \"Bob\":
20+
ensure => 'present'
21+
}"
22+
}
23+
let(:new_manifest) { "
24+
user { 'Bob':
25+
ensure => 'present'
26+
}"
27+
}
28+
29+
it "should return changes" do
30+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
31+
expect(problems_fixed).to eq(1)
32+
expect(new_content).to eq(new_manifest)
33+
end
34+
end
35+
36+
describe "Given a complete manifest which has multiple fixable lint errors" do
37+
let(:manifest) { "
38+
// bad comment
39+
user { \"Bob\":
40+
name => 'username',
41+
ensure => 'present'
42+
}"
43+
}
44+
let(:new_manifest) { "
45+
# bad comment
46+
user { 'Bob':
47+
name => 'username',
48+
ensure => 'present'
49+
}"
50+
}
51+
52+
it "should return changes" do
53+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
54+
expect(problems_fixed).to eq(3)
55+
expect(new_content).to eq(new_manifest)
56+
end
57+
end
58+
59+
60+
describe "Given a complete manifest which has unfixable lint errors" do
61+
let(:manifest) { "
62+
user { 'Bob':
63+
name => 'name',
64+
ensure => 'present'
65+
}"
66+
}
67+
68+
it "should return no changes" do
69+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
70+
expect(problems_fixed).to eq(0)
71+
expect(new_content).to eq(manifest)
72+
end
73+
end
74+
75+
describe "Given a complete manifest with CRLF which has fixable lint errors" do
76+
let(:manifest) { "user { \"Bob\":\r\nensure => 'present'\r\n}" }
77+
let(:new_manifest) { "user { 'Bob':\r\nensure => 'present'\r\n}" }
78+
79+
it "should preserve CRLF" do
80+
pending('Release of https://github.com/rodjek/puppet-lint/commit/2a850ab3fd3694a4dd0c4d2f22a1e60b9ca0a495')
81+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
82+
expect(problems_fixed).to eq(1)
83+
expect(new_content).to eq(new_manifest)
84+
end
85+
end
86+
87+
describe "Given a complete manifest which has disabed fixable lint errors" do
88+
let(:manifest) { "
89+
user { \"Bob\": # lint:ignore:double_quoted_strings
90+
ensure => 'present'
91+
}"
92+
}
93+
94+
it "should return no changes" do
95+
problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
96+
expect(problems_fixed).to eq(0)
97+
expect(new_content).to eq(manifest)
98+
end
99+
end
100+
end
101+
6102
describe '#validate' do
7103
describe "Given an incomplete manifest which has syntax errors" do
8104
let(:manifest) { 'user { "Bob"' }

0 commit comments

Comments
 (0)