Merge pull request #210 from glennsarti/add-puppet-lint-fix

(GH-50) Add document formatter for puppet-lint

Author: jpogran

client/package.json

@@ -267,6 +267,12 @@
       "type": "object",
       "title": "puppet",
       "properties": {
+        "puppet.format.enable": {
+          "type": "boolean",
+          "scope": "window",
+          "default": true,
+          "description": "Enable/disable the Puppet formatter"
+        },
         "puppet.languageserver.address": {
           "type": "string",
           "default": "127.0.0.1",

client/src/commands/puppetcommands.ts

@@ -7,6 +7,7 @@ import {
   getNodeGraphUri, showNodeGraph
 } from '../../src/providers/previewNodeGraphProvider';
 import { puppetResourceCommand } from '../commands/puppet/puppetResourceCommand';
+import { PuppetFormatDocumentProvider } from '../providers/puppetFormatDocumentProvider';
 
 export function setupPuppetCommands(langID:string, connManager:IConnectionManager, ctx:vscode.ExtensionContext, logger: ILogger){
 
@@ -16,6 +17,16 @@ export function setupPuppetCommands(langID:string, connManager:IConnectionManage
     resourceCommand.run();
   }));
 
+  ctx.subscriptions.push(vscode.languages.registerDocumentFormattingEditProvider('puppet', {
+    provideDocumentFormattingEdits: (document, options, token) => {
+      if (vscode.workspace.getConfiguration('puppet').get('format.enable')) {
+        return PuppetFormatDocumentProvider(document, options, connManager)
+      } else {
+        return []
+      }
+    }
+  }));
+
   ctx.subscriptions.push(vscode.commands.registerCommand(messages.PuppetCommandStrings.PuppetNodeGraphToTheSideCommandId,
     uri => showNodeGraph(uri, true))
   );

client/src/messages.ts

@@ -16,7 +16,7 @@ export interface PuppetVersionDetails {
 
 export interface PuppetResourceRequestParams {
   typename: string;
-  title:string;
+  title: string;
 }
 
 export namespace PuppetResourceRequest {
@@ -28,6 +28,21 @@ export interface PuppetResourceResponse {
   error: string;
 }
 
+export interface PuppetFixDiagnosticErrorsRequestParams {
+  documentUri: string;
+  alwaysReturnContent: boolean;
+}
+
+export namespace PuppetFixDiagnosticErrorsRequest {
+  export const type = new RequestType<PuppetFixDiagnosticErrorsRequestParams, any, void, void>('puppet/fixDiagnosticErrors');
+}
+
+export interface PuppetFixDiagnosticErrorsResponse {
+  documentUri: string;
+  fixesApplied: number;
+  newContent: string;
+}
+
 export namespace CompileNodeGraphRequest {
   export const type = new RequestType<any, any, void, void>('puppet/compileNodeGraph');
 }

client/src/providers/puppetFormatDocumentProvider.ts

@@ -0,0 +1,35 @@
+'use strict';
+
+import * as vscode from 'vscode';
+import { ConnectionStatus } from '../interfaces';
+import { IConnectionManager } from '../connection';
+import * as messages from '../messages';
+
+class RequestParams implements messages.PuppetFixDiagnosticErrorsRequestParams {
+  documentUri: string;
+  alwaysReturnContent: boolean;
+}
+
+export function PuppetFormatDocumentProvider(document: vscode.TextDocument, options:vscode.FormattingOptions, connMgr: IConnectionManager) : Thenable<vscode.TextEdit[]> {
+  if (connMgr.status != ConnectionStatus.Running ) {
+    vscode.window.showInformationMessage("Formatting Puppet files is not available as the Language Server is not ready");
+    return new Promise((resolve) => { resolve([]); });
+  }
+
+  let requestParams = new RequestParams;
+  requestParams.documentUri = document.uri.toString(false);
+  requestParams.alwaysReturnContent = false;
+
+  return connMgr.languageClient
+    .sendRequest(messages.PuppetFixDiagnosticErrorsRequest.type, requestParams)
+    .then(
+      (result) => {
+        result = result as messages.PuppetFixDiagnosticErrorsResponse
+        if (result.fixesApplied > 0 && result.newContent != null) {
+          return [vscode.TextEdit.replace(new vscode.Range(0,0, document.lineCount,0), result.newContent)]
+        } else {
+          return []
+        }
+      }
+    )
+}

server/lib/languageserver/constants.rb

@@ -49,4 +49,9 @@ module LanguageServer
   DIAGNOSTICSEVERITY_WARNING = 2
   DIAGNOSTICSEVERITY_INFORMATION = 3
   DIAGNOSTICSEVERITY_HINT = 4
+
+  MESSAGE_TYPE_ERROR = 1
+  MESSAGE_TYPE_WARNING = 2
+  MESSAGE_TYPE_INFO = 3
+  MESSAGE_TYPE_LOG = 2
 end

server/lib/languageserver/languageserver.rb

@@ -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

server/lib/languageserver/puppet_fix_diagnostic_errors.rb

@@ -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

server/lib/puppet-languageserver/document_validator.rb

@@ -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

server/lib/puppet-languageserver/message_router.rb

@@ -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']

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

@@ -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"' }