Fix #133 Warn about duplicated build stage names

Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Aug 8, 2017 · 6ec0cdf · 6ec0cdf
1 parent 41db25f
commit 6ec0cdf
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
+### Added
+- textDocument/publishDiagnostics
+  - warn about duplicated build stage names ([#133](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/133))
+
 ### Fixed
 - fix completion handling so that the escape parser directive is suggested in more cases ([#138](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/138))
 - always use the first declaration of a variable for its definition ([#141](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/141))

diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -26,6 +26,7 @@ export enum ValidationCode {
 	ARGUMENT_REQUIRES_TWO,
 	ARGUMENT_REQUIRES_ONE_OR_THREE,
 	ARGUMENT_UNNECESSARY,
+	DUPLICATE_NAME,
 	FLAG_AT_LEAST_ONE,
 	FLAG_DUPLICATE,
 	FLAG_INVALID_DURATION,
@@ -198,6 +199,29 @@ export class Validator {
 				}
 			}
 		}
+
+		const names = {};
+		const froms = dockerfile.getFROMs();
+		for (let from of froms) {
+			let name = from.getBuildStage();
+			if (name) {
+				name = name.toLowerCase();
+				if (!names[name]) {
+					names[name] = [];
+				}
+				names[name].push(from.getBuildStageRange());
+			}
+		}
+
+		for (let name in names) {
+			// duplicates found
+			if (names[name].length > 1) {
+				for (let range of names[name]) {
+					problems.push(Validator.createDuplicateName(range, name));
+				}
+			}
+		}
+
 		let hasFrom = false;
 		for (let instruction of dockerfile.getInstructions()) {
 			let keyword = instruction.getKeyword();
@@ -751,6 +775,8 @@ export class Validator {
 		"syntaxMissingEquals": "Syntax error - can't find = in \"${0}\". Must be of the form: name=value",
 		"syntaxMissingNames": "${0} names can not be blank",
 
+		"duplicateName": "duplicate name ${0}",
+
 		"flagAtLeastOne": "${0} must be at least 1 (not ${1})",
 		"flagDuplicate": "Duplicate flag specified: ${0}",
 		"flagInvalidDuration": "time: invalid duration ${0}",
@@ -798,6 +824,10 @@ export class Validator {
 		return Validator.dockerProblems["noSourceImage"];
 	}
 
+	public static getDiagnosticMessage_DuplicateName(name: string) {
+		return Validator.formatMessage(Validator.dockerProblems["duplicateName"], name);
+	}
+
 	public static getDiagnosticMessage_FlagAtLeastOne(flagName: string, flagValue: string) {
 		return Validator.formatMessage(Validator.dockerProblems["flagAtLeastOne"], flagName, flagValue);
 	}
@@ -918,6 +948,10 @@ 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);
+	}
+
 	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);
 	}

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -396,6 +396,17 @@ function assertSyntaxMissingNames(diagnostic: Diagnostic, instrument: string, st
 	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);
+	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
+	assert.equal(diagnostic.source, source);
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_DuplicateName(instrument));
+	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 testValidArgument(instruction: string, argument: string) {
 	let gaps = [ " ", "\t", " \\\n", " \\\r", " \\\r\n" ];
 	for (let gap of gaps) {
@@ -1679,6 +1690,23 @@ describe("Docker Validator Tests", function() {
 				assert.equal(diagnostics.length, 1);
 				assertInvalidAs(diagnostics[0], 0, 10, 0, 12);
 			});
+
+			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);
+
+				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);
+
+				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);
+			});
 		});
 
 		describe("wrong args number", function() {