diff --git a/lib/puppet-syntax/tasks/puppet-syntax.rb b/lib/puppet-syntax/tasks/puppet-syntax.rb index 8e0c3cf..c100bc3 100644 --- a/lib/puppet-syntax/tasks/puppet-syntax.rb +++ b/lib/puppet-syntax/tasks/puppet-syntax.rb @@ -48,9 +48,16 @@ def initialize(*args) $stderr.puts "---> #{t.name}" c = PuppetSyntax::Templates.new - errors = c.check(filelist_templates) - $stdout.puts "#{errors.join("\n")}\n" unless errors.empty? - exit 1 unless errors.empty? + result = c.check(filelist_templates) + unless result[:warnings].empty? + $stdout.puts "WARNINGS:" + $stdout.puts result[:warnings].join("\n") + end + unless result[:errors].empty? + $stderr.puts "ERRORS:" + $stderr.puts result[:errors].join("\n") + exit 1 + end end desc 'Syntax check Hiera config files' diff --git a/lib/puppet-syntax/templates.rb b/lib/puppet-syntax/templates.rb index 602bc5b..93db820 100644 --- a/lib/puppet-syntax/templates.rb +++ b/lib/puppet-syntax/templates.rb @@ -9,38 +9,64 @@ def check(filelist) # We now have to redirect STDERR in order to capture warnings. $stderr = warnings = StringIO.new() - errors = [] + result = { warnings: [], errors: [] } filelist.each do |file| if File.extname(file) == '.epp' or PuppetSyntax.epp_only - errors.concat validate_epp(file) + tmp = validate_epp(file) elsif File.extname(file) == '.erb' - errors.concat validate_erb(file) + tmp = validate_erb(file) end + result.merge!(tmp) { |k, a, b| a.concat(b) } unless tmp.nil? end $stderr = STDERR - errors << warnings.string unless warnings.string.empty? - errors.map! { |e| e.to_s } + result[:warnings] << warnings.string unless warnings.string.empty? - errors + result[:errors].map! { |e| e.to_s } + result[:warnings].map! { |w| w.to_s } + + result end def validate_epp(filename) require 'puppet/pops' - errors = [] + result = { warnings: [], errors: [] } + formatter = Puppet::Pops::Validation::DiagnosticFormatterPuppetStyle.new + evaluating_parser = Puppet::Pops::Parser::EvaluatingParser::EvaluatingEppParser.new() + parser = evaluating_parser.parser() begin - parser = Puppet::Pops::Parser::EvaluatingParser::EvaluatingEppParser.new() - parser.parse_file(filename) - rescue => detail - errors << detail + parse_result = parser.parse_file(filename) + validation_result = evaluating_parser.validate(parse_result.model) + + # print out any warnings + validation_result.warnings.each do |warn| + message = formatter.format_message(warn) + file = warn.file + line = warn.source_pos.line + column = warn.source_pos.pos + result[:warnings] << "#{file}:#{line}:#{column}: #{message}" + end + + # collect errors and return them in order to indicate validation failure + validation_result.errors.each do |err| + message = formatter.format_message(err) + file = err.file + line = err.source_pos.line + column = err.source_pos.pos + result[:errors] << "#{file}:#{line}:#{column}: #{message}" + end + rescue Puppet::ParseError, SyntaxError => exc + result[:errors] << exc + rescue => exc + result[:errors] << exc end - errors + result end def validate_erb(filename) - errors = [] + result = { warnings: [], errors: [] } begin erb = ERB.new(File.read(filename), nil, '-') @@ -53,10 +79,10 @@ def validate_erb(filename) # This is normal because we don't have the variables that would # ordinarily be bound by the parent Puppet manifest. rescue SyntaxError => error - errors << error + result[:errors] << error end - errors + result end end end diff --git a/spec/puppet-syntax/templates_spec.rb b/spec/puppet-syntax/templates_spec.rb index 495a2da..3ee8564 100644 --- a/spec/puppet-syntax/templates_spec.rb +++ b/spec/puppet-syntax/templates_spec.rb @@ -18,91 +18,99 @@ files = fixture_templates('pass.erb') res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should ignore NameErrors from unbound variables' do files = fixture_templates('pass_unbound_var.erb') res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should catch SyntaxError' do files = fixture_templates('fail_error.erb') res = subject.check(files) - expect(res.size).to eq(1) - expect(res[0]).to match(/2: syntax error, unexpected/) + expect(res[:errors].size).to eq(1) + expect(res[:errors][0]).to match(/2: syntax error, unexpected/) end it 'should catch Ruby warnings' do files = fixture_templates('fail_warning.erb') res = subject.check(files) - expect(res.size).to eq(1) - expect(res[0]).to match(conditional_warning_regex) + expect(res[:warnings].size).to eq(1) + expect(res[:warnings][0]).to match(conditional_warning_regex) end it 'should read more than one valid file' do files = fixture_templates(['pass.erb', 'pass_unbound_var.erb']) res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should continue after finding an error in the first file' do files = fixture_templates(['fail_error.erb', 'fail_warning.erb']) res = subject.check(files) - expect(res.size).to eq(2) - expect(res[0]).to match(/2: syntax error, unexpected/) - expect(res[1]).to match(conditional_warning_regex) + expect(res[:warnings].size).to eq(1) + expect(res[:errors].size).to eq(1) + expect(res[:errors][0]).to match(/2: syntax error, unexpected/) + expect(res[:warnings][0]).to match(conditional_warning_regex) end it 'should ignore a TypeError' do files = fixture_templates('typeerror_shouldwin.erb') res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should ignore files without .erb extension' do files = fixture_templates('ignore.tpl') res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should return nothing from a valid file' do files = fixture_templates('pass.epp') res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should catch SyntaxError' do files = fixture_templates('fail_error.epp') res = subject.check(files) - expect(res.size).to eq(1) - expect(res[0]).to match(/This Type-Name has no effect/) + expect(res[:errors].size).to eq(1) + expect(res[:errors][0]).to match(/This Type-Name has no effect/) end it 'should read more than one valid file' do files = fixture_templates(['pass.epp', 'pass_also.epp']) res = subject.check(files) - expect(res).to match([]) + expect(res[:warnings]).to match([]) + expect(res[:errors]).to match([]) end it 'should continue after finding an error in the first file' do files = fixture_templates(['fail_error.epp', 'fail_error_also.epp']) res = subject.check(files) - expect(res.size).to eq(2) - expect(res[0]).to match(/This Type-Name has no effect/) - expect(res[1]).to match(/Syntax error at '}' \(file: \S*\/fail_error_also.epp, line: 2, column: 4\)/) + expect(res[:errors].size).to eq(2) + expect(res[:errors][0]).to match(/This Type-Name has no effect/) + expect(res[:errors][1]).to match(/Syntax error at '}' \(file: \S*\/fail_error_also.epp, line: 2, column: 4\)/) end context "when the 'epp_only' options is set" do @@ -114,7 +122,7 @@ files = fixture_templates('pass.erb') res = subject.check(files) - expect(res.size).to eq(1) + expect(res[:errors].size).to eq(1) end end end