diff --git a/packages/docker-parser/README.md b/packages/docker-parser/README.md index f2c45d4e..817abaeb 100644 --- a/packages/docker-parser/README.md +++ b/packages/docker-parser/README.md @@ -44,6 +44,47 @@ const output = deparse(ast); console.log(output); // FROM node:18-alpine ``` +### Comments + +A comment belongs to the instruction below it, as `leadingComments`, and a blank +line above a node is `blankBefore`. Both are emitted by the deparser, so a +parsed Dockerfile keeps its comments and section spacing through a round-trip, +and a generated one can carry its own: + +```typescript +deparse({ + type: 'Dockerfile', + directives: [], + comments: [], + stages: [ + { + type: 'Stage', + from: { type: 'FromInstruction', instruction: 'FROM', image: 'node:22-alpine' }, + instructions: [ + { + type: 'CopyInstruction', + instruction: 'COPY', + sources: ['package.json'], + destination: './', + blankBefore: true, + leadingComments: [{ type: 'Comment', value: 'manifests only: cache the install layer' }], + }, + ], + }, + ], +}); +// FROM node:22-alpine +// # manifests only: cache the install layer +// +// COPY package.json ./ +``` + +`Dockerfile.comments` holds every comment in source order, and comments below +the last instruction land in `Dockerfile.trailingComments`. + +Line continuations are not preserved: a `RUN` written across several lines with +`\` deparses as one line. + ### AST Comparison ```typescript diff --git a/packages/docker-parser/__tests__/deparser.test.ts b/packages/docker-parser/__tests__/deparser.test.ts index 756f13f5..197135a8 100644 --- a/packages/docker-parser/__tests__/deparser.test.ts +++ b/packages/docker-parser/__tests__/deparser.test.ts @@ -148,4 +148,84 @@ FROM alpine`; expect(result).toContain('# escape=`'); }); }); + + describe('mount flags', () => { + it('should keep a cache mount out of the command', () => { + const ast = parse( + 'FROM alpine\nRUN --mount=type=cache,id=pnpm-store,target=/store pnpm install' + ); + const run = ast.stages[0].instructions[0]; + + expect(run).toMatchObject({ + type: 'RunInstruction', + command: 'pnpm install', + mount: [{ type: 'cache', id: 'pnpm-store', target: '/store' }] + }); + expect(deparse(ast)).toBe( + 'FROM alpine\nRUN --mount=type=cache,target=/store,id=pnpm-store pnpm install' + ); + }); + }); + + describe('comments', () => { + it('should emit a comment above the instruction it leads', () => { + const source = 'FROM alpine\n# why this copy is separate\nCOPY a.json ./'; + + expect(deparse(parse(source))).toBe(source); + }); + + it('should emit a comment block and keep the blank line below it', () => { + const source = 'FROM alpine\n\n# section header\n\nCOPY a.json ./\nCOPY b.json ./'; + + expect(deparse(parse(source))).toBe(source); + }); + + it('should emit comments that lead a later stage', () => { + const source = 'FROM alpine AS build\n\n# the runtime image\nFROM alpine\nCOPY --from=build /out /out'; + + expect(deparse(parse(source))).toBe(source); + }); + + it('should emit a trailing comment that leads nothing', () => { + const source = 'FROM alpine\nCOPY a.json ./\n# trailing note'; + + expect(deparse(parse(source))).toBe(source); + }); + + it('should attach comments to the node below, not the one above', () => { + const ast = parse('FROM alpine\nCOPY a.json ./\n# about b\nCOPY b.json ./'); + const [copyA, copyB] = ast.stages[0].instructions; + + expect(copyA.leadingComments).toBeUndefined(); + expect(copyB.leadingComments).toEqual([ + expect.objectContaining({ type: 'Comment', value: 'about b' }) + ]); + }); + + it('should emit comments built by hand, without a parse', () => { + const result = deparse({ + type: 'Dockerfile', + directives: [], + comments: [], + stages: [ + { + type: 'Stage', + from: { type: 'FromInstruction', instruction: 'FROM', image: 'alpine' }, + instructions: [ + { + type: 'CopyInstruction', + instruction: 'COPY', + sources: ['a.json'], + destination: './', + blankBefore: true, + leadingComments: [{ type: 'Comment', value: 'generated: the handler manifest' }] + } + ] + } + ] + }); + + expect(result).toBe('FROM alpine\n# generated: the handler manifest\n\nCOPY a.json ./'); + }); + }); }); diff --git a/packages/docker-parser/__tests__/roundtrip.test.ts b/packages/docker-parser/__tests__/roundtrip.test.ts index 205851af..9c9aabc1 100644 --- a/packages/docker-parser/__tests__/roundtrip.test.ts +++ b/packages/docker-parser/__tests__/roundtrip.test.ts @@ -154,6 +154,58 @@ FROM alpine`); }); }); + describe('RUN flags', () => { + it('should round-trip a cache mount', () => { + expectRoundTrip( + 'FROM alpine\nRUN --mount=type=cache,id=pnpm-store,target=/root/.local/share/pnpm/store pnpm install' + ); + }); + + it('should round-trip a bind mount with a source', () => { + expectRoundTrip('FROM alpine\nRUN --mount=type=bind,source=/src,target=/dst make'); + }); + + it('should round-trip a secret mount', () => { + expectRoundTrip('FROM alpine\nRUN --mount=type=secret,id=npmrc,required npm ci'); + }); + + it('should round-trip two mounts on one RUN', () => { + expectRoundTrip( + 'FROM alpine\nRUN --mount=type=cache,target=/cache --mount=type=bind,target=/src build.sh' + ); + }); + + it('should round-trip --network and --security', () => { + expectRoundTrip('FROM alpine\nRUN --network=none --security=insecure build.sh'); + }); + }); + + describe('comments and blank lines', () => { + it('should round-trip a comment above an instruction', () => { + expectRoundTrip('FROM alpine\n# why this copy is separate\nCOPY a.json ./'); + }); + + it('should round-trip a block of consecutive comments', () => { + expectRoundTrip('FROM alpine\n# first line\n# second line\n# third line\nCOPY a.json ./'); + }); + + it('should round-trip a blank line between a comment and its instruction', () => { + expectRoundTrip('FROM alpine\n# section header\n\nCOPY a.json ./'); + }); + + it('should round-trip comments leading a stage', () => { + expectRoundTrip('FROM alpine AS build\nRUN build.sh\n\n# the runtime image\nFROM alpine\nCOPY --from=build /out /out'); + }); + + it('should round-trip a comment after the last instruction', () => { + expectRoundTrip('FROM alpine\nCOPY a.json ./\n# trailing note'); + }); + + it('should round-trip an empty comment line', () => { + expectRoundTrip('FROM alpine\n# heading\n#\n# body\nCOPY a.json ./'); + }); + }); + describe('complete Dockerfiles', () => { it('should round-trip a typical Node.js Dockerfile', () => { expectRoundTrip(`FROM node:18-alpine diff --git a/packages/docker-parser/src/deparser.ts b/packages/docker-parser/src/deparser.ts index 0a82fd4c..95926026 100644 --- a/packages/docker-parser/src/deparser.ts +++ b/packages/docker-parser/src/deparser.ts @@ -116,17 +116,56 @@ export class Deparser { lines.push(this.deparseDirective(directive)); } - // Then stages - for (const stage of dockerfile.stages) { - if (lines.length > 0) { + // Then stages, separated by a blank line — unless the stage already places + // that separator itself via blankBefore (on the stage, or on the first of + // its leading comments). A separator is never inserted where the AST does + // not ask for one: an invented blank line would come back as blankBefore on + // the next parse and break the round-trip. + dockerfile.stages.forEach((stage, index) => { + if (index > 0 && !this.leadsWithBlank(stage)) { lines.push(''); } lines.push(this.deparseStage(stage)); + }); + + for (const comment of dockerfile.trailingComments ?? []) { + if (comment.blankBefore) { + lines.push(''); + } + lines.push(this.deparseComment(comment)); } return lines.join(this.options.newline); } + /** + * Prefix a node's rendered lines with its blank line and leading comments + */ + private withLeading(node: Node, rendered: string): string { + const lines: string[] = []; + // Each comment carries its own blank line, so a blank above the comment + // block and a blank between the block and the instruction stay distinct. + for (const comment of node.leadingComments ?? []) { + if (comment.blankBefore) { + lines.push(''); + } + lines.push(this.deparseComment(comment)); + } + if (node.blankBefore) { + lines.push(''); + } + lines.push(rendered); + return lines.join(this.options.newline); + } + + /** + * Whether a node already renders a blank line above itself + */ + private leadsWithBlank(node: Node): boolean { + const first = node.leadingComments?.[0]; + return first ? Boolean(first.blankBefore) : Boolean(node.blankBefore); + } + /** * Deparse parser directive */ @@ -141,21 +180,22 @@ export class Deparser { const lines: string[] = []; // FROM instruction - lines.push(this.deparseFrom(stage.from)); + lines.push(this.withLeading(stage.from, this.deparseFrom(stage.from))); // Other instructions for (const instruction of stage.instructions) { - lines.push(this.deparseInstruction(instruction)); + lines.push(this.withLeading(instruction, this.deparseInstruction(instruction))); } - return lines.join(this.options.newline); + return this.withLeading(stage, lines.join(this.options.newline)); } /** * Deparse comment */ private deparseComment(comment: Comment): string { - return `# ${comment.value}`; + // An empty comment is a bare `#`; `# ` would add trailing whitespace. + return comment.value ? `# ${comment.value}` : '#'; } /** diff --git a/packages/docker-parser/src/lexer.ts b/packages/docker-parser/src/lexer.ts index 380c2f77..8b0a0bc8 100644 --- a/packages/docker-parser/src/lexer.ts +++ b/packages/docker-parser/src/lexer.ts @@ -267,6 +267,33 @@ export class Lexer { return result; } + /** + * Read a flag, including its `=value` if present + * + * Unlike readWord, this does not stop at `=`: a flag's value is part of the + * flag (`--mount=type=cache,id=store`, `--from=builder`). Stopping at the + * first `=` leaves the value behind as free text, where an instruction parser + * reads it as the start of the command. + */ + private readFlag(): string { + let result = ''; + while (!this.isAtEnd()) { + const char = this.peek(); + if (char === ' ' || char === '\t' || char === '\n' || char === '\r') { + break; + } + if (char === this.escapeChar && (this.peek(1) === '\n' || this.peek(1) === '\r')) { + // Line continuation + this.advance(); + if (this.peek() === '\r') this.advance(); + if (this.peek() === '\n') this.advance(); + continue; + } + result += this.advance(); + } + return result; + } + /** * Read a comment or directive */ @@ -389,9 +416,9 @@ export class Lexer { }; } - // Flag (--something) + // Flag (--something, or --something=value) if (char === '-' && this.peek(1) === '-') { - const value = this.readWord(); + const value = this.readFlag(); return { type: TokenType.FLAG, value, diff --git a/packages/docker-parser/src/parser.ts b/packages/docker-parser/src/parser.ts index 90629891..14fe957b 100644 --- a/packages/docker-parser/src/parser.ts +++ b/packages/docker-parser/src/parser.ts @@ -4,6 +4,7 @@ import { Lexer, Token, TokenType } from './lexer'; import { AddInstruction, ArgInstruction, + BaseNode, CmdInstruction, Comment, CopyInstruction, @@ -46,6 +47,10 @@ export class Parser { private pos: number = 0; private escapeChar: string = '\\'; private options: ParserOptions; + /** Comments read but not yet attached to the node they lead. */ + private pendingComments: Comment[] = []; + /** Whether a blank line preceded the node about to be parsed. */ + private pendingBlank: boolean = false; constructor(options: ParserOptions = {}) { this.options = { @@ -98,7 +103,9 @@ export class Parser { dockerfile.directives.push(this.parseDirective()); } else if (token.type === TokenType.COMMENT) { if (this.options.includeComments) { - dockerfile.comments.push(this.parseComment()); + const comment = this.parseComment(); + dockerfile.comments.push(comment); + this.pendingComments.push(comment); } else { this.advance(); } @@ -118,7 +125,9 @@ export class Parser { if (token.type === TokenType.COMMENT) { if (this.options.includeComments) { - dockerfile.comments.push(this.parseComment()); + const comment = this.takeBlank(this.parseComment()); + dockerfile.comments.push(comment); + this.pendingComments.push(comment); } else { this.advance(); } @@ -127,19 +136,20 @@ export class Parser { if (token.type === TokenType.FROM) { const from = this.parseFrom(); - currentStage = { + currentStage = this.takeLeading({ type: 'Stage', from, instructions: [], name: from.name, range: from.range - }; + } as Stage); dockerfile.stages.push(currentStage); continue; } // Parse other instructions - const instruction = this.parseInstruction(); + const parsed = this.parseInstruction(); + const instruction = parsed ? this.takeLeading(parsed) : parsed; if (instruction) { if (currentStage) { currentStage.instructions.push(instruction); @@ -162,6 +172,13 @@ export class Parser { } } + // Comments after the last instruction lead nothing; keep them so the file + // can be deparsed without losing its tail. + if (this.pendingComments.length > 0) { + dockerfile.trailingComments = this.pendingComments; + this.pendingComments = []; + } + return dockerfile; } @@ -194,17 +211,52 @@ export class Parser { } /** - * Skip whitespace and newlines + * Skip whitespace and newlines, recording whether a blank line was crossed */ private skipWhitespaceAndNewlines(): void { + let newlines = 0; while (!this.isAtEnd()) { const token = this.peek(); - if (token.type === TokenType.WHITESPACE || token.type === TokenType.NEWLINE) { + if (token.type === TokenType.NEWLINE) { + newlines++; + this.advance(); + } else if (token.type === TokenType.WHITESPACE) { this.advance(); } else { break; } } + // Two newlines in a row means an empty line between instructions. At the + // very start of the file there is no preceding instruction to separate from. + if (newlines > 1 && this.pos > newlines) { + this.pendingBlank = true; + } + } + + /** + * Attach the comments and blank line collected since the last node + */ + private takeLeading(node: T): T { + if (this.pendingComments.length > 0) { + node.leadingComments = this.pendingComments; + this.pendingComments = []; + } + return this.takeBlank(node); + } + + /** + * Attach only the blank line collected since the last node + * + * A comment takes the blank line above it but not the comments above it: + * consecutive comments are siblings in one block leading the same + * instruction, not a chain where each leads the next. + */ + private takeBlank(node: T): T { + if (this.pendingBlank) { + node.blankBefore = true; + this.pendingBlank = false; + } + return node; } /** diff --git a/packages/docker-parser/src/types.ts b/packages/docker-parser/src/types.ts index 42e0c3e7..7631722f 100644 --- a/packages/docker-parser/src/types.ts +++ b/packages/docker-parser/src/types.ts @@ -21,6 +21,17 @@ export interface Range { export interface BaseNode { type: string; range?: Range; + /** + * Comment lines immediately above this node, in source order. + * + * A Dockerfile's comments explain the instruction they sit on — why a COPY is + * split out, what busts a cache layer — so they belong to that node rather + * than to the file. The deparser emits them, which is what lets a generated + * Dockerfile carry its reasoning and a parsed one survive a round-trip. + */ + leadingComments?: Comment[]; + /** Emit one blank line above this node (and above its leading comments). */ + blankBefore?: boolean; } /** @@ -30,7 +41,13 @@ export interface Dockerfile extends BaseNode { type: 'Dockerfile'; directives: ParserDirective[]; stages: Stage[]; + /** + * Every comment in the file, in source order, whether or not it is also + * attached to a node as a leading comment. + */ comments: Comment[]; + /** Comments after the last instruction, which lead no node. */ + trailingComments?: Comment[]; } /**