Fix #140 Highlight redeclared ARG variables

Highlight all the declarations of an ARG variable instead of only assuming that it is declared once. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Aug 7, 2017 · 41db25f · 41db25f
1 parent 04b4e2b
commit 41db25f
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file.
 ### 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))
+- highlight ARG variables that get declared again ([#140](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/140))
 
 ## [0.0.5] - 2017-08-07
 ### Fixed

diff --git a/src/dockerHighlight.ts b/src/dockerHighlight.ts
@@ -52,12 +52,12 @@ export class DockerHighlight {
 				}
 			}
 		} else {
-			highlights.push(DocumentHighlight.create(location.range, DocumentHighlightKind.Write));
 
 			let definition = document.getText().substring(document.offsetAt(location.range.start), document.offsetAt(location.range.end));
 			for (let from of dockerfile.getFROMs()) {
 				let range = from.getBuildStageRange();
 				if (range && range.start.line === location.range.start.line) {
+					highlights.push(DocumentHighlight.create(location.range, DocumentHighlightKind.Write));
 					for (let instruction of dockerfile.getCOPYs()) {
 						let source = instruction.getFromValue();
 						if (source === definition) {
@@ -68,9 +68,17 @@ export class DockerHighlight {
 				}
 			}
 
+			for (let arg of dockerfile.getARGs()) {
+				let property = arg.getProperty();
+				// property may be null if it's an ARG with no arguments
+				if (property && property.getName() === definition) {
+					highlights.push(DocumentHighlight.create(property.getNameRange(), DocumentHighlightKind.Write));
+				}
+			}
+
 			for (let env of dockerfile.getENVs()) {
 				for (let property of env.getProperties()) {
-					if (property.getName() === definition && !Util.rangeEquals(property.getNameRange(), location.range)) {
+					if (property.getName() === definition) {
 						highlights.push(DocumentHighlight.create(property.getNameRange(), DocumentHighlightKind.Write));
 					}
 				}

diff --git a/test/dockerHighlight.test.ts b/test/dockerHighlight.test.ts
@@ -269,6 +269,86 @@ describe("Dockerfile Document Highlight tests", function() {
 				ranges = computeHighlightRanges(document, 1, 13);
 				assertHighlightRanges(ranges, [ arg, run ]);
 			});
+
+			it("repeated declaration $var", function() {
+				let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
+				let declaration2 = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
+				let run = DocumentHighlight.create(Range.create(Position.create(2, 10), Position.create(2, 13)), DocumentHighlightKind.Read);
+				let run2 = DocumentHighlight.create(Range.create(Position.create(3, 10), Position.create(3, 13)), DocumentHighlightKind.Read);
+				let expected = [ declaration, declaration2, run, run2 ];
+				let document = createDocument(instruction + " var=value\n" + instruction + " var=value\nRUN echo $var\nRUN echo $var");
+				let ranges = computeHighlightRanges(document, 0, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 1, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 2, 11);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 3, 11);
+				assertHighlightRanges(ranges, expected);
+			});
+
+			it("repeated declaration $var no value", function() {
+				let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
+				let declaration2 = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
+				let run = DocumentHighlight.create(Range.create(Position.create(2, 10), Position.create(2, 13)), DocumentHighlightKind.Read);
+				let run2 = DocumentHighlight.create(Range.create(Position.create(3, 10), Position.create(3, 13)), DocumentHighlightKind.Read);
+				let expected = [ declaration, declaration2, run, run2 ];
+				let document = createDocument(instruction + " var\n" + instruction + " var\nRUN echo $var\nRUN echo $var");
+				let ranges = computeHighlightRanges(document, 0, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 1, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 2, 11);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 3, 11);
+				assertHighlightRanges(ranges, expected);
+			});
+
+			it("repeated declaration ${var}", function() {
+				let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
+				let declaration2 = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
+				let run = DocumentHighlight.create(Range.create(Position.create(2, 11), Position.create(2, 14)), DocumentHighlightKind.Read);
+				let run2 = DocumentHighlight.create(Range.create(Position.create(3, 11), Position.create(3, 14)), DocumentHighlightKind.Read);
+				let expected = [ declaration, declaration2, run, run2 ];
+				let document = createDocument(instruction + " var=value\n" + instruction + " var=value\nRUN echo ${var}\nRUN echo ${var}");
+				let ranges = computeHighlightRanges(document, 0, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 1, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 2, 11);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 3, 11);
+				assertHighlightRanges(ranges, expected);
+			});
+
+			it("repeated declaration $var no value", function() {
+				let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
+				let declaration2 = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
+				let run = DocumentHighlight.create(Range.create(Position.create(2, 11), Position.create(2, 14)), DocumentHighlightKind.Read);
+				let run2 = DocumentHighlight.create(Range.create(Position.create(3, 11), Position.create(3, 14)), DocumentHighlightKind.Read);
+				let expected = [ declaration, declaration2, run, run2 ];
+				let document = createDocument(instruction + " var\n" + instruction + " var\nRUN echo ${var}\nRUN echo ${var}");
+				let ranges = computeHighlightRanges(document, 0, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 1, 5);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 2, 12);
+				assertHighlightRanges(ranges, expected);
+
+				ranges = computeHighlightRanges(document, 3, 12);
+				assertHighlightRanges(ranges, expected);
+			});
 		});
 	}