WIP #255 Allow textDocument/formatting to ignore instructions that sp…

…an multiple lines Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Apr 11, 2021 · 314c8f7 · 314c8f7
1 parent 5693361
commit 314c8f7
Show file tree

Hide file tree

Showing 5 changed files with 329 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,10 +3,16 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 ### Added
+- settings
+  - docker.languageserver.formatter.ignoreMultilineInstructions? ([#255](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/255))
+    - value = ( true | false )
+    - this will make the formatter ignore instructions that span multiple lines
 - suggest working directories if editing the last argument of ADD and COPY instructions that aren't written in JSON ([rcjsuen/dockerfile-language-service#77](https://github.com/rcjsuen/dockerfile-language-service/issues/77))
 - allow multiple arguments to be defined for ARG instructions to support Docker Engine 20.10 ([rcjsuen/dockerfile-utils#92](https://github.com/rcjsuen/dockerfile-utils/issues/92))
 - optimized range formatting so that it does not return unnecessary edits ([rcjsuen/dockerfile-language-service#81](https://github.com/rcjsuen/dockerfile-language-service/issues/81))
 - optimized on type formatting so that it does not return unnecessary edits ([rcjsuen/dockerfile-language-service#82](https://github.com/rcjsuen/dockerfile-language-service/issues/82))
+- textDocument/formatting
+  - allow the formatter to skip formatting of instructions that span multiple lines ([#255](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/255))
 
 ### Fixed
 - do not validate variable substitutions if found in CMD and ENTRYPOINT ([rcjsuen/dockerfile-utils#89](https://github.com/rcjsuen/dockerfile-utils/issues/89))

diff --git a/README.md b/README.md
@@ -103,6 +103,9 @@ interface Settings {
         instructionEntrypointMultiple?: string,
         instructionHealthcheckMultiple?: string,
         instructionJSONInSingleQuotes?: string
+      },
+      formatter?: {
+        ignoreMultilineInstructions?: boolean,
       }
     }
   }

diff --git a/src/server.ts b/src/server.ts
@@ -19,14 +19,18 @@ import {
 } from 'vscode-languageserver/node';
 import { uriToFilePath } from 'vscode-languageserver/lib/node/files';
 import { ValidatorSettings, ValidationSeverity } from 'dockerfile-utils';
-import { CommandIds, DockerfileLanguageServiceFactory } from 'dockerfile-language-service';
+import { CommandIds, DockerfileLanguageServiceFactory, FormatterSettings } from 'dockerfile-language-service';
+
+let formatterConfiguration: FormatterConfiguration | null = null;
 
 /**
  * The settings to use for the validator if the client doesn't support
  * workspace/configuration requests.
  */
 let validatorSettings: ValidatorSettings | null = null;
 
+const formatterConfigurations: Map<string, Thenable<FormatterConfiguration>> = new Map();
+
 /**
  * The validator settings that correspond to an individual file retrieved via
  * the workspace/configuration request.
@@ -311,7 +315,7 @@ function convertValidatorConfiguration(config: ValidatorConfiguration): Validato
 
 function validateTextDocument(textDocument: TextDocument): void {
 	if (configurationSupport) {
-		getConfiguration(textDocument.uri).then((config: ValidatorConfiguration) => {
+		getValidatorConfiguration(textDocument.uri).then((config: ValidatorConfiguration) => {
 			const fileSettings = convertValidatorConfiguration(config);
 			const diagnostics = service.validate(textDocument.getText(), fileSettings);
 			connection.sendDiagnostics({ uri: textDocument.uri, diagnostics });
@@ -322,6 +326,10 @@ function validateTextDocument(textDocument: TextDocument): void {
 	}
 }
 
+interface FormatterConfiguration {
+	ignoreMultilineInstructions?: boolean;
+}
+
 interface ValidatorConfiguration {
 	deprecatedMaintainer?: string,
 	directiveCasing?: string,
@@ -337,7 +345,8 @@ interface ValidatorConfiguration {
 interface Settings {
 	docker: {
 		languageserver: {
-			diagnostics?: ValidatorConfiguration
+			diagnostics?: ValidatorConfiguration,
+			formatter?: FormatterConfiguration
 		}
 	}
 }
@@ -354,13 +363,22 @@ function getSeverity(severity: string | undefined): ValidationSeverity | null {
 	return null;
 }
 
+function getFormatterConfiguration(resource: string): Thenable<FormatterConfiguration> {
+	let result = formatterConfigurations.get(resource);
+	if (!result) {
+		result = connection.workspace.getConfiguration({ section: "docker.languageserver.formatter", scopeUri: resource });
+		formatterConfigurations.set(resource, result);
+	}
+	return result;
+}
+
 /**
  * Gets the validation configuration that pertains to the specified resource.
  * 
  * @param resource the interested resource
  * @return the configuration to use to validate the interested resource
  */
-function getConfiguration(resource: string): Thenable<ValidatorConfiguration> {
+function getValidatorConfiguration(resource: string): Thenable<ValidatorConfiguration> {
 	let result = validatorConfigurations.get(resource);
 	if (!result) {
 		result = connection.workspace.getConfiguration({ section: "docker.languageserver.diagnostics", scopeUri: resource });
@@ -374,17 +392,36 @@ connection.onNotification(DidChangeConfigurationNotification.type, () => {
 	refreshConfigurations();
 });
 
-/**
- * Wipes and reloads the internal cache of validator configurations.
- */
-function refreshConfigurations() {
+function getConfigurationItems(sectionName): ConfigurationItem[] {
 	// store all the URIs that need to be refreshed
-	const settingsRequest: ConfigurationItem[] = [];
-	for (let uri in documents) {
-		settingsRequest.push({ section: "docker.languageserver.diagnostics", scopeUri: uri });
+	const configurationItems: ConfigurationItem[] = [];
+	for (const uri in documents) {
+		configurationItems.push({ section: sectionName, scopeUri: uri });
 	}
+	return configurationItems;
+}
+
+function refreshFormatterConfigurations() {
+	// store all the URIs that need to be refreshed
+	const settingsRequest = getConfigurationItems("docker.languageserver.formatter");
 	// clear the cache
-	validatorConfigurations.clear();
+	formatterConfigurations.clear();
+
+	// ask the workspace for the configurations
+	connection.workspace.getConfiguration(settingsRequest).then((settings: FormatterConfiguration[]) => {
+		for (let i = 0; i < settings.length; i++) {
+			const resource = settingsRequest[i].scopeUri;
+			// a value might have been stored already, use it instead and ignore this one if so
+			if (settings[i] && !formatterConfigurations.has(resource)) {
+				formatterConfigurations.set(resource, Promise.resolve(settings[i]));
+			}
+		}
+	});
+}
+
+function refreshValidatorConfigurations() {
+	// store all the URIs that need to be refreshed
+	const settingsRequest = getConfigurationItems("docker.languageserver.diagnostics");
 
 	// ask the workspace for the configurations
 	connection.workspace.getConfiguration(settingsRequest).then((values: ValidatorConfiguration[]) => {
@@ -404,14 +441,28 @@ function refreshConfigurations() {
 	});
 }
 
+/**
+ * Wipes and reloads the internal cache of configurations.
+ */
+function refreshConfigurations() {
+	refreshFormatterConfigurations();
+	refreshValidatorConfigurations();
+}
+
 connection.onDidChangeConfiguration((change) => {
 	if (configurationSupport) {
 		refreshConfigurations();
 	} else {
 		let settings = <Settings>change.settings;
-		if (settings.docker && settings.docker.languageserver && settings.docker.languageserver.diagnostics) {
-			validatorSettings = convertValidatorConfiguration(settings.docker.languageserver.diagnostics);
+		if (settings.docker && settings.docker.languageserver) {
+			if (settings.docker.languageserver.diagnostics) {
+				validatorSettings = convertValidatorConfiguration(settings.docker.languageserver.diagnostics);
+			}
+			if (settings.docker.languageserver.formatter) {
+				formatterConfiguration = settings.docker.languageserver.formatter;
+			}
 		} else {
+			formatterConfiguration = null;
 			validatorSettings = convertValidatorConfiguration(null);
 		}
 		// validate all the documents again
@@ -569,8 +620,21 @@ connection.onDocumentSymbol((documentSymbolParams: DocumentSymbolParams): Promis
 
 connection.onDocumentFormatting((documentFormattingParams: DocumentFormattingParams): PromiseLike<TextEdit[]> => {
 	return getDocument(documentFormattingParams.textDocument.uri).then((document) => {
+		if (configurationSupport) {
+			return getFormatterConfiguration(document.uri).then((configuration: FormatterConfiguration) => {
+				if (document) {
+					const options: FormatterSettings = documentFormattingParams.options;
+					options.ignoreMultilineInstructions = configuration !== null && configuration.ignoreMultilineInstructions;
+					return service.format(document.getText(), options);
+				}
+				return [];
+			});
+		}
+
 		if (document) {
-			return service.format(document.getText(), documentFormattingParams.options);
+			const options: FormatterSettings = documentFormattingParams.options;
+			options.ignoreMultilineInstructions = formatterConfiguration !== null && formatterConfiguration.ignoreMultilineInstructions;
+			return service.format(document.getText(), options);
 		}
 		return [];
 	});

diff --git a/test/server.test.ts b/test/server.test.ts
@@ -1843,6 +1843,82 @@ describe("Dockerfile LSP Tests", function() {
 		});
 	});
 
+	function test255(fileName: string, configurationSet: boolean, ignoreMultilineAttribute: any, callback: Function): void {
+		if (configurationSet) {
+			sendNotification("workspace/didChangeConfiguration", {
+				settings: {
+					docker: {
+						languageserver: {
+							formatter: {
+								ignoreMultilineInstructions: ignoreMultilineAttribute
+							}
+						}
+					}
+				}
+			});
+		} else {
+			sendNotification("workspace/didChangeConfiguration", {settings: {}});
+		}
+
+		const documentURI = "uri://dockerfile/" + fileName;
+		sendNotification("textDocument/didOpen", {
+			textDocument: {
+				languageId: "dockerfile",
+				version: 1,
+				uri: documentURI,
+				text: "FROM node AS\\\n build"
+			}
+		});
+
+		const requestId = sendRequest("textDocument/formatting", {
+			textDocument: {
+				uri: documentURI,
+			},
+			options: {
+				insertSpaces: true,
+				tabSize: 4
+			}
+		});
+
+		const listener = (json: any) => {
+			if (json.id === requestId) {
+				lspProcess.removeListener("message", listener);
+				sendNotification("textDocument/didClose", { textDocument: { uri: documentURI } });
+
+				assert.ok(json.result instanceof Array);
+				if (ignoreMultilineAttribute === true) {
+					assert.strictEqual(json.result.length, 0);
+				} else {
+					assert.strictEqual(json.result.length, 1);
+					assert.strictEqual(json.result[0].newText, "    ");
+					assert.strictEqual(json.result[0].range.start.line, 1);
+					assert.strictEqual(json.result[0].range.start.character, 0);
+					assert.strictEqual(json.result[0].range.end.line, 1);
+					assert.strictEqual(json.result[0].range.end.character, 1);
+				}
+				callback();
+			}
+		};
+		lspProcess.on("message", listener);
+	}
+
+	describe("issue #255 workspace configuration", () => {
+		it("workspace configuration not defined", finished => {
+			this.timeout(5000);
+			test255("255-workspace-configuration-not-defined", false, null, finished);
+		});
+
+		it("workspace configuration true", finished => {
+			this.timeout(5000);
+			test255("255-workspace-configuration-true", true, true, finished);
+		});
+
+		it("workspace configuration false", finished => {
+			this.timeout(5000);
+			test255("255-workspace-configuration-false", true, false, finished);
+		});
+	});
+
 	function testInvalidFile(request: string, assertionCallback: Function) {
 		it(request, function(finished) {
 			this.timeout(5000);