parser: an escaped line break must not be pulled into the token that precedes it

[Eljees]

related-to: hadolint/hadolint#1060

What I did

COPY/ADD reject a valid Dockerfile when a line continuation follows the source path
without a space before the backslash:

COPY foo.json\
  /root/foo.json

Dockerfile:2:17: unexpected end of line. At least two arguments are required for COPY

Docker builds this file: the line continuation joins the lines and the leading spaces of the
next line separate the two arguments.

The same defect has a silent form, which is arguably worse than the error above — the
instruction parses, but the AST is wrong:

input	AST before	AST after
COPY a b\⏎ c /app	sources ["a", "b c"]	sources ["a", "b", "c"]
COPY --from=build a\⏎ b /app	sources ["a b"]	sources ["a", "b"]
COPY --chown=root:root a\⏎ b /app	sources ["a b"]	sources ["a", "b"]

A path that contains a space in the middle is produced out of two separate paths.

How I did it

someUnless (src/Language/Docker/Parser/Prelude.hs) builds a token out of three
alternatives, and the first one is

castToSpace <$> escapedLineBreaks

escapedLineBreaks returns FoundWhitespace when the continuation is followed by spaces, and
castToSpace turns that into a literal space inside the token. The third alternative already
respects the predicate (“do not swallow this character if it is the delimiter”); the first one
did not. For fileList, which calls someUnless "a file" (== ' ') \sepEndBy1` requiredWhitespace, that means both file paths end up in a single token, and sepEndBy1` sees one path instead of two.

The patch keeps the line break out of the token whenever whitespace ends it — i.e. exactly when
predicate ' ' holds — and leaves it unconsumed so that requiredWhitespace can parse it as the
separator it is. Two callers change behaviour: COPY/ADD paths and the flags that end at a
space; everything that ends at a newline (--from=, RUN, ENV, …) is untouched.

The try added to the third alternative is what makes the token end rather than the parse
fail: that branch consumes the backslash before its notFollowedBy fails, which used to be
unreachable because the first branch always matched a backslash + newline first.

The boundary case is preserved deliberately:

COPY foo.json\
/root/foo.json

Docker joins these two lines without inserting a space, so COPY really does get a single
argument and must fail. There is a test for that (expectFail), so the two cases cannot drift
apart.

How to verify it

cabal test --test-show-details=always

• three new tests in test/Language/Docker/ParseCopySpec.hs and
  test/Language/Docker/ParseAddSpec.hs;
• before the change: 301 examples, 3 failures, two of them with the exact error from the
  issue and one “ASTs are not equal” for the multi-file case;
• after: 301 examples, 0 failures (the 297 pre-existing tests are untouched);
• hlint src/ test/ reports the same 5 hints before and after the patch.

Environment: GHC 9.8.4, cabal test as in .github/workflows/haskell.yml.

[m-ildefons]

Thanks a lot again, this is awesome.