Fix #160 Suggest numbered build stage indices

If a build stage is unnamed, it is possible to use a numerical index that corresponds to its position in the Dockerfile instead. Such numbers should be suggested when the client sends a textDocument/completion request for valid arguments to a COPY's --from flag. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Aug 23, 2017 · bef6839 · bef6839
1 parent cad55bc
commit bef6839
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
   - add '-' as a trigger character to suggest instruction flags ([#155](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/155))
   - suggest image tags in FROM instructions ([#154](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/154))
   - set source image as suggested build stage's documentation text ([#159](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/159))
+  - suggest numeric build stage index if source image is unnamed ([#160](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/160))
 - textDocument/hover
   - COPY's --from build stage flag ([#150](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/150))
 - textDocument/publishDiagnostics

diff --git a/src/dockerAssist.ts b/src/dockerAssist.ts
@@ -245,29 +245,39 @@ export class DockerAssist {
 		let range = copy.getFromValueRange();
 		// is the user in the --from= area
 		if (range && Util.isInsideRange(position, copy.getFromValueRange())) {
-			const names = {};
-			const images = {};
+			const names: { [key: string]: boolean; } = {};
+			const items: CompletionItem[] = [];
+			let stageIndex = 0;
+			// determines if the last build stage was named or not
+			let lastNumber = false;
 			// get the prefix
 			let stagePrefix = this.document.getText().substring(this.document.offsetAt(range.start), offset).toLowerCase();
 			for (let from of dockerfile.getFROMs()) {
 				if (copy.isAfter(from)) {
+					const image = from.getImage();
 					let stage = from.getBuildStage();
 					if (stage) {
 						const lowercase = stage.toLowerCase();
 						if (lowercase.indexOf(stagePrefix) === 0 && !names[lowercase]) {
-							names[lowercase] = stage;
-							images[lowercase] = from.getImage();
+							names[lowercase] = true;
+							items.push(this.createSourceImageCompletionItem(stage, image, stageIndex, stagePrefix.length, offset));
 						}
+						lastNumber = false;
+					} else if (!names[stageIndex]) {
+						names[stageIndex] = true;
+						items.push(this.createSourceImageCompletionItem(stageIndex.toString(), image, stageIndex, stagePrefix.length, offset));
+						lastNumber = true;
 					}
+					stageIndex++;
+				} else {
+					break;
 				}
 			}
-			const items: CompletionItem[] = [];
-			for (const name in names) {
-				items.push(this.createSourceImageCompletionItem(names[name], images[name], stagePrefix.length, offset));
+
+			// last build stage was not named, don't suggest it as it is recursive
+			if (lastNumber && items.length > 0) {
+				items.pop();
 			}
-			items.sort((item: CompletionItem, item2: CompletionItem) => {
-				return item.label.localeCompare(item2.label);
-			});
 			return items;
 		}
 
@@ -598,13 +608,14 @@ export class DockerAssist {
 		};
 	}
 
-	private createSourceImageCompletionItem(label: string, documentation: string, prefixLength: number, offset: number): CompletionItem {
+	private createSourceImageCompletionItem(label: string, documentation: string, buildIndex: number, prefixLength: number, offset: number): CompletionItem {
 		return {
 			textEdit: this.createTextEdit(prefixLength, offset, label),
 			label: label,
 			documentation: documentation,
 			kind: CompletionItemKind.Reference,
 			insertTextFormat: InsertTextFormat.PlainText,
+			sortText: buildIndex.toString()
 		};
 	}
 

diff --git a/test/dockerAssist.test.ts b/test/dockerAssist.test.ts
@@ -529,10 +529,11 @@ function assertWORKDIR(item: CompletionItem, line: number, character: number, pr
 	assert.equal(item.textEdit.range.end.character, character + prefixLength);
 }
 
-function assertSourceImage(item: CompletionItem, sourceImage: string, documentation: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+function assertSourceImage(item: CompletionItem, sourceImage: string, buildIndex: number, documentation: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
 	assert.equal(item.label, sourceImage);
 	assert.equal(item.kind, CompletionItemKind.Reference);
 	assert.equal(item.insertTextFormat, InsertTextFormat.PlainText);
+	assert.equal(item.sortText, buildIndex.toString());
 	assert.equal(item.documentation, documentation);
 	assert.equal(item.textEdit.newText, sourceImage);
 	assert.equal(item.textEdit.range.start.line, startLine);
@@ -1551,43 +1552,58 @@ describe('Docker Content Assist Tests', function() {
 				it("source image", function() {
 					var proposals = computePosition("FROM busybox AS source\n" + onbuild + "COPY --from=", 1, triggerOffset + 12);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "source", "busybox", 1, triggerOffset + 12, 1, triggerOffset + 12);
+					assertSourceImage(proposals[0], "source", 0, "busybox", 1, triggerOffset + 12, 1, triggerOffset + 12);
 				});
 
 				it("source images alphabetical", function() {
 					var proposals = computePosition("FROM ubuntu:trusty AS setup\nFROM redhat:rawhide AS dev\n" + onbuild + "COPY --from=", 2, triggerOffset + 12);
 					assert.equal(proposals.length, 2);
-					assertSourceImage(proposals[0], "dev", "redhat:rawhide", 2, triggerOffset + 12, 2, triggerOffset + 12);
-					assertSourceImage(proposals[1], "setup", "ubuntu:trusty", 2, triggerOffset + 12, 2, triggerOffset + 12);
+					assertSourceImage(proposals[0], "setup", 0, "ubuntu:trusty", 2, triggerOffset + 12, 2, triggerOffset + 12);
+					assertSourceImage(proposals[1], "dev", 1, "redhat:rawhide", 2, triggerOffset + 12, 2, triggerOffset + 12);
 				});
 
 				it("source image prefix", function() {
 					var proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\n" + onbuild + "COPY --from=s", 2, triggerOffset + 13);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "setup", "busybox", 2, triggerOffset + 12, 2, triggerOffset + 13);
+					assertSourceImage(proposals[0], "setup", 0, "busybox", 2, triggerOffset + 12, 2, triggerOffset + 13);
 
 					// casing should be ignored
 					proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\n" + onbuild + "COPY --from=S", 2, triggerOffset + 13);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "setup", "busybox", 2, triggerOffset + 12, 2, triggerOffset + 13);
+					assertSourceImage(proposals[0], "setup", 0, "busybox", 2, triggerOffset + 12, 2, triggerOffset + 13);
 				});
 
 				it("no duplicate source images", function() {
 					let proposals = computePosition("FROM busybox AS source\nFROM busybox AS source\n" + onbuild + "COPY --from=", 2, triggerOffset + 12);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "source", "busybox", 2, triggerOffset + 12, 2, triggerOffset + 12);
+					assertSourceImage(proposals[0], "source", 0, "busybox", 2, triggerOffset + 12, 2, triggerOffset + 12);
 				});
 
 				it("no duplicate source images ignoring case", function() {
 					let proposals = computePosition("FROM busybox AS source\nFROM busybox AS soURCe\n" + onbuild + "COPY --from=", 2, triggerOffset + 12);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "source", "busybox", 2, triggerOffset + 12, 2, triggerOffset + 12);
+					assertSourceImage(proposals[0], "source", 0, "busybox", 2, triggerOffset + 12, 2, triggerOffset + 12);
 				});
 
 				it("only suggest previously declared source images", function() {
 					let proposals = computePosition("FROM node AS dev\n" + onbuild + "COPY --from=\nFROM node AS test", 1, triggerOffset + 12);
 					assert.equal(proposals.length, 1);
-					assertSourceImage(proposals[0], "dev", "node", 1, triggerOffset + 12, 1, triggerOffset + 12);
+					assertSourceImage(proposals[0], "dev", 0, "node", 1, triggerOffset + 12, 1, triggerOffset + 12);
+				});
+
+				it("source image indices", function() {
+					let proposals = computePosition("FROM alpine\nFROM node\n" + onbuild + "COPY --from=\nFROM busybox", 2, triggerOffset + 12);
+					assert.equal(proposals.length, 1);
+					assertSourceImage(proposals[0], "0", 0, "alpine", 2, triggerOffset + 12, 2, triggerOffset + 12);
+
+					proposals = computePosition("FROM alpine AS linux\nFROM node\nFROM busybox\n" + onbuild + "COPY --from=", 3, triggerOffset + 12);
+					assert.equal(proposals.length, 2);
+					assertSourceImage(proposals[0], "linux", 0, "alpine", 3, triggerOffset + 12, 3, triggerOffset + 12);
+					assertSourceImage(proposals[1], "1", 1, "node", 3, triggerOffset + 12, 3, triggerOffset + 12);
+
+					proposals = computePosition("FROM alpine AS 1\nFROM node\nFROM busybox\n" + onbuild + "COPY --from=", 3, triggerOffset + 12);
+					assert.equal(proposals.length, 1);
+					assertSourceImage(proposals[0], "1", 0, "alpine", 3, triggerOffset + 12, 3, triggerOffset + 12);
 				});
 			});