Fix #129 Only consider parser directives if at the top

Dockerfile parser directives must be declared at the top. If an instruction, comment, or empty line is encountered then the parser should not try searching or validating for parser directives anymore. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Aug 4, 2017 · b51f546 · b51f546
1 parent 5120455
commit b51f546
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ All notable changes to this project will be documented in this file.
 - prevent undeclared variables from being suggested as completion items ([#118](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/118))
 - prevent completion items from being suggested in multiline instructions ([#125](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/125))
 - handle TCP and UDP in an EXPOSE instruction's argument ([#123](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/123))
+- only search for parser directives at the top of a Dockerfile ([#129](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/129))
 
 ## [0.0.2] - 2017-07-31
 ### Added

diff --git a/src/parser/dockerfileParser.ts b/src/parser/dockerfileParser.ts
@@ -65,9 +65,11 @@ export class DockerfileParser {
 			switch (buffer.charAt(i)) {
 				case ' ':
 				case '\t':
+					break;
 				case '\r':
 				case '\n':
-					break;
+					// parser directives must be at the top of the Dockerfile
+					break directiveCheck;
 				case '#':
 					let commentStart = i;
 					let directiveStart = -1;

diff --git a/test/dockerSymbols.test.ts b/test/dockerSymbols.test.ts
@@ -146,42 +146,20 @@ describe("Dockerfile document symbols", function () {
 			assert.equal(symbols[0].location.range.start.character, 2);
 			assert.equal(symbols[0].location.range.end.line, 0);
 			assert.equal(symbols[0].location.range.end.character, 8);
+		});
 
-			document = createDocument("\r#escape=`");
-			symbols = symbolsProvider.parseSymbolInformation(document, uri);
-			assert.equal(symbols.length, 1);
-			assert.equal(symbols[0].containerName, undefined);
-			assert.equal(symbols[0].name, "escape");
-			assert.equal(symbols[0].kind, SymbolKind.Property);
-			assert.equal(symbols[0].location.uri, uri);
-			assert.equal(symbols[0].location.range.start.line, 1);
-			assert.equal(symbols[0].location.range.start.character, 1);
-			assert.equal(symbols[0].location.range.end.line, 1);
-			assert.equal(symbols[0].location.range.end.character, 7);
+		it("directive becomes a comment if not at the top", function() {
+			let document = createDocument("\r#escape=`");
+			let symbols = symbolsProvider.parseSymbolInformation(document, uri);
+			assert.equal(symbols.length, 0);
 
 			document = createDocument("\n#escape=`");
 			symbols = symbolsProvider.parseSymbolInformation(document, uri);
-			assert.equal(symbols.length, 1);
-			assert.equal(symbols[0].containerName, undefined);
-			assert.equal(symbols[0].name, "escape");
-			assert.equal(symbols[0].kind, SymbolKind.Property);
-			assert.equal(symbols[0].location.uri, uri);
-			assert.equal(symbols[0].location.range.start.line, 1);
-			assert.equal(symbols[0].location.range.start.character, 1);
-			assert.equal(symbols[0].location.range.end.line, 1);
-			assert.equal(symbols[0].location.range.end.character, 7);
+			assert.equal(symbols.length, 0);
 
 			document = createDocument("\r\n#escape=`");
 			symbols = symbolsProvider.parseSymbolInformation(document, uri);
-			assert.equal(symbols.length, 1);
-			assert.equal(symbols[0].containerName, undefined);
-			assert.equal(symbols[0].name, "escape");
-			assert.equal(symbols[0].kind, SymbolKind.Property);
-			assert.equal(symbols[0].location.uri, uri);
-			assert.equal(symbols[0].location.range.start.line, 1);
-			assert.equal(symbols[0].location.range.start.character, 1);
-			assert.equal(symbols[0].location.range.end.line, 1);
-			assert.equal(symbols[0].location.range.end.character, 7);
+			assert.equal(symbols.length, 0);
 		});
 
 		it("invalid directive definition with leading comment", function () {

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -1219,6 +1219,11 @@ describe("Docker Validator Tests", function() {
 				assert.equal(diagnostics.length, 1);
 				assertNoSourceImage(diagnostics[0], 0, 0, 0, 0);
 			});
+
+			it("ignored on second line", function() {
+				let diagnostics = validate("\n# escape=a\nFROM node");
+				assert.equal(diagnostics.length, 0);
+			});
 		});
 
 		describe("escape casing", function() {
@@ -1262,6 +1267,11 @@ describe("Docker Validator Tests", function() {
 				assert.equal(diagnostics.length, 1);
 				assertDirectiveCasing(diagnostics[0], DiagnosticSeverity.Error, 0, 2, 0, 8);
 			});
+
+			it("ignored on second line", function() {
+				let diagnostics = validate("\n# esCAPe=a\nFROM node");
+				assert.equal(diagnostics.length, 0);
+			});
 		});
 	});