fix(docker-parser): parse flag values and keep comments through a deparse - #103
Merged
Merged
Conversation
…arse A flag's token stopped at the first '=' , so RUN --mount=type=cache,id=... lexed as '--mount' and left '=type=cache,...' to be read as the start of the command: the mount lost its id and target, defaulted to type=bind, and deparsed as a corrupt 'RUN --mount=type=bind =type=cache,... '. Comments parsed into a flat Dockerfile.comments array that the deparser never emitted, so every comment vanished on a round-trip. They now attach to the node below them as leadingComments, with a blank line above a node as blankBefore, and the deparser emits both — which is also what lets a generated Dockerfile carry the reasoning for its layers.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs that a Dockerfile generator runs straight into. Found while replacing
constructive-db's string-template Dockerfiles withdocker-parser— the templates use--mount=type=cacheand are heavily commented, and both were destroyed by a round-trip.1. A flag lost its value.
readWord()stops at=, so the FLAG token was just--mountand the rest stayed in the stream, whereparseRunread it as the beginning of the command:parseMountFlagwas already correct — it never received the value. Fixed with areadFlag()that reads through=to whitespace, so--mount,--from,--platform,--networkand--securityall keep theirs. (Only--mountwas corrupted in output; the others survived because their=valuere-lexed into the same string.)2. Every comment vanished. Comments parsed into a flat
Dockerfile.commentsarray thatdeparseDockerfilenever emitted. They now attach to the node they lead:Dockerfile.commentsstill holds every comment, so existing consumers are unaffected. A comment takes the blank line above itself, which keeps# header/ blank /COPYdistinct from# header/COPY— and consecutive comments stay siblings of one block rather than a chain where each leads the next. The deparser no longer inserts a blank line the AST didn't ask for (it used to add one after the directives), since an invented blank comes back asblankBeforeon the next parse and breaks the round-trip.Together this makes
parse → deparselossless overconstructive-db's four Dockerfiles apart from cosmetics: mount options come back in canonical order, and aRUNcontinued with\deparses onto one line (noted in the README; not addressed here).The point of (2) beyond fidelity: a generated Dockerfile can now carry the reasoning for its layers, which is what a hand-written template gave us for free and the main thing lost by generating one.
Tests
96 (was 78). New: 5 round-trips for
RUNmount/network/security flags, 6 for comment placement, and exact-output assertions indeparser.test.ts— including one buildingleadingComments/blankBeforeby hand with no parse, which is the generator's path.Link to Devin session: https://app.devin.ai/sessions/f1a21b66c33d45d099297808a227cd3e
Requested by: @pyramation