Fix #132 Warn about invalid build stage names in FROM

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

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file.
 ### Added
 - textDocument/publishDiagnostics
   - warn if ENV or LABEL is missing closing quote ([#143](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/143))
+  - warn if FROM's build stage name is invalid ([#132](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/132))
 
 ### Fixed
 - correct handling of escaped quotes in ENV variables ([#144](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/144))

src/dockerValidator.ts

@@ -26,7 +26,8 @@ export enum ValidationCode {
 	ARGUMENT_REQUIRES_TWO,
 	ARGUMENT_REQUIRES_ONE_OR_THREE,
 	ARGUMENT_UNNECESSARY,
-	DUPLICATE_NAME,
+	DUPLICATE_BUILD_STAGE_NAME,
+	INVALID_BUILD_STAGE_NAME,
 	FLAG_AT_LEAST_ONE,
 	FLAG_DUPLICATE,
 	FLAG_INVALID_DURATION,
@@ -140,10 +141,12 @@ export class Validator {
 			for (let i = 0; i < expectedArgCount.length; i++) {
 				if (expectedArgCount[i] === args.length) {
 					for (let j = 0; j < args.length; j++) {
-						let createInvalidDiagnostic = validate(j, args[j].getValue());
-						if (createInvalidDiagnostic) {
-							let range = args[j].getRange();
-							problems.push(createInvalidDiagnostic(range.start, range.end, args[i].getValue()));
+						let range = args[j].getRange();
+						let createInvalidDiagnostic = validate(j, args[j].getValue(), range);
+						if (createInvalidDiagnostic instanceof Function) {
+							problems.push(createInvalidDiagnostic(range.start, range.end, args[j].getValue()));
+						} else if (createInvalidDiagnostic !== null) {
+							problems.push(createInvalidDiagnostic);
 						}
 					}
 					return;
@@ -219,7 +222,7 @@ export class Validator {
 			// duplicates found
 			if (names[name].length > 1) {
 				for (let range of names[name]) {
-					problems.push(Validator.createDuplicateName(range, name));
+					problems.push(Validator.createDuplicateBuildStageName(range, name));
 				}
 			}
 		}
@@ -327,11 +330,20 @@ export class Validator {
 					}
 					break;
 				case "FROM":
-					this.checkArguments(instruction, problems, [ 1, 3 ], function(index: number, argument: string) {
-						if (index === 1) {
-							return argument.toUpperCase() === "AS" ? null : Validator.createInvalidAs;
+					this.checkArguments(instruction, problems, [ 1, 3 ], function(index: number, argument: string, range: Range) {
+						switch (index) {
+							case 1:
+								return argument.toUpperCase() === "AS" ? null : Validator.createInvalidAs;
+							case 2:
+								argument = argument.toLowerCase();
+								let regexp = new RegExp(/^[a-z]([a-z0-9_\-.]*)*$/);
+								if (regexp.test(argument)) {
+									return null;
+								}
+								return Validator.createInvalidBuildStageName(range, argument);;
+							default:
+								return null;
 						}
-						return null;
 					}, Validator.createRequiresOneOrThreeArguments);
 					break;
 				case "HEALTHCHECK":
@@ -804,7 +816,8 @@ export class Validator {
 		"syntaxMissingSingleQuote": "failed to process \"${0}\": unexpected end of statement while looking for matching single-quote",
 		"syntaxMissingDoubleQuote": "failed to process \"${0}\": unexpected end of statement while looking for matching double-quote",
 
-		"duplicateName": "duplicate name ${0}",
+		"duplicateBuildStageName": "duplicate name ${0}",
+		"invalidBuildStageName": "invalid name for build stage: \"${0}\", name can't start with a number or contain symbols",
 
 		"flagAtLeastOne": "${0} must be at least 1 (not ${1})",
 		"flagDuplicate": "Duplicate flag specified: ${0}",
@@ -853,9 +866,13 @@ export class Validator {
 		return Validator.dockerProblems["noSourceImage"];
 	}
 
-	public static getDiagnosticMessage_DuplicateName(name: string) {
-		return Validator.formatMessage(Validator.dockerProblems["duplicateName"], name);
+	public static getDiagnosticMessage_DuplicateBuildStageName(name: string) {
+		return Validator.formatMessage(Validator.dockerProblems["duplicateBuildStageName"], name);
 	}
+
+		public static getDiagnosticMessage_InvalidBuildStageName(name: string) {
+			return Validator.formatMessage(Validator.dockerProblems["invalidBuildStageName"], name);
+		}
 
 	public static getDiagnosticMessage_FlagAtLeastOne(flagName: string, flagValue: string) {
 		return Validator.formatMessage(Validator.dockerProblems["flagAtLeastOne"], flagName, flagValue);
@@ -985,8 +1002,12 @@ export class Validator {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_DirectiveEscapeInvalid(value), ValidationCode.INVALID_ESCAPE_DIRECTIVE);
 	}
 
-	private static createDuplicateName(range: Range, name: string): Diagnostic {
-		return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_DuplicateName(name), ValidationCode.DUPLICATE_NAME);
+	private static createDuplicateBuildStageName(range: Range, name: string): Diagnostic {
+		return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_DuplicateBuildStageName(name), ValidationCode.DUPLICATE_BUILD_STAGE_NAME);
+	}
+
+	private static createInvalidBuildStageName(range: Range, name: string): Diagnostic {
+		return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_InvalidBuildStageName(name), ValidationCode.INVALID_BUILD_STAGE_NAME);
 	}
 
 	static createFlagAtLeastOne(start: Position, end: Position, flagName: string, flagValue: string): Diagnostic {

test/dockerValidator.test.ts

@@ -418,11 +418,22 @@ function assertSyntaxMissingDoubleQuote(diagnostic: Diagnostic, key: string, sta
 	assert.equal(diagnostic.range.end.character, endCharacter);
 }
 
-function assertDuplicateName(diagnostic: Diagnostic, instrument: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
-	assert.equal(diagnostic.code, ValidationCode.DUPLICATE_NAME);
+function assertDuplicateBuildStageName(diagnostic: Diagnostic, name: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+	assert.equal(diagnostic.code, ValidationCode.DUPLICATE_BUILD_STAGE_NAME);
 	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
 	assert.equal(diagnostic.source, source);
-	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_DuplicateName(instrument));
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_DuplicateBuildStageName(name));
+	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 assertInvalidBuildStageName(diagnostic: Diagnostic, name: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+	assert.equal(diagnostic.code, ValidationCode.INVALID_BUILD_STAGE_NAME);
+	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
+	assert.equal(diagnostic.source, source);
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_InvalidBuildStageName(name));
 	assert.equal(diagnostic.range.start.line, startLine);
 	assert.equal(diagnostic.range.start.character, startCharacter);
 	assert.equal(diagnostic.range.end.line, endLine);
@@ -1759,6 +1770,9 @@ describe("Docker Validator Tests", function() {
 
 				diagnostics = validate("FROM node AS \\ \n setup");
 				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM node AS a_lpi-n.e99");
+				assert.equal(diagnostics.length, 0);
 			});
 
 			it("invalid as", function() {
@@ -1770,18 +1784,32 @@ describe("Docker Validator Tests", function() {
 			it("duplicate name", function() {
 				let diagnostics = validate("FROM node AS setup\nFROM node AS setup");
 				assert.equal(diagnostics.length, 2);
-				assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18);
-				assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18);
+				assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18);
+				assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18);
 
 				diagnostics = validate("FROM node AS setup\nFROM node AS setUP");
 				assert.equal(diagnostics.length, 2);
-				assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18);
-				assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18);
+				assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18);
+				assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18);
 
 				diagnostics = validate("FROM node AS SETUP\nFROM node AS seTUp");
 				assert.equal(diagnostics.length, 2);
-				assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18);
-				assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18);
+				assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18);
+				assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18);
+			});
+
+			it("invalid name", function() {
+				let diagnostics = validate("FROM node AS 1s");
+				assert.equal(diagnostics.length, 1);
+				assertInvalidBuildStageName(diagnostics[0], "1s", 0, 13, 0, 15);
+
+				diagnostics = validate("FROM node AS _s");
+				assert.equal(diagnostics.length, 1);
+				assertInvalidBuildStageName(diagnostics[0], "_s", 0, 13, 0, 15);
+
+				diagnostics = validate("FROM node AS a_lpi-n.e,99");
+				assert.equal(diagnostics.length, 1);
+				assertInvalidBuildStageName(diagnostics[0], "a_lpi-n.e,99", 0, 13, 0, 25);
 			});
 		});