Fix #157 Warn if COPY does not have enough arguments

COPY instructions should have at least two arguments. If this case is detected by the validator, a diagnostic will now be published to notify the client about this. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Aug 27, 2017 · 62f2232 · 62f2232
1 parent 79f3c04
commit 62f2232
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
   - 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))
   - warn if an invalid unit of time is used in a duration flag ([#152](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/152))
+  - warn if COPY does not have two arguments ([#157](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/157))
 - textDocument/signatureHelp
   - escape parser directive ([#147](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/147))
   - instruction flags ([#147](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/147))

diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -24,6 +24,7 @@ export enum ValidationCode {
 	ARGUMENT_EXTRA,
 	ARGUMENT_REQUIRES_ONE,
 	ARGUMENT_REQUIRES_TWO,
+	ARGUMENT_REQUIRES_AT_LEAST_TWO,
 	ARGUMENT_REQUIRES_ONE_OR_THREE,
 	ARGUMENT_UNNECESSARY,
 	DUPLICATE_BUILD_STAGE_NAME,
@@ -473,18 +474,25 @@ export class Validator {
 					break;
 				case "COPY":
 					let copyArgs = instruction.getArguments();
-					if (copyArgs.length === 0) {
-						let range = instruction.getInstructionRange();
-						problems.push(Validator.createMissingArgument(range.start, range.end));
-					} else {
-						let flags = (instruction as ModifiableInstruction).getFlags();
-						if (flags.length > 0 && flags[0].getName() !== "from") {
+					let flags = (instruction as ModifiableInstruction).getFlags();
+					if (flags.length > 0) {
+						if (flags[0].getName() !== "from") {
 							let range = flags[0].getNameRange();
 							problems.push(Validator.createFlagUnknown(range.start, range.end, flags[0].getName()));
 						}
-						this.checkFlagValue(flags, [ "from" ], problems);
-						this.checkDuplicateFlags(flags, [ "from" ], problems);
+
+						if (copyArgs.length == 2) {
+							problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(copyArgs[1].getRange()));
+						} else if (copyArgs.length == 1) {
+							problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(instruction.getInstructionRange()));
+						}
+					} else if (copyArgs.length === 1) {
+						problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(copyArgs[0].getRange()));
+					} else if (copyArgs.length === 0) {
+						problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(instruction.getInstructionRange()));
 					}
+					this.checkFlagValue(flags, [ "from" ], problems);
+					this.checkDuplicateFlags(flags, [ "from" ], problems);
 					break;
 				default:
 					this.checkArguments(instruction, problems, [ -1 ], function() {
@@ -866,6 +874,7 @@ export class Validator {
 		"instructionMissingArgument": "Instruction has no arguments",
 		"instructionMultiple": "There can only be one ${0} instruction in a Dockerfile",
 		"instructionRequiresOneArgument": "${0} requires exactly one argument",
+		"instructionRequiresAtLeastTwoArguments": "${0} requires at least two arguments",
 		"instructionRequiresTwoArguments": "${0} must have two arguments",
 		"instructionUnnecessaryArgument": "${0} takes no arguments",
 		"instructionUnknown": "Unknown instruction: ${0}",
@@ -973,6 +982,10 @@ export class Validator {
 		return Validator.formatMessage(Validator.dockerProblems["instructionRequiresOneArgument"], "ARG");
 	}
 
+	public static getDiagnosticMessage_COPYRequiresAtLeastTwoArguments() {
+		return Validator.formatMessage(Validator.dockerProblems["instructionRequiresAtLeastTwoArguments"], "COPY");
+	}
+
 	public static getDiagnosticMessage_ENVRequiresTwoArguments() {
 		return Validator.formatMessage(Validator.dockerProblems["instructionRequiresTwoArguments"], "ENV");
 	}
@@ -1117,6 +1130,10 @@ export class Validator {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_ARGRequiresOneArgument(), ValidationCode.ARGUMENT_REQUIRES_ONE);
 	}
 
+	private static createCOPYRequiresAtLeastTwoArguments(range: Range): Diagnostic {
+		return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_COPYRequiresAtLeastTwoArguments(), ValidationCode.ARGUMENT_REQUIRES_AT_LEAST_TWO);
+	}
+
 	static createENVRequiresTwoArguments(start: Position, end: Position): Diagnostic {
 		return Validator.createError(start, end, Validator.getDiagnosticMessage_ENVRequiresTwoArguments(), ValidationCode.ARGUMENT_REQUIRES_TWO);
 	}

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -275,6 +275,17 @@ function assertHealthcheckCmdArgumentMissing(diagnostic: Diagnostic, startLine:
 	assert.equal(diagnostic.range.end.character, endCharacter);
 }
 
+function assertCOPYRequiresAtLeastTwoArguments(diagnostic: Diagnostic, startLine: number, startCharacter: number, endLine: number, endCharacter: number) {
+	assert.equal(diagnostic.code, ValidationCode.ARGUMENT_REQUIRES_AT_LEAST_TWO);
+	assert.equal(diagnostic.severity, DiagnosticSeverity.Error);
+	assert.equal(diagnostic.source, source);
+	assert.equal(diagnostic.message, Validator.getDiagnosticMessage_COPYRequiresAtLeastTwoArguments());
+	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);
@@ -839,27 +850,30 @@ describe("Docker Validator Tests", function() {
 		});
 
 		describe("missing argument", function() {
-			function testMissingArgument(instruction: string, prefix: string, middle: string, suffix: string, safe?: boolean) {
+			function testMissingArgument(instruction: string, prefix: string, middle: string, suffix: string, safe?: boolean, assertFunction?: Function) {
 				var length = instruction.length;
 				let diagnostics = validate("FROM node\n" + instruction + prefix + middle + suffix);
 				if (safe) {
 					assert.equal(diagnostics.length, 0);
+				} else if (assertFunction) {
+					assert.equal(diagnostics.length, 1);
+					assertFunction(diagnostics[0], 1, 0, 1, length);
 				} else {
 					assert.equal(diagnostics.length, 1);
 					assertInstructionMissingArgument(diagnostics[0], 1, 0, 1, length);
 				}
 			}
 
-			function testMissingArgumentLoop(instruction: string, safe?: boolean) {
+			function testMissingArgumentLoop(instruction: string, safe?: boolean, assertFunction?: Function) {
 				let newlines = [ "", "\r", "\n", "\r\n", "\\\r", "\\\n", "\\\r\n" ];
 				for (let newline of newlines) {
-					testMissingArgument(instruction, "", newline, "", safe);
-					testMissingArgument(instruction, "", newline, " ", safe);
-					testMissingArgument(instruction, " ", newline, "", safe);
-					testMissingArgument(instruction, " ", newline, " ", safe);
-					testMissingArgument(instruction, "", newline, "\t", safe);
-					testMissingArgument(instruction, "\t", newline, "", safe);
-					testMissingArgument(instruction, "\t", newline, "\t", safe);
+					testMissingArgument(instruction, "", newline, "", safe, assertFunction);
+					testMissingArgument(instruction, "", newline, " ", safe, assertFunction);
+					testMissingArgument(instruction, " ", newline, "", safe, assertFunction);
+					testMissingArgument(instruction, " ", newline, " ", safe, assertFunction);
+					testMissingArgument(instruction, "", newline, "\t", safe, assertFunction);
+					testMissingArgument(instruction, "\t", newline, "", safe, assertFunction);
+					testMissingArgument(instruction, "\t", newline, "\t", safe, assertFunction);
 				}
 			}
 
@@ -876,7 +890,7 @@ describe("Docker Validator Tests", function() {
 			});
 
 			it("COPY", function() {
-				return testMissingArgumentLoop("COPY");
+				return testMissingArgumentLoop("COPY", false, assertCOPYRequiresAtLeastTwoArguments);
 			});
 
 			it("ENTRYPOINT", function() {
@@ -1416,6 +1430,31 @@ describe("Docker Validator Tests", function() {
 	});
 
 	describe("COPY", function() {
+		describe("arguments", function() {
+			it("ok", function() {
+				let diagnostics = validate("FROM alpine\nCOPY . .");
+				assert.equal(diagnostics.length, 0);
+			});
+
+			it("requires at least two", function() {
+				let diagnostics = validate("FROM alpine\nCOPY ");
+				assert.equal(diagnostics.length, 1);
+				assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 0, 1, 4);
+
+				diagnostics = validate("FROM alpine\nCOPY .");
+				assert.equal(diagnostics.length, 1);
+				assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 5, 1, 6);
+
+				diagnostics = validate("FROM alpine\nCOPY --from=busybox");
+				assert.equal(diagnostics.length, 1);
+				assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 0, 1, 4);
+
+				diagnostics = validate("FROM alpine\nCOPY --from=busybox .");
+				assert.equal(diagnostics.length, 1);
+				assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 20, 1, 21);
+			});
+		});
+
 		describe("build stages", function() {
 			it("ok", function() {
 				let diagnostics = validate("FROM alpine\nFROM busybox AS bb\nCOPY --from=bb . .");