Fix #89 Warn if --retries of HEALTHCHECK is not positive

The retries flag of a HEALTHCHECK specifies the number of times that
the HEALTHCHECK CMD should be run before the container is considered
to be `unhealthy`. As such, the retries flag should be a positive
integer as negative values or "zero retries" don't make any sense. A
diagnostic will now be created to handle this error in Dockerfiles.

Signed-off-by: Remy Suen <[email protected]>

CHANGELOG.md

@@ -28,6 +28,7 @@ All notable changes to this project will be documented in this file.
   - HEALTHCHECK
     - 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))
 - created a CHANGELOG.md file to document the project's changes ([#77](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/77))
 
 ### Fixed

src/dockerValidator.ts

@@ -21,6 +21,7 @@ export enum ValidationCode {
 	ARGUMENT_REQUIRES_TWO,
 	ARGUMENT_REQUIRES_ONE_OR_THREE,
 	ARGUMENT_UNNECESSARY,
+	FLAG_AT_LEAST_ONE,
 	NO_SOURCE_IMAGE,
 	INVALID_ESCAPE_DIRECTIVE,
 	INVALID_AS,
@@ -276,11 +277,16 @@ export class Validator {
 								if (diagnostic === null) {
 									if (value.indexOf("--retries=") === 0) {
 										value = value.substring(10);
+										let integer = parseInt(value);
 										// test for NaN or numbers with decimals
-										if (isNaN(parseInt(value)) || value.indexOf('.') !== -1) {
+										if (isNaN(integer) || value.indexOf('.') !== -1) {
 											let start = Position.create(range.start.line, range.start.character + 10);
 											let end = Position.create(range.end.line, range.end.character);
 											problems.push(Validator.createInvalidSyntax(start, end, value));
+										} else if (integer < 1) {
+											let start = Position.create(range.start.line, range.start.character + 10);
+											let end = Position.create(range.end.line, range.end.character);
+											problems.push(Validator.createFlagAtLeastOne(start, end, "--retries", integer.toString()));
 										}
 									}
 								} else {
@@ -365,7 +371,8 @@ export class Validator {
 		"invalidSyntax": "parsing \"${0}\": invalid syntax",
 
 		"syntaxMissingEquals": "Syntax error - can't find = in \"${0}\". Must be of the form: name=value",
-
+
+		"flagAtLeastOne": "${0} must be at least 1 (not ${1})",
 		"flagUnknown": "Unknown flag: ${0}",
 
 		"instructionExtraArgument": "Instruction has an extra argument",
@@ -380,8 +387,11 @@ export class Validator {
 		"deprecatedMaintainer": "MAINTAINER has been deprecated",
 	};
 
-	private static formatMessage(text: string, variable: string): string {
-		return text.replace("${0}", variable);
+	private static formatMessage(text: string, ...variables: string[]): string {
+		for (let i = 0; i < variables.length; i++) {
+			text = text.replace("${" + i + "}", variables[i]);
+		}
+		return text;
 	}
 
 	public static getDiagnosticMessage_DirectiveCasing() {
@@ -396,6 +406,10 @@ export class Validator {
 		return Validator.dockerProblems["noSourceImage"];
 	}
 
+	public static getDiagnosticMessage_FlagAtLeastOne(flagName: string, flagValue: string) {
+		return Validator.formatMessage(Validator.dockerProblems["flagAtLeastOne"], flagName, flagValue);
+	}
+
 	public static getDiagnosticMessage_FlagUnknown(flag: string) {
 		return Validator.formatMessage(Validator.dockerProblems["flagUnknown"], flag);
 	}
@@ -464,6 +478,10 @@ export class Validator {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_DirectiveEscapeInvalid(value), ValidationCode.INVALID_ESCAPE_DIRECTIVE);
 	}
 
+	static createFlagAtLeastOne(start: Position, end: Position, flagName: string, flagValue: string): Diagnostic {
+		return Validator.createError(start, end, Validator.getDiagnosticMessage_FlagAtLeastOne(flagName, flagValue), ValidationCode.FLAG_AT_LEAST_ONE);
+	}
+
 	static createFlagUnknown(start: Position, end: Position, flag: string): Diagnostic {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_FlagUnknown(flag), ValidationCode.UNKNOWN_FLAG);
 	}

test/dockerValidator.test.ts

@@ -54,6 +54,17 @@ function assertNoSourceImage(diagnostic: Diagnostic, startLine: number, startCha
 	assert.equal(diagnostic.range.end.character, endCharacter);
 }
 
+function assertFlagAtLeastOne(diagnostic: Diagnostic, flagName: string, flagValue: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+	assert.equal(diagnostic.code, ValidationCode.FLAG_AT_LEAST_ONE);
+	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
+	assert.equal(diagnostic.source, source);
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_FlagAtLeastOne(flagName, flagValue));
+	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 assertFlagUnknown(diagnostic: Diagnostic, flag: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
 	assert.equal(diagnostic.code, ValidationCode.UNKNOWN_FLAG);
 	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
@@ -1466,6 +1477,16 @@ describe("Docker Validator Tests", function() {
 					assert.equal(diagnostics.length, 1);
 					assertInvalidSyntax(diagnostics[0], "1.0", 1, 22, 1, 25);
 				});
+
+				it("--retries at least one", function() {
+					let diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=0 CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagAtLeastOne(diagnostics[0], "--retries", "0", 1, 22, 1, 23);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=-1 CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagAtLeastOne(diagnostics[0], "--retries", "-1", 1, 22, 1, 24);
+				});
 			});
 		});