Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-implement indented string parsing to be more preservative and match the Nix one #210

Merged
merged 2 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions src/Nixfmt/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,50 @@ indentedStringPart :: Parser StringPart
indentedStringPart =
TextPart
<$> someText
( chunk "''\\n"
<|> chunk "''\\r"
<|> chunk "''\\t"
<|> chunk "''\\"
*> (Text.singleton <$> anySingle)
<|> chunk "''$"
<|> chunk "'''"
<|> chunk "$$"
<|> try (chunk "$" <* notFollowedBy (char '{'))
<|> try (chunk "'" <* notFollowedBy (char '\''))
<|> someP (\t -> t /= '\'' && t /= '$' && t /= '\n')
( {-
This should match https://github.com/NixOS/nix/blob/052f1320ddf72d617e337479ff1bf22cb4ee682a/src/libexpr/lexer.l#L182-L205
To understand that file, [this](https://westes.github.io/flex/manual/Matching.html#Matching) is important:
> If it finds more than one match, it takes the one matching the most text.
> If it finds two or more matches of the same length, the rule listed first in the flex input file is chosen.

There are some inconsequential differences though:
- We only parse one line at a time here, so we make sure to not process any newlines
- We don't want to transform the input in any way, e.g. don't turn ''' into ''
This is to preserve the input string as-is
-}

-- <IND_STRING>([^\$\']|\$[^\{\']|\'[^\'\$])+
-- While this rule doesn't have a fixed length, it's non-conflicting with the rules below,
-- so we can do it first without worrying about the length matching
someText
( Text.singleton
<$> satisfy (\t -> t /= '$' && t /= '\'' && t /= '\n')
<|> try (Text.snoc <$> chunk "$" <*> satisfy (\t -> t /= '{' && t /= '\'' && t /= '\n'))
<|> try (Text.snoc <$> chunk "'" <*> satisfy (\t -> t /= '\'' && t /= '$' && t /= '\n'))
)
-- These rules are of length 3, they need to come before shorter ones
-- <IND_STRING>\'\'\$
<|> chunk "''$"
-- <IND_STRING>\'\'\'
<|> chunk "'''"
-- <IND_STRING>\'\'\\{ANY} -> Note that ANY can match newlines as well, but we need to ignore those
<|> do
prefix <- chunk "''\\"
-- If we do have a newline
rest <-
-- If there's no newline, take the next character
(notFollowedBy (char '\n') *> (Text.singleton <$> anySingle))
-- Otherwise there's an unconsumed newline, which we don't need to handle,
-- it's consumed elsewhere
<|> pure ""
pure $ prefix <> rest
-- These are rules with length 2 and 1
-- <IND_STRING>\$\{ -> don't match, this will be an interpolation
-- <IND_STRING>\$ -> do match, just a dollar
<|> try (chunk "$" <* notFollowedBy (char '{'))
-- <IND_STRING>\'\' -> don't match, indented string ends
-- <IND_STRING>\' -> do match, just a quote
<|> try (chunk "'" <* notFollowedBy (char '\''))
)

indentedLine :: Parser [StringPart]
Expand Down
6 changes: 4 additions & 2 deletions test/correct/dollars-before-interpolation.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
let bar = "bar";
let
bar = "bar";

in [
in
[
"$$\${bar}"
"$\$${bar}"
''
Expand Down
14 changes: 14 additions & 0 deletions test/correct/indented-string.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# These come from previous parser bugs in Nix
[
''
''\a
''\

'${true}'

$'\t'
''

''ending dollar $''
''$''
]
3 changes: 2 additions & 1 deletion test/correct/paths-with-interpolations.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ let
foo = "foo";
bar = "bar";

in [
in
[
/${foo}
./${foo}
/foo/${bar}
Expand Down
10 changes: 9 additions & 1 deletion test/correct/quotes-in-inherit-2.nix
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,12 @@ let
${"baz"} = 3;
${"in"} = 4;

in { inherit ${"foo"} bar "baz" "in"; }
in
{
inherit
${"foo"}
bar
"baz"
"in"
;
}
3 changes: 3 additions & 0 deletions test/correct/regression-197.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
''
''\'''${A}
''
2 changes: 2 additions & 0 deletions test/diff/operator-after-operator/out.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# https://github.com/NixOS/nixfmt/issues/122
(1 + 1) (1 + 0.4)
17 changes: 13 additions & 4 deletions test/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,22 @@ fi
# Do a test run to make sure it compiles fine
nixfmt --version

# Verify "correct"
# Verify "correct", files that don't change when formatted
for file in test/correct/*.nix; do
if ! nixfmt --verify < "$file" > /dev/null; then
echo "[ERROR] $file failed nixfmt verification"
echo "Checking $file …"
if ! out=$(nixfmt --verify < "$file"); then
echo "[ERROR] failed nixfmt verification"
exit 1
fi

if diff --color=always --unified "$file" <(echo "$out"); then
echo "[OK]"
elif [[ $* == *--update-diff* ]]; then
echo "$out" > "$file"
echo "[UPDATED] $file"
else
echo "[OK] $file"
echo "[ERROR] Formatting not stable (run with --update-diff to update the diff)"
exit 1
fi
done

Expand Down