Skip to content

Commit

Permalink
Fix formatting of T.Some (#2608)
Browse files Browse the repository at this point in the history
Fixes #2601

According to the standard the `Some` needs to be escaped when used as
a field accessor because the `any-label` grammar rule kicks in, which
specifically does not permit `Some`.
  • Loading branch information
Gabriella439 authored Oct 5, 2024
1 parent 40d0d39 commit dc48911
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 23 deletions.
7 changes: 4 additions & 3 deletions dhall-lsp-server/src/Dhall/LSP/Backend/Completion.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Dhall.Context (Context, empty, insert, toList)
import Dhall.LSP.Backend.Diagnostics (Position, positionToOffset)
import Dhall.LSP.Backend.Parsing (holeExpr)
import Dhall.Parser (Src, exprFromText)
import Dhall.Pretty (UnescapedLabel(..))
import Dhall.TypeCheck (typeOf, typeWithA)
import System.Directory (doesDirectoryExist, listDirectory)
import System.Environment (getEnvironment)
Expand Down Expand Up @@ -186,9 +187,9 @@ completeProjections (CompletionContext context values) expr =
-- complete a union constructor by inspecting the union value
completeUnion _A (Union m) =
let constructor (k, Nothing) =
Completion (Dhall.Pretty.escapeLabel True k) (Just _A)
Completion (Dhall.Pretty.escapeLabel AnyLabelOrSome k) (Just _A)
constructor (k, Just v) =
Completion (Dhall.Pretty.escapeLabel True k) (Just (Pi mempty k v _A))
Completion (Dhall.Pretty.escapeLabel AnyLabelOrSome k) (Just (Pi mempty k v _A))
in map constructor (Dhall.Map.toList m)
completeUnion _ _ = []

Expand All @@ -197,5 +198,5 @@ completeProjections (CompletionContext context values) expr =
completeRecord (Record m) = map toCompletion (Dhall.Map.toList $ recordFieldValue <$> m)
where
toCompletion (name, typ) =
Completion (Dhall.Pretty.escapeLabel True name) (Just typ)
Completion (Dhall.Pretty.escapeLabel AnyLabel name) (Just typ)
completeRecord _ = []
1 change: 1 addition & 0 deletions dhall/src/Dhall/Pretty.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module Dhall.Pretty
, Dhall.Pretty.Internal.layoutOpts

, escapeEnvironmentVariable
, UnescapedLabel(..)
, escapeLabel

, temporalToText
Expand Down
51 changes: 35 additions & 16 deletions dhall/src/Dhall/Pretty/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module Dhall.Pretty.Internal (
, prettyEnvironmentVariable

, prettyConst
, UnescapedLabel(..)
, escapeLabel
, prettyLabel
, prettyAnyLabel
Expand Down Expand Up @@ -518,26 +519,44 @@ headCharacter c = alpha c || c == '_'
tailCharacter :: Char -> Bool
tailCharacter c = alphaNum c || c == '_' || c == '-' || c == '/'

-- | The set of labels which do not need to be escaped
data UnescapedLabel
= NonReservedLabel
-- ^ This corresponds to the `nonreserved-label` rule in the grammar
| AnyLabel
-- ^ This corresponds to the `any-label` rule in the grammar
| AnyLabelOrSome
-- ^ This corresponds to the `any-label-or-some` rule in the grammar

-- | Escape a label if it is not valid when unquoted
escapeLabel :: Bool -> Text -> Text
escapeLabel allowReserved l =
escapeLabel :: UnescapedLabel -> Text -> Text
escapeLabel allowedLabel l =
case Text.uncons l of
Just (h, t)
| headCharacter h && Text.all tailCharacter t && (notReservedIdentifier || (allowReserved && someOrNotLanguageKeyword)) && l /= "?"
| headCharacter h && Text.all tailCharacter t && allowed && l /= "?"
-> l
_ -> "`" <> l <> "`"
where
notReservedIdentifier = not (Data.HashSet.member l reservedIdentifiers)
someOrNotLanguageKeyword = l == "Some" || not (Data.HashSet.member l reservedKeywords)
where
allowed = case allowedLabel of
NonReservedLabel -> notReservedIdentifier
AnyLabel -> notReservedKeyword
AnyLabelOrSome -> notReservedKeyword || l == "Some"

notReservedIdentifier = not (Data.HashSet.member l reservedIdentifiers)

prettyLabelShared :: Bool -> Text -> Doc Ann
notReservedKeyword = not (Data.HashSet.member l reservedKeywords)

prettyLabelShared :: UnescapedLabel -> Text -> Doc Ann
prettyLabelShared b l = label (Pretty.pretty (escapeLabel b l))

prettyLabel :: Text -> Doc Ann
prettyLabel = prettyLabelShared False
prettyLabel = prettyLabelShared NonReservedLabel

prettyAnyLabel :: Text -> Doc Ann
prettyAnyLabel = prettyLabelShared True
prettyAnyLabel = prettyLabelShared AnyLabel

prettyAnyLabelOrSome :: Text -> Doc Ann
prettyAnyLabelOrSome = prettyLabelShared AnyLabelOrSome

prettyKeys
:: Foldable list
Expand Down Expand Up @@ -571,7 +590,7 @@ prettyKeys prettyK keys = Pretty.group (Pretty.flatAlt long short)
prettyLabels :: [Text] -> Doc Ann
prettyLabels a
| null a = lbrace <> rbrace
| otherwise = braces (map (duplicate . prettyAnyLabel) a)
| otherwise = braces (map (duplicate . prettyAnyLabelOrSome) a)

prettyNumber :: Integer -> Doc Ann
prettyNumber = literal . Pretty.pretty
Expand Down Expand Up @@ -846,7 +865,7 @@ prettyPrinters characterSet =
prettyKeyValue prettyKey prettyOperatorExpression equals
(makeKeyValue b c)

prettyKey (WithLabel text) = prettyAnyLabel text
prettyKey (WithLabel text) = prettyAnyLabelOrSome text
prettyKey WithQuestion = syntax "?"
prettyExpression (Assert a) =
Pretty.group (Pretty.flatAlt long short)
Expand Down Expand Up @@ -1558,7 +1577,7 @@ prettyPrinters characterSet =
prettyRecord :: Pretty a => Map Text (RecordField Src a) -> Doc Ann
prettyRecord =
( braces
. map (prettyKeyValue prettyAnyLabel prettyExpression colon . adapt)
. map (prettyKeyValue prettyAnyLabelOrSome prettyExpression colon . adapt)
. Map.toList
)
where
Expand Down Expand Up @@ -1615,14 +1634,14 @@ prettyPrinters characterSet =
| Var (V key' 0) <- Dhall.Syntax.shallowDenote val
, key == key'
, not (containsComment mSrc2) ->
duplicate (prettyKeys prettyAnyLabel [(mSrc0, key, mSrc1)])
duplicate (prettyKeys prettyAnyLabelOrSome [(mSrc0, key, mSrc1)])
_ ->
prettyKeyValue prettyAnyLabel prettyExpression equals kv
prettyKeyValue prettyAnyLabelOrSome prettyExpression equals kv

prettyAlternative (key, Just val) =
prettyKeyValue prettyAnyLabel prettyExpression colon (makeKeyValue (pure key) val)
prettyKeyValue prettyAnyLabelOrSome prettyExpression colon (makeKeyValue (pure key) val)
prettyAlternative (key, Nothing) =
duplicate (prettyAnyLabel key)
duplicate (prettyAnyLabelOrSome key)

prettyUnion :: Pretty a => Map Text (Maybe (Expr Src a)) -> Doc Ann
prettyUnion =
Expand Down
8 changes: 4 additions & 4 deletions dhall/src/Dhall/TypeCheck.hs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import Dhall.Eval
, Val (..)
, (~>)
)
import Dhall.Pretty (Ann)
import Dhall.Pretty (Ann, UnescapedLabel(..))
import Dhall.Src (Src)
import Lens.Family (over)
import Prettyprinter (Doc, Pretty (..), vsep)
Expand Down Expand Up @@ -2915,7 +2915,7 @@ prettyTypeMessage (InvalidDuplicateField k expr0 expr1) =
\ \n\
\... which is not a record type \n"
where
txt0 = insert (Dhall.Pretty.Internal.escapeLabel True k)
txt0 = insert (Dhall.Pretty.Internal.escapeLabel AnyLabelOrSome k)
txt1 = insert expr0
txt2 = insert expr1

Expand Down Expand Up @@ -3073,7 +3073,7 @@ prettyTypeMessage (DuplicateFieldCannotBeMerged ks) = ErrorMessages {..}
\ \n\
\" <> txt1 <> "\n"
where
txt0 = insert (Dhall.Pretty.Internal.escapeLabel True (NonEmpty.head ks))
txt0 = insert (Dhall.Pretty.Internal.escapeLabel AnyLabelOrSome (NonEmpty.head ks))

txt1 = insert (toPath ks)

Expand Down Expand Up @@ -5055,7 +5055,7 @@ checkContext context =
toPath :: (Functor list, Foldable list) => list Text -> Text
toPath ks =
Text.intercalate "."
(Foldable.toList (fmap (Dhall.Pretty.Internal.escapeLabel True) ks))
(Foldable.toList (fmap (Dhall.Pretty.Internal.escapeLabel AnyLabelOrSome) ks))

duplicateElement :: Ord a => [a] -> Maybe a
duplicateElement = go Data.Set.empty
Expand Down
11 changes: 11 additions & 0 deletions dhall/tests/format/issue2601A.dhall
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
let T = < Some | Type >

let t
: T
= T.`Some`

let x
: T
= T.Type

in True
11 changes: 11 additions & 0 deletions dhall/tests/format/issue2601B.dhall
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
let T = < Some | Type >

let t
: T
= T.`Some`

let x
: T
= T.Type

in True
27 changes: 27 additions & 0 deletions nix/packages/lsp-test.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{ mkDerivation, aeson, aeson-pretty, ansi-terminal, async, base
, bytestring, co-log-core, conduit, conduit-parse, containers
, data-default, Diff, directory, exceptions, extra, filepath, Glob
, hspec, lens, lib, lsp, lsp-types, mtl, parser-combinators
, process, row-types, some, text, time, transformers, unix
, unliftio
}:
mkDerivation {
pname = "lsp-test";
version = "0.15.0.1";
sha256 = "ad5be9baa344337b87958dfeb765e3edceca47c4ada57fb1ffeccf4056c57ad8";
libraryHaskellDepends = [
aeson aeson-pretty ansi-terminal async base bytestring co-log-core
conduit conduit-parse containers data-default Diff directory
exceptions filepath Glob lens lsp lsp-types mtl parser-combinators
process row-types some text time transformers unix
];
testHaskellDepends = [
aeson base co-log-core containers data-default directory filepath
hspec lens lsp mtl parser-combinators process text unliftio
];
testToolDepends = [ lsp ];
benchmarkHaskellDepends = [ base extra lsp process ];
homepage = "https://github.com/haskell/lsp/blob/master/lsp-test/README.md";
description = "Functional test framework for LSP servers";
license = lib.licenses.bsd3;
}

0 comments on commit dc48911

Please sign in to comment.