From 12c5628f54fb446b146dab667cc910dfb925f742 Mon Sep 17 00:00:00 2001 From: Remy Suen Date: Sun, 20 Aug 2017 04:01:23 +0900 Subject: [PATCH] Fix #148 Add a completion item for COPY's --from flag Signed-off-by: Remy Suen --- CHANGELOG.md | 2 + src/dockerAssist.ts | 40 +++-- src/dockerPlainText.ts | 3 + test/dockerAssist.test.ts | 313 +++++++++++++++++++++++++------------- 4 files changed, 244 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff2791b..93fe83e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file. ## [Unreleased] ### Added +- textDocument/completion + - COPY's --from build stage flag ([#148](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/148)) - textDocument/publishDiagnostics - 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)) diff --git a/src/dockerAssist.ts b/src/dockerAssist.ts index e9fecc6..e6999bf 100644 --- a/src/dockerAssist.ts +++ b/src/dockerAssist.ts @@ -130,7 +130,7 @@ export class DockerAssist { } else if (Util.isInsideRange(position, instruction.getRange())) { switch (instruction.getKeyword()) { case "COPY": - return this.createBuildStageProposals(dockerfile, instruction as Copy, position, offset); + return this.createCopyProposals(dockerfile, instruction as Copy, position, offset, prefix); case "HEALTHCHECK": let subcommand = (instruction as Healthcheck).getSubcommand(); if (subcommand && subcommand.isBefore(position)) { @@ -145,12 +145,17 @@ export class DockerAssist { break instructionsCheck; } else { let trigger = (instruction as Onbuild).getTriggerInstruction(); - if (trigger !== null && trigger.getKeyword() === "HEALTHCHECK") { - let subcommand = (trigger as Healthcheck).getSubcommand(); - if (subcommand && subcommand.isBefore(position)) { - return []; + if (trigger !== null) { + switch (trigger.getKeyword()) { + case "COPY": + return this.createCopyProposals(dockerfile, trigger as Copy, position, offset, prefix); + case "HEALTHCHECK": + let subcommand = (trigger as Healthcheck).getSubcommand(); + if (subcommand && subcommand.isBefore(position)) { + return []; + } + return this.createHealthcheckProposals(dockerfile, position, offset, prefix); } - return this.createHealthcheckProposals(dockerfile, position, offset, prefix); } } return []; @@ -219,17 +224,17 @@ export class DockerAssist { return proposals; } - private createBuildStageProposals(dockerfile: Dockerfile, copy: Copy, position: Position, offset: number) { + private createCopyProposals(dockerfile: Dockerfile, copy: Copy, position: Position, offset: number, prefix: string) { let range = copy.getFromValueRange(); // is the user in the --from= area if (range && Util.isInsideRange(position, copy.getFromValueRange())) { // get the prefix - let prefix = this.document.getText().substring(this.document.offsetAt(range.start), offset).toLowerCase(); + let stagePrefix = this.document.getText().substring(this.document.offsetAt(range.start), offset).toLowerCase(); let items: CompletionItem[] = []; for (let from of dockerfile.getFROMs()) { let stage = from.getBuildStage(); - if (stage && stage.toLowerCase().indexOf(prefix) === 0) { - items.push(this.createSourceImageCompletionItem(stage, prefix.length, offset)); + if (stage && stage.toLowerCase().indexOf(stagePrefix) === 0) { + items.push(this.createSourceImageCompletionItem(stage, stagePrefix.length, offset)); } } items.sort((item: CompletionItem, item2: CompletionItem) => { @@ -237,6 +242,14 @@ export class DockerAssist { }); return items; } + + let copyArgs = copy.getArguments(); + if (copyArgs.length === 0) { + return [ this.createCOPY_FlagFrom(0, offset) ]; + } else if (Util.isInsideRange(position, copyArgs[0].getRange()) && prefix !== "--from=" && "--from=".indexOf(prefix) === 0) { + return [ this.createCOPY_FlagFrom(prefix.length, offset) ]; + } + return []; } @@ -409,6 +422,13 @@ export class DockerAssist { }; } + private createCOPY_FlagFrom(prefixLength: number, offset: number): CompletionItem { + if (this.snippetSupport) { + return this.createFlagCompletionItem("--from=stage", prefixLength, offset, "--from=${1:stage}", "COPY_FlagFrom"); + } + return this.createFlagCompletionItem("--from=", prefixLength, offset, "--from=", "COPY_FlagFrom"); + } + private createHEALTHCHECK_FlagInterval(prefixLength: number, offset: number): CompletionItem { if (this.snippetSupport) { return this.createFlagCompletionItem("--interval=30s", prefixLength, offset, "--interval=${1:30s}", "HEALTHCHECK_FlagInterval"); diff --git a/src/dockerPlainText.ts b/src/dockerPlainText.ts index 7522c9b..2d100f4 100644 --- a/src/dockerPlainText.ts +++ b/src/dockerPlainText.ts @@ -26,6 +26,7 @@ export class PlainTextDocumentation { "hoverVolume": "Create a mount point with the specifid name and mark it as holding externally mounted volumes from the native host or from other containers.\n\n", "hoverWorkdir": "Set the working directory for any subsequent ADD, COPY, CMD, ENTRYPOINT, or RUN` instructions that follow it in the `Dockerfile`.\n\n", + "hoverCopyFlagFrom": "The previous build stage to use as the source location instead of the build's context.", "hoverHealthcheckFlagInterval": "The seconds to wait for the health check to run after the container has started, and then again the number of seconds to wait before running again after the previous check has completed.", "hoverHealthcheckFlagRetries": "The number of consecutive failures of this health check before the container is considered to be `unhealthy`.", "hoverHealthcheckFlagStartPeriod": "The number of seconds to wait for the container to startup. Failures during this grace period will not count towards the maximum number of retries. However, should a health check succeed during this period then any subsequent failures will count towards the maximum number of retries.", @@ -70,6 +71,8 @@ export class PlainTextDocumentation { "COPY hello.txt relative/to/workdir" , + COPY_FlagFrom: this.dockerMessages["hoverCopyFlagFrom"], + ENTRYPOINT: this.dockerMessages["hoverEntrypoint"] + "ENTRYPOINT [ \"/opt/app/run.sh\", \"--port\", \"8080\" ]" , diff --git a/test/dockerAssist.test.ts b/test/dockerAssist.test.ts index ec6485b..38127fb 100644 --- a/test/dockerAssist.test.ts +++ b/test/dockerAssist.test.ts @@ -235,6 +235,26 @@ function assertHEALTHCHECK_CMD(item: CompletionItem, line: number, character: nu assert.equal(item.textEdit.range.end.character, character + prefixLength); } +function assertCOPY_FlagFrom(item: CompletionItem, startLine: number, startCharacter: number, endLine: number, endCharacter: number, snippetSupport?: boolean) { + if (snippetSupport === undefined || snippetSupport) { + assert.equal(item.label, "--from=stage"); + } else { + assert.equal(item.label, "--from="); + } + assert.equal(item.kind, CompletionItemKind.Field); + if (snippetSupport === undefined || snippetSupport) { + assert.equal(item.insertTextFormat, InsertTextFormat.Snippet); + assert.equal(item.textEdit.newText, "--from=${1:stage}"); + } else { + assert.equal(item.insertTextFormat, InsertTextFormat.PlainText); + assert.equal(item.textEdit.newText, "--from="); + } + assert.equal(item.textEdit.range.start.line, startLine); + assert.equal(item.textEdit.range.start.character, startCharacter); + assert.equal(item.textEdit.range.end.line, endLine); + assert.equal(item.textEdit.range.end.character, endCharacter); +} + function assertHEALTHCHECK_FlagInterval(item: CompletionItem, startLine: number, startCharacter: number, endLine: number, endCharacter: number, snippetSupport?: boolean) { if (snippetSupport === undefined || snippetSupport) { assert.equal(item.label, "--interval=30s"); @@ -844,110 +864,140 @@ describe('Docker Content Assist Tests', function() { content = header + "FROM node\n" + instruction + " " + escapeChar + "\n"; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); - } else if (instruction === "ONBUILD") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertONBUILDProposals(proposals, lastLine, 0, 0); - } else { - assert.equal(proposals.length, 0); + let split = content.split("\n"); + let lastLine = split.length - 1; + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 0, lastLine, 0); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 0, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + " " + escapeChar + "\r\n"; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); - } else if (instruction === "ONBUILD") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertONBUILDProposals(proposals, lastLine, 0, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 0, lastLine, 0); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 0, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + " " + escapeChar + " \n"; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); - } else if (instruction === "ONBUILD") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertONBUILDProposals(proposals, lastLine, 0, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 0, lastLine, 0); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 0, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + " " + escapeChar + " \r\n"; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); - } else if (instruction === "ONBUILD") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertONBUILDProposals(proposals, lastLine, 0, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 0, lastLine, 0); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 0, lastLine, 0); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 0, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + escapeChar + "\n "; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); - } else if (instruction === "ONBUILD") { - let lastLine = content.split("\n").length; - assertONBUILDProposals(proposals, lastLine - 1, 1, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 1, lastLine, 1); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 1, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + escapeChar + "\r\n "; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); - } else if (instruction === "ONBUILD") { - let lastLine = content.split("\n").length; - assertONBUILDProposals(proposals, lastLine - 1, 1, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 1, lastLine, 1); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 1, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + " " + escapeChar + "\n "; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); - } else if (instruction === "ONBUILD") { - let lastLine = content.split("\n").length; - assertONBUILDProposals(proposals, lastLine - 1, 1, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 1, lastLine, 1); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 1, 0); + break; + default: + assert.equal(proposals.length, 0); } content = header + "FROM node\n" + instruction + " " + escapeChar + "\r\n "; proposals = compute(content, content.length); - if (instruction === "HEALTHCHECK") { - let split = content.split("\n"); - let lastLine = split.length - 1; - assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); - } else if (instruction === "ONBUILD") { - let lastLine = content.split("\n").length; - assertONBUILDProposals(proposals, lastLine - 1, 1, 0); - } else { - assert.equal(proposals.length, 0); + switch (instruction) { + case "COPY": + assert.equal(proposals.length, 1); + assertCOPY_FlagFrom(proposals[0], lastLine, 1, lastLine, 1); + break; + case "HEALTHCHECK": + assertHealthcheckItems(proposals, lastLine, 1, lastLine, 1); + break; + case "ONBUILD": + assertONBUILDProposals(proposals, lastLine, 1, 0); + break; + default: + assert.equal(proposals.length, 0); } } @@ -1483,38 +1533,92 @@ describe('Docker Content Assist Tests', function() { }); }) - describe("COPY", function() { - describe("--from=", function() { - it("no sources", function() { - var proposals = computePosition("FROM busybox\nCOPY --from=", 1, 12); - assert.equal(proposals.length, 0); - }); - - it("source image", function() { - var proposals = computePosition("FROM busybox AS source\nCOPY --from=", 1, 12); - assert.equal(proposals.length, 1); - assertSourceImage(proposals[0], "source", 1, 12, 1, 12); - }); + function testCopy(trigger: boolean) { + describe("COPY", function() { + let onbuild = trigger ? "ONBUILD " : ""; + let triggerOffset = onbuild.length; - it("source images alphabetical", function() { - var proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\nCOPY --from=", 2, 12); - assert.equal(proposals.length, 2); - assertSourceImage(proposals[0], "dev", 2, 12, 2, 12); - assertSourceImage(proposals[1], "setup", 2, 12, 2, 12); + describe("build stages", function() { + it("no sources", function() { + var proposals = computePosition("FROM busybox\n" + onbuild + "COPY --from=", 1, triggerOffset + 12); + assert.equal(proposals.length, 0); + }); + + 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", 1, triggerOffset + 12, 1, triggerOffset + 12); + }); + + it("source images alphabetical", function() { + var proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\n" + onbuild + "COPY --from=", 2, triggerOffset + 12); + assert.equal(proposals.length, 2); + assertSourceImage(proposals[0], "dev", 2, triggerOffset + 12, 2, triggerOffset + 12); + assertSourceImage(proposals[1], "setup", 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", 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", 2, triggerOffset + 12, 2, triggerOffset + 13); + }); }); - - it("source image prefix", function() { - var proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\nCOPY --from=s", 2, 13); - assert.equal(proposals.length, 1); - assertSourceImage(proposals[0], "setup", 2, 12, 2, 13); - - // casing should be ignored - proposals = computePosition("FROM busybox AS setup\nFROM busybox AS dev\nCOPY --from=S", 2, 13); - assert.equal(proposals.length, 1); - assertSourceImage(proposals[0], "setup", 2, 12, 2, 13); + + describe("arguments", function() { + it("full", function() { + let items = computePosition("FROM busybox\n" + onbuild + "COPY ", 1, triggerOffset + 5); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 5); + + items = computePosition("FROM busybox\n" + onbuild + "COPY ", 1, triggerOffset + 5); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 5); + }); + + it("prefix", function() { + let items = computePosition("FROM busybox\n" + onbuild + "COPY -", 1, triggerOffset + 6); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 6); + + items = computePosition("FROM busybox\n" + onbuild + "COPY --", 1, triggerOffset + 7); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 7); + + items = computePosition("FROM busybox\n" + onbuild + "COPY --f", 1, triggerOffset + 8); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 8); + + items = computePosition("FROM busybox\n" + onbuild + "COPY --fr", 1, triggerOffset + 9); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 9); + + items = computePosition("FROM busybox\n" + onbuild + "COPY --fro", 1, triggerOffset + 10); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 10); + + items = computePosition("FROM busybox\n" + onbuild + "COPY --from", 1, triggerOffset + 11); + assertCOPY_FlagFrom(items[0], 1, triggerOffset + 5, 1, triggerOffset + 11); + }); + + it("none", function() { + let items = computePosition("FROM busybox\n" + onbuild + "COPY --from=", 1, triggerOffset + 12); + assert.equal(items.length, 0); + + items = computePosition("FROM busybox\n" + onbuild + "COPY app app", 1, triggerOffset + 6); + assert.equal(items.length, 0); + + items = computePosition("FROM busybox\n" + onbuild + "COPY app app", 1, triggerOffset + 10); + assert.equal(items.length, 0); + + items = computePosition("FROM busybox\n" + onbuild + "COPY app app", 1, triggerOffset + 9); + assert.equal(items.length, 0); + + items = computePosition("FROM busybox\n" + onbuild + "COPY app --fr app", 1, triggerOffset + 13); + assert.equal(items.length, 0); + }); }); }); - }); + } + + testCopy(false); function testHealthcheck(trigger: boolean) { describe("HEALTHCHECK", function() { @@ -1753,6 +1857,7 @@ describe('Docker Content Assist Tests', function() { }); }); + testCopy(true); testHealthcheck(true); });