From cbe995c9ffa5fc9c3539101f5079fde9c7432378 Mon Sep 17 00:00:00 2001 From: Joe Hermaszewski Date: Fri, 16 Oct 2020 19:13:03 +0800 Subject: [PATCH 1/2] Correct location annotation for parenthesized expressions At the moment parentheses are not included in the location annotation for nix expressions. This changes the code to include any parentheses in the SrcSpan of the underlying expression. Information which is difficult to recover otherwise. --- src/Nix/Parser.hs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Nix/Parser.hs b/src/Nix/Parser.hs index a3b43cc9b..1ef4d3d45 100644 --- a/src/Nix/Parser.hs +++ b/src/Nix/Parser.hs @@ -58,6 +58,7 @@ import Data.Char ( isAlpha , isSpace ) import Data.Data ( Data(..) ) +import Data.Fix ( Fix(..) ) import Data.Functor import Data.Functor.Identity import Data.HashSet ( HashSet ) @@ -195,8 +196,11 @@ nixBool = nixNull :: Parser NExprLoc nixNull = annotateLocation1 (mkNullF <$ reserved "null" "null") +-- | 'nixTopLevelForm' returns an expression annotated with a source position, +-- however this position doesn't include the parsed parentheses, so remove the +-- "inner" location annotateion and annotate again, including the parentheses. nixParens :: Parser NExprLoc -nixParens = parens nixToplevelForm "parens" +nixParens = annotateLocation1 (stripAnn . unFix <$> (parens nixToplevelForm "parens")) nixList :: Parser NExprLoc nixList = annotateLocation1 (brackets (NList <$> many nixTerm) "list") From b00c6fa05da0029549c2dcf20b8072ff06f9d8bf Mon Sep 17 00:00:00 2001 From: Joe Hermaszewski Date: Thu, 29 Oct 2020 10:30:24 +0800 Subject: [PATCH 2/2] Restrict type of parens and brackets to prevent negative use of NExprLoc it is not generally appropriate to have higher-order parsers operate on annotated locations as they are liable to perform changes to the parser which are not captured in the annotation. See https://github.com/haskell-nix/hnix/pull/739 and https://github.com/haskell-nix/hnix/pull/744 for examples. --- src/Nix/Parser.hs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Nix/Parser.hs b/src/Nix/Parser.hs index 1ef4d3d45..fa45086ef 100644 --- a/src/Nix/Parser.hs +++ b/src/Nix/Parser.hs @@ -200,7 +200,7 @@ nixNull = annotateLocation1 (mkNullF <$ reserved "null" "null") -- however this position doesn't include the parsed parentheses, so remove the -- "inner" location annotateion and annotate again, including the parentheses. nixParens :: Parser NExprLoc -nixParens = annotateLocation1 (stripAnn . unFix <$> (parens nixToplevelForm "parens")) +nixParens = annotateLocation1 (parens (stripAnn . unFix <$> nixToplevelForm) "parens") nixList :: Parser NExprLoc nixList = annotateLocation1 (brackets (NList <$> many nixTerm) "list") @@ -411,7 +411,7 @@ nixBinders = (inherit <+> namedVar) `endBy` semi where <*> (equals *> nixToplevelForm) <*> pure p "variable binding" - scope = parens nixToplevelForm "inherit scope" + scope = nixParens "inherit scope" keyName :: Parser (NKeyName NExprLoc) keyName = dynamicKey <+> staticKey where @@ -496,6 +496,13 @@ identifier = lexeme $ try $ do where identLetter x = isAlpha x || isDigit x || x == '_' || x == '\'' || x == '-' +-- We restrict the type of 'parens' and 'brackets' here because if they were to +-- take a @Parser NExprLoc@ argument they would parse additional text which +-- wouldn't be captured in the source location annotation. +-- +-- Braces and angles in hnix don't enclose a single expression so this type +-- restriction would not be useful. +parens, brackets :: Parser (NExprF f) -> Parser (NExprF f) parens = between (symbol "(") (symbol ")") braces = between (symbol "{") (symbol "}") -- angles = between (symbol "<") (symbol ">")