Fix #121 Add highlight range support for heredocs

Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Jun 19, 2024 · e558bb1 · e558bb1
1 parent 925dabd
commit e558bb1
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
+### Added
+- support computing highlight ranges for heredocs ([#121](https://github.com/rcjsuen/dockerfile-language-service/issues/121))
+
 ### Fixed
 - consider default value of a variable when determining if FROM is invalid or not ([rcjsuen/dockerfile-utils#126](https://github.com/rcjsuen/dockerfile-utils/issues/126))
 

diff --git a/src/dockerDefinition.ts b/src/dockerDefinition.ts
@@ -166,20 +166,17 @@ export class DockerDefinition {
     public computeDefinitionRange(content: string, position: Position): Range | null {
         let dockerfile = DockerfileParser.parse(content);
         let range = this.computeBuildStageDefinition(dockerfile, position);
-        return range ? range : this.computeVariableDefinition(dockerfile, position);
+        if (range === null) {
+            range = this.computeVariableDefinition(dockerfile, position);
+            if (range === null) {
+                return DockerDefinition.computeHeredocDefinition(dockerfile, position);
+            }
+        }
+        return range;
     }
 
     public computeDefinition(textDocument: TextDocumentIdentifier, content: string, position: Position): Location | null {
-        let dockerfile = DockerfileParser.parse(content);
-        let range = this.computeBuildStageDefinition(dockerfile, position);
-        if (range !== null) {
-            return Location.create(textDocument.uri, range);
-        }
-        range = this.computeVariableDefinition(dockerfile, position);
-        if (range !== null) {
-            return Location.create(textDocument.uri, range);
-        }
-        range = DockerDefinition.computeHeredocDefinition(dockerfile, position);
+        const range = this.computeDefinitionRange(content, position);
         if (range !== null) {
             return Location.create(textDocument.uri, range);
         }

diff --git a/src/dockerHighlight.ts b/src/dockerHighlight.ts
@@ -8,7 +8,7 @@ import { TextDocument } from 'vscode-languageserver-textdocument';
 import {
     Position, DocumentHighlight, DocumentHighlightKind
 } from 'vscode-languageserver-types';
-import { DockerfileParser, From } from 'dockerfile-ast';
+import { Copy, DockerfileParser, From, Run } from 'dockerfile-ast';
 import { DockerDefinition } from './dockerDefinition';
 import { Util } from './docker';
 
@@ -74,6 +74,21 @@ export class DockerHighlight {
                 }
             }
         } else {
+            for (const instruction of dockerfile.getInstructions()) {
+                if (instruction instanceof Copy || instruction instanceof Run) {
+                    for (const heredoc of instruction.getHeredocs()) {
+                        const nameRange = heredoc.getNameRange();
+                        if (Util.positionEquals(definitionRange.start, nameRange.start) && Util.positionEquals(definitionRange.start, nameRange.start)) {
+                            highlights.push(DocumentHighlight.create(definitionRange, DocumentHighlightKind.Write));
+                            const delimiterRange = heredoc.getDelimiterRange();
+                            if (delimiterRange !== null) {
+                                highlights.push(DocumentHighlight.create(delimiterRange, DocumentHighlightKind.Read));
+                            }
+                            return highlights;
+                        }
+                    }
+                }
+            }
             let document = TextDocument.create("", "", 0, content);
             let definition = document.getText().substring(document.offsetAt(definitionRange.start), document.offsetAt(definitionRange.end));
             let isBuildStage = false;

diff --git a/test/dockerHighlight.test.ts b/test/dockerHighlight.test.ts
@@ -2271,4 +2271,103 @@ describe("Dockerfile Document Highlight tests", function () {
             });
         });
     });
+
+    function createHeredocTests(instruction: string) {
+        describe(instruction, () => {
+            const offset = instruction.length;
+            const tests = [
+                {
+                    testName: `<<file`,
+                    content: `FROM alpine\n${instruction} echo <<file\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 8, 1, offset + 12),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<-file`,
+                    content: `FROM alpine\n${instruction} echo <<-file\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<'file'`,
+                    content: `FROM alpine\n${instruction} echo <<'file'\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<'file'`,
+                    content: `FROM alpine\n${instruction} echo <<-'file'\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 10, 1, offset + 14),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<"file"`,
+                    content: `FROM alpine\n${instruction} echo <<"file"\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<-"file"`,
+                    content: `FROM alpine\n${instruction} echo <<-"file"\nabc\nfile`,
+                    writeRange: Range.create(1, offset + 10, 1, offset + 14),
+                    hasReadRange: true
+                },
+                {
+                    testName: `<<file`,
+                    content: `FROM alpine\n${instruction} echo <<file\nabc`,
+                    writeRange: Range.create(1, offset + 8, 1, offset + 12),
+                    hasReadRange: false
+                },
+                {
+                    testName: `<<-file`,
+                    content: `FROM alpine\n${instruction} echo <<-file\nabc`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: false
+                },
+                {
+                    testName: `<<'file'`,
+                    content: `FROM alpine\n${instruction} echo <<'file'\nabc`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: false
+                },
+                {
+                    testName: `<<'file'`,
+                    content: `FROM alpine\n${instruction} echo <<-'file'\nabc`,
+                    writeRange: Range.create(1, offset + 10, 1, offset + 14),
+                    hasReadRange: false
+                },
+                {
+                    testName: `<<"file"`,
+                    content: `FROM alpine\n${instruction} echo <<"file"\nabc`,
+                    writeRange: Range.create(1, offset + 9, 1, offset + 13),
+                    hasReadRange: false
+                },
+                {
+                    testName: `<<-"file"`,
+                    content: `FROM alpine\n${instruction} echo <<-"file"\nabc`,
+                    writeRange: Range.create(1, offset + 10, 1, offset + 14),
+                    hasReadRange: false
+                }
+            ];
+
+            tests.forEach((test) => {
+                it(test.testName, () => {
+                    const ranges = computeHighlightRanges(test.content, 1, offset + 11);
+                    if (test.hasReadRange) {
+                        assert.strictEqual(ranges.length, 2);
+                        assertHighlightRange(ranges[0], { range: test.writeRange, kind: DocumentHighlightKind.Write });
+                        assertHighlightRange(ranges[1], { range: Range.create(3, 0, 3, 4), kind: DocumentHighlightKind.Read });
+                    } else {
+                        assert.strictEqual(ranges.length, 1);
+                        assertHighlightRange(ranges[0], { range: test.writeRange, kind: DocumentHighlightKind.Write });
+                    }
+                });
+            });
+        });
+    }
+
+    describe("Heredoc", () => {
+        createHeredocTests("COPY");
+        createHeredocTests("RUN");
+    });
 });