From 4358777a17b43de25d50f94994f46b585b567d90 Mon Sep 17 00:00:00 2001 From: Remy Suen Date: Tue, 14 Nov 2017 10:49:21 +0900 Subject: [PATCH] Fix #191 Single quote LABEL values should be literal If an environment variable such as a $var or ${var} is found in a LABEL's value that is contained in single quotes, they should be ignored. This is because the content in a single quoted string should be read literally and not go through any processing for variable expansions and such. Signed-off-by: Remy Suen --- CHANGELOG.md | 1 + src/parser/instructions/label.ts | 24 +++++++++ test/dockerDefinition.test.ts | 48 +++++++++++++++++ test/dockerHighlight.test.ts | 88 ++++++++++++++++++++++++++++++ test/dockerHover.test.ts | 22 ++++++++ test/dockerRename.test.ts | 92 ++++++++++++++++++++++++++++++++ 6 files changed, 275 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16c4384..3808ded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. ### Fixed - prevent completion items from being displayed in comments ([#190](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/190)) - expand environment variables when validating an EXPOSE ([#192](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/192)) +- ignore variables that are in a LABEL's single quoted value string ([#191](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/191)) ## [0.0.10] - 2017-10-23 ### Added diff --git a/src/parser/instructions/label.ts b/src/parser/instructions/label.ts index d87fb48..fa4e78e 100644 --- a/src/parser/instructions/label.ts +++ b/src/parser/instructions/label.ts @@ -4,8 +4,10 @@ * ------------------------------------------------------------------------------------------ */ import { TextDocument, Range } from 'vscode-languageserver'; import { Dockerfile } from '../dockerfile'; +import { Variable } from '../variable'; import { Property } from '../property'; import { PropertyInstruction } from './propertyInstruction'; +import { Util } from '../../docker'; export class Label extends PropertyInstruction { @@ -13,6 +15,28 @@ export class Label extends PropertyInstruction { super(document, range, dockerfile, escapeChar, instruction, instructionRange); } + public getVariables(): Variable[] { + const variables = super.getVariables(); + const properties = this.getProperties(); + // iterate over all of this LABEL's properties + for (const property of properties) { + const value = property.getRawValue(); + // check if the value is contained in single quotes, + // single quotes would indicate a literal value + if (value.length >= 2 && value.charAt(0) === '\'' && value.charAt(value.length - 1) === '\'') { + const range = property.getValueRange(); + for (let i = 0; i < variables.length; i++) { + // if a variable is in a single quote, remove it from the list + if (Util.isInsideRange(variables[i].getRange().start, range)) { + variables.splice(i, 1); + i--; + } + } + } + } + return variables; + } + public getProperties(): Property[] { return super.getProperties(); } diff --git a/test/dockerDefinition.test.ts b/test/dockerDefinition.test.ts index db3839d..1b8b09c 100644 --- a/test/dockerDefinition.test.ts +++ b/test/dockerDefinition.test.ts @@ -187,6 +187,18 @@ describe("Dockerfile Document Definition tests", function() { location = computeDefinition(document, Position.create(2, 5)); assertLocation(location, document.uri, 0, 4, 0, 7); }); + + it("label value with single quotes", function() { + let document = createDocument(instruction + " var\nLABEL label='${var}'"); + let location = computeDefinition(document, Position.create(1, 17)); + assert.equal(location, null); + }); + + it("label value with double quotes", function() { + let document = createDocument(instruction + " var\nLABEL label=\"${var}\""); + let location = computeDefinition(document, Position.create(1, 17)); + assertLocation(location, document.uri, 0, 4, 0, 7); + }); }); describe("$var", function() { @@ -305,6 +317,18 @@ describe("Dockerfile Document Definition tests", function() { location = computeDefinition(document, Position.create(2, 5)); assertLocation(location, document.uri, 0, 4, 0, 7); }); + + it("label value with single quotes", function() { + let document = createDocument(instruction + " var\nLABEL label='$var'"); + let location = computeDefinition(document, Position.create(1, 15)); + assert.equal(location, null); + }); + + it("label value with double quotes", function() { + let document = createDocument(instruction + " var\nLABEL label=\"$var\""); + let location = computeDefinition(document, Position.create(1, 15)); + assertLocation(location, document.uri, 0, 4, 0, 7); + }); }); }); @@ -486,6 +510,18 @@ describe("Dockerfile Document Definition tests", function() { location = computeDefinition(document, Position.create(7, 5)); assertLocation(location, document.uri, 5, 4, 5, 7); }); + + it("label value with single quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label='$var'"); + let location = computeDefinition(document, Position.create(2, 15)); + assert.equal(location, null); + }); + + it("label value with double quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label=\"$var\""); + let location = computeDefinition(document, Position.create(2, 15)); + assertLocation(location, document.uri, 1, 4, 1, 7); + }); }); describe("$var", function() { @@ -719,6 +755,18 @@ describe("Dockerfile Document Definition tests", function() { location = computeDefinition(document, Position.create(7, 5)); assertLocation(location, document.uri, 5, 4, 5, 7); }); + + it("label value with single quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label='${var}'"); + let location = computeDefinition(document, Position.create(2, 17)); + assert.equal(location, null); + }); + + it("label value with double quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label=\"${var}\""); + let location = computeDefinition(document, Position.create(2, 17)); + assertLocation(location, document.uri, 1, 4, 1, 7); + }); }); }); }); diff --git a/test/dockerHighlight.test.ts b/test/dockerHighlight.test.ts index a086e75..bae5de9 100644 --- a/test/dockerHighlight.test.ts +++ b/test/dockerHighlight.test.ts @@ -351,6 +351,50 @@ describe("Dockerfile Document Highlight tests", function() { ranges = computeHighlightRanges(document, 3, 12); assertHighlightRanges(ranges, expected); }); + + it("$var in LABEL value with single quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write); + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'"); + let ranges = computeHighlightRanges(document, 0, 5); + assertHighlightRanges(ranges, [ declaration ]); + + ranges = computeHighlightRanges(document, 1, 15); + assert.equal(ranges.length, 0); + }); + + it("$var in LABEL value with double quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write); + let labelValue = DocumentHighlight.create(Range.create(Position.create(1, 14), Position.create(1, 17)), DocumentHighlightKind.Read); + let expected = [ declaration, labelValue ]; + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\""); + let ranges = computeHighlightRanges(document, 0, 5); + assertHighlightRanges(ranges, expected); + + ranges = computeHighlightRanges(document, 1, 15); + assertHighlightRanges(ranges, expected); + }); + + it("${var} in LABEL value with single quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write); + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'"); + let ranges = computeHighlightRanges(document, 0, 5); + assertHighlightRanges(ranges, [ declaration ]); + + ranges = computeHighlightRanges(document, 1, 17); + assert.equal(ranges.length, 0); + }); + + it("${var} in LABEL value with double quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write); + let labelValue = DocumentHighlight.create(Range.create(Position.create(1, 15), Position.create(1, 18)), DocumentHighlightKind.Read); + let expected = [ declaration, labelValue ]; + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\""); + let ranges = computeHighlightRanges(document, 0, 5); + assertHighlightRanges(ranges, expected); + + ranges = computeHighlightRanges(document, 1, 17); + assertHighlightRanges(ranges, expected); + }); }); describe("build stage", function() { @@ -722,6 +766,50 @@ describe("Dockerfile Document Highlight tests", function() { ranges = computeHighlightRanges(document, 9, 11); assertHighlightRanges(ranges, expectedB); }); + + it("$var in LABEL value with single quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write); + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='$var'"); + let ranges = computeHighlightRanges(document, 1, 5); + assertHighlightRanges(ranges, [ declaration ]); + + ranges = computeHighlightRanges(document, 2, 15); + assert.equal(ranges.length, 0); + }); + + it("$var in LABEL value with double quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write); + let labelValue = DocumentHighlight.create(Range.create(Position.create(2, 14), Position.create(2, 17)), DocumentHighlightKind.Read); + let expected = [ declaration, labelValue ]; + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"$var\""); + let ranges = computeHighlightRanges(document, 1, 5); + assertHighlightRanges(ranges, expected); + + ranges = computeHighlightRanges(document, 2, 15); + assertHighlightRanges(ranges, expected); + }); + + it("${var} in LABEL value with single quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write); + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='${var}'"); + let ranges = computeHighlightRanges(document, 1, 5); + assertHighlightRanges(ranges, [ declaration ]); + + ranges = computeHighlightRanges(document, 2, 17); + assert.equal(ranges.length, 0); + }); + + it("${var} in LABEL value with double quotes", function() { + let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write); + let labelValue = DocumentHighlight.create(Range.create(Position.create(2, 15), Position.create(2, 18)), DocumentHighlightKind.Read); + let expected = [ declaration, labelValue ]; + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"${var}\""); + let ranges = computeHighlightRanges(document, 1, 5); + assertHighlightRanges(ranges, expected); + + ranges = computeHighlightRanges(document, 2, 17); + assertHighlightRanges(ranges, expected); + }); }); }); } diff --git a/test/dockerHover.test.ts b/test/dockerHover.test.ts index 18e26a8..ecb9825 100644 --- a/test/dockerHover.test.ts +++ b/test/dockerHover.test.ts @@ -813,6 +813,28 @@ describe("Dockerfile hover", function() { assert.equal(onHover(document, 3, 9), null); assert.equal(onHover(document, 4, 11), null); }); + + it("$var in LABEL value with single quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'"); + assert.equal(onHover(document, 1, 15), null); + }); + + it("$var in LABEL value with double quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\""); + let hover = onHover(document, 1, 15); + assert.equal(hover.contents, "value"); + }); + + it("${var} in LABEL value with single quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'"); + assert.equal(onHover(document, 1, 17), null); + }); + + it("${var} in LABEL value with double quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\""); + let hover = onHover(document, 1, 17); + assert.equal(hover.contents, "value"); + }); }); } diff --git a/test/dockerRename.test.ts b/test/dockerRename.test.ts index ab170e3..ed5b775 100644 --- a/test/dockerRename.test.ts +++ b/test/dockerRename.test.ts @@ -297,6 +297,52 @@ describe("Dockerfile Document Rename tests", function() { assertEdit(edits[4], "renamed", 4, 11, 4, 14); assertEdit(edits[5], "renamed", 5, 11, 5, 14); }); + + it("$var in LABEL value with single quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'"); + let edits = rename(document, 0, 5, "renamed"); + assert.equal(edits.length, 1); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + + edits = rename(document, 1, 15, "renamed"); + assert.equal(edits.length, 0); + }); + + it("$var in LABEL value with double quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\""); + let edits = rename(document, 0, 5, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + assertEdit(edits[1], "renamed", 1, 14, 1, 17); + + edits = rename(document, 1, 15, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + assertEdit(edits[1], "renamed", 1, 14, 1, 17); + }); + + it("${var} in LABEL value with single quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'"); + let edits = rename(document, 0, 5, "renamed"); + assert.equal(edits.length, 1); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + + edits = rename(document, 1, 17, "renamed"); + assert.equal(edits.length, 0); + }); + + it("${var} in LABEL value with double quotes", function() { + let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\""); + let edits = rename(document, 0, 5, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + assertEdit(edits[1], "renamed", 1, 15, 1, 18); + + edits = rename(document, 1, 17, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 0, 4, 0, 7); + assertEdit(edits[1], "renamed", 1, 15, 1, 18); + }); }); describe("build stage", function() { @@ -423,6 +469,52 @@ describe("Dockerfile Document Rename tests", function() { assertEdits(rename(document, 12, 12, "renamed"), expectedEdits2); assertEdits(rename(document, 13, 13, "renamed"), expectedEdits2); }); + + it("$var in LABEL value with single quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='$var'"); + let edits = rename(document, 1, 5, "renamed"); + assert.equal(edits.length, 1); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + + edits = rename(document, 2, 15, "renamed"); + assert.equal(edits.length, 0); + }); + + it("$var in LABEL value with double quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"$var\""); + let edits = rename(document, 1, 5, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + assertEdit(edits[1], "renamed", 2, 14, 2, 17); + + edits = rename(document, 2, 15, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + assertEdit(edits[1], "renamed", 2, 14, 2, 17); + }); + + it("${var} in LABEL value with single quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='${var}'"); + let edits = rename(document, 1, 5, "renamed"); + assert.equal(edits.length, 1); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + + edits = rename(document, 2, 17, "renamed"); + assert.equal(edits.length, 0); + }); + + it("${var} in LABEL value with double quotes", function() { + let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"${var}\""); + let edits = rename(document, 1, 5, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + assertEdit(edits[1], "renamed", 2, 15, 2, 18); + + edits = rename(document, 2, 17, "renamed"); + assert.equal(edits.length, 2); + assertEdit(edits[0], "renamed", 1, 4, 1, 7); + assertEdit(edits[1], "renamed", 2, 15, 2, 18); + }); }); }); }