Fix #96 Warn if HEALTHCHECK CMD has no arguments

For a HEALTHCHECK CMD to run, it must have a command defined for the healthcheck to be performed. If a HEALTHCHECK CMD is defined but it has no arguments following it, a diagnostic will be created to warn the client about this. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Jul 21, 2017 · a879391 · a879391
1 parent ce08c0b
commit a879391
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,6 +39,7 @@ All notable changes to this project will be documented in this file.
     - warn if arguments follow a HEALTHCHECK NONE ([#84](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/84))
     - warn if the retries flag doesn't specify a number ([#85](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/85))
     - warn if the retries flag is not a positive intger ([#89](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/89))
+    - warn if no arguments follow a HEALTHCHECK CMD ([#96](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/96))
 - created a CHANGELOG.md file to document the project's changes ([#77](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/77))
 
 ### Fixed

diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -9,6 +9,7 @@ import { Dockerfile } from './parser/dockerfile';
 import { Flag } from './parser/flag';
 import { Instruction } from './parser/instruction';
 import { Env } from './parser/instructions/env';
+import { Healthcheck } from './parser/instructions/healthcheck';
 import { ModifiableInstruction } from './parser/instructions/modifiableInstruction';
 import { DockerfileParser } from './parser/dockerfileParser';
 import { DIRECTIVE_ESCAPE } from './docker';
@@ -36,7 +37,8 @@ export enum ValidationCode {
 	MULTIPLE_INSTRUCTIONS,
 	UNKNOWN_INSTRUCTION,
 	UNKNOWN_FLAG,
-	DEPRECATED_MAINTAINER
+	DEPRECATED_MAINTAINER,
+	HEALTHCHECK_CMD_ARGUMENT_MISSING
 }
 
 export enum ValidationSeverity {
@@ -275,6 +277,12 @@ export class Validator {
 									}
 									// don't need to validate flags of a NONE
 									break validateInstruction;
+								} else if (uppercase === "CMD") {
+									if (i === args.length - 1) {
+										let range = args[i].getRange();
+										problems.push(Validator.createHealthcheckCmdArgumentMissing(range.start, range.end));
+									}
+									break;
 								}
 							}
 
@@ -413,6 +421,8 @@ export class Validator {
 		"instructionCasing": "Instructions should be written in uppercase letters",
 
 		"deprecatedMaintainer": "MAINTAINER has been deprecated",
+
+		"healthcheckCmdArgumentMissing": "Missing command after HEALTHCHECK CMD",
 	};
 
 	private static formatMessage(text: string, ...variables: string[]): string {
@@ -510,6 +520,10 @@ export class Validator {
 		return Validator.dockerProblems["deprecatedMaintainer"];
 	}
 
+	public static getDiagnosticMessage_HealthcheckCmdArgumentMissing() {
+		return Validator.dockerProblems["healthcheckCmdArgumentMissing"];
+	}
+
 	static createInvalidEscapeDirective(start: Position, end: Position, value: string): Diagnostic {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_DirectiveEscapeInvalid(value), ValidationCode.INVALID_ESCAPE_DIRECTIVE);
 	}
@@ -566,6 +580,10 @@ export class Validator {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_ENVRequiresTwoArguments(), ValidationCode.ARGUMENT_REQUIRES_TWO);
 	}
 
+	private static createHealthcheckCmdArgumentMissing(start: Position, end: Position): Diagnostic {
+		return Validator.createError(start, end, Validator.getDiagnosticMessage_HealthcheckCmdArgumentMissing(), ValidationCode.HEALTHCHECK_CMD_ARGUMENT_MISSING);
+	}
+
 	static createRequiresOneOrThreeArguments(start: Position, end: Position): Diagnostic {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_InstructionRequiresOneOrThreeArguments(), ValidationCode.ARGUMENT_REQUIRES_ONE_OR_THREE);
 	}

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -209,6 +209,17 @@ function assertInstructionRequiresOneArgument(diagnostic: Diagnostic, startLine:
 	assert.equal(diagnostic.range.end.character, endCharacter);
 }
 
+function assertHealthcheckCmdArgumentMissing(diagnostic: Diagnostic, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+	assert.equal(diagnostic.code, ValidationCode.HEALTHCHECK_CMD_ARGUMENT_MISSING);
+	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
+	assert.equal(diagnostic.source, source);
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_HealthcheckCmdArgumentMissing());
+	assert.equal(diagnostic.range.start.line, startLine);
+	assert.equal(diagnostic.range.start.character, startCharacter);
+	assert.equal(diagnostic.range.end.line, endLine);
+	assert.equal(diagnostic.range.end.character, endCharacter);
+}
+
 function assertENVRequiresTwoArguments(diagnostic: Diagnostic, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
 	assert.equal(diagnostic.code, ValidationCode.ARGUMENT_REQUIRES_TWO);
 	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
@@ -1551,6 +1562,32 @@ describe("Docker Validator Tests", function() {
 					assert.equal(diagnostics.length, 1);
 					assertFlagAtLeastOne(diagnostics[0], "--retries", "-1", 1, 22, 1, 24);
 				});
+
+				it("CMD no arguments", function() {
+					let diagnostics = validate("FROM alpine\nHEALTHCHECK CMD");
+					assert.equal(diagnostics.length, 1);
+					assertHealthcheckCmdArgumentMissing(diagnostics[0], 1, 12, 1, 15);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK CMD ");
+					assert.equal(diagnostics.length, 1);
+					assertHealthcheckCmdArgumentMissing(diagnostics[0], 1, 12, 1, 15);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK CMD \n");
+					assert.equal(diagnostics.length, 1);
+					assertHealthcheckCmdArgumentMissing(diagnostics[0], 1, 12, 1, 15);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK CMD CMD");
+					assert.equal(diagnostics.length, 0);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK CMD cmd");
+					assert.equal(diagnostics.length, 0);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK cmd CMD");
+					assert.equal(diagnostics.length, 0);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK cmd cmd");
+					assert.equal(diagnostics.length, 0);
+				});
 			});
 		});