From caac3c47d7bf7986509cfb89bf67212a92f714a9 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Sat, 15 Jun 2024 11:14:28 -0500 Subject: [PATCH 1/5] Improve some lexer error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The message mentioned in #5060 was incorrect. This also uses the passed- in `id` for that message and related messages (removing the reliance on ANSI colors for conveying where the error is). And it adds a suggestion on how to avoid the error. It also does some minor adjustment of highlighting – styling individual identifiers rather that a list of them. Fixes #5060 --- parser-typechecker/src/Unison/PrintError.hs | 44 ++++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/parser-typechecker/src/Unison/PrintError.hs b/parser-typechecker/src/Unison/PrintError.hs index d95ffed4a..77dcdd0b0 100644 --- a/parser-typechecker/src/Unison/PrintError.hs +++ b/parser-typechecker/src/Unison/PrintError.hs @@ -1366,31 +1366,45 @@ renderParseErrors s = \case <> style ErrorSite (fromString open) <> ".\n\n" <> excerpt - L.InvalidWordyId _id -> + L.InvalidWordyId id -> Pr.lines - [ "This identifier isn't valid syntax: ", + [ "The identifier " <> style Code id <> " isn't valid syntax: ", "", excerpt, "Here's a few examples of valid syntax: " - <> style Code "abba1', snake_case, Foo.zoink!, 🌻" + <> style Code "abba1'" + <> ", " + <> style Code "snake_case" + <> ", " + <> style Code "Foo.zoink!" + <> ", and " + <> style Code "🌻" ] - L.ReservedWordyId _id -> + L.ReservedWordyId id -> Pr.lines - [ "The identifier used here isn't allowed to be a reserved keyword: ", - "", - excerpt - ] - L.InvalidSymbolyId _id -> - Pr.lines - [ "This infix identifier isn't valid syntax: ", + [ "The identifier " <> style Code id <> " used here is a reserved keyword: ", "", excerpt, - "Here's a few valid examples: " - <> style Code "++, Float./, `List.map`" + Pr.wrap $ + "You can avoid this problem either by renaming the identifier or wrapping it in backticks (like " + <> style Code ("`" <> id <> "`") + <> ")." ] - L.ReservedSymbolyId _id -> + L.InvalidSymbolyId id -> Pr.lines - [ "This identifier is reserved by Unison and can't be used as an operator: ", + [ "The infix identifier " <> style Code id <> " isn’t valid syntax: ", + "", + excerpt, + "Here are a few valid examples: " + <> style Code "++" + <> ", " + <> style Code "Float./" + <> ", and " + <> style Code "`List.map`" + ] + L.ReservedSymbolyId id -> + Pr.lines + [ "The identifier " <> style Code id <> " is reserved by Unison and can't be used as an operator: ", "", excerpt ] From bb4f39fb2feb0525622647b3f4041a1293223155 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 20 Jun 2024 13:44:55 -0400 Subject: [PATCH 2/5] Update error message in transcripts --- unison-src/transcripts/error-messages.output.md | 4 +++- unison-src/transcripts/generic-parse-errors.output.md | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/unison-src/transcripts/error-messages.output.md b/unison-src/transcripts/error-messages.output.md index 525df31ee..eb0154804 100644 --- a/unison-src/transcripts/error-messages.output.md +++ b/unison-src/transcripts/error-messages.output.md @@ -343,10 +343,12 @@ use.keyword.in.namespace = 1 Loading changes detected in scratch.u. - The identifier used here isn't allowed to be a reserved keyword: + The identifier namespace used here is a reserved keyword: 1 | use.keyword.in.namespace = 1 + You can avoid this problem either by renaming the identifier + or wrapping it in backticks (like `namespace` ). ``` ```unison diff --git a/unison-src/transcripts/generic-parse-errors.output.md b/unison-src/transcripts/generic-parse-errors.output.md index b055ba968..039a1fb00 100644 --- a/unison-src/transcripts/generic-parse-errors.output.md +++ b/unison-src/transcripts/generic-parse-errors.output.md @@ -30,10 +30,12 @@ namespace.blah = 1 Loading changes detected in scratch.u. - The identifier used here isn't allowed to be a reserved keyword: + The identifier namespace used here is a reserved keyword: 1 | namespace.blah = 1 + You can avoid this problem either by renaming the identifier + or wrapping it in backticks (like `namespace` ). ``` ```unison From 6f26a1640150541f040caa6a615c64e5d38ad870 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 20 Jun 2024 21:40:07 -0400 Subject: [PATCH 3/5] Add `quoteCode` for printing errors This both colorizes and wraps code in backticks, in order to separate it from surrounding context. --- parser-typechecker/src/Unison/PrintError.hs | 41 +++++++++++-------- .../transcripts/error-messages.output.md | 12 +++--- .../generic-parse-errors.output.md | 2 +- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/parser-typechecker/src/Unison/PrintError.hs b/parser-typechecker/src/Unison/PrintError.hs index 77dcdd0b0..56bf11fcf 100644 --- a/parser-typechecker/src/Unison/PrintError.hs +++ b/parser-typechecker/src/Unison/PrintError.hs @@ -126,6 +126,10 @@ styleAnnotated sty a = (,sty) <$> rangeForAnnotated a style :: s -> String -> Pretty (AnnotatedText s) style sty str = Pr.lit . AT.annotate sty $ fromString str +-- | Applies the color highlighting for `Code`, but also quotes the code, to separate it from the containing context. +quoteCode :: String -> Pretty ColorText +quoteCode = Pr.backticked . style Code + stylePretty :: Color -> Pretty ColorText -> Pretty ColorText stylePretty = Pr.map . AT.annotate @@ -1368,21 +1372,21 @@ renderParseErrors s = \case <> excerpt L.InvalidWordyId id -> Pr.lines - [ "The identifier " <> style Code id <> " isn't valid syntax: ", + [ "The identifier, " <> quoteCode id <> ", isn't valid syntax: ", "", excerpt, "Here's a few examples of valid syntax: " - <> style Code "abba1'" + <> quoteCode "abba1'" <> ", " - <> style Code "snake_case" + <> quoteCode "snake_case" <> ", " - <> style Code "Foo.zoink!" + <> quoteCode "Foo.zoink!" <> ", and " - <> style Code "🌻" + <> quoteCode "🌻" ] L.ReservedWordyId id -> Pr.lines - [ "The identifier " <> style Code id <> " used here is a reserved keyword: ", + [ "The identifier, " <> quoteCode id <> ", used here is a reserved keyword: ", "", excerpt, Pr.wrap $ @@ -1392,19 +1396,19 @@ renderParseErrors s = \case ] L.InvalidSymbolyId id -> Pr.lines - [ "The infix identifier " <> style Code id <> " isn’t valid syntax: ", + [ "The infix identifier, " <> quoteCode id <> ", isn’t valid syntax: ", "", excerpt, "Here are a few valid examples: " - <> style Code "++" + <> quoteCode "++" <> ", " - <> style Code "Float./" + <> quoteCode "Float./" <> ", and " - <> style Code "`List.map`" + <> quoteCode "List.map" ] L.ReservedSymbolyId id -> Pr.lines - [ "The identifier " <> style Code id <> " is reserved by Unison and can't be used as an operator: ", + [ "The identifier, " <> quoteCode id <> ", is reserved by Unison and can't be used as an operator: ", "", excerpt ] @@ -1458,11 +1462,12 @@ renderParseErrors s = \case "", excerpt, Pr.wrap $ - "I was expecting some digits after the '.'," - <> "for example: " - <> style Code (n <> "0") + "I was expecting some digits after the " + <> quoteCode "." + <> ", for example: " + <> quoteCode (n <> "0") <> "or" - <> Pr.group (style Code (n <> "1e37") <> ".") + <> Pr.group (quoteCode (n <> "1e37") <> ".") ] L.MissingExponent n -> Pr.lines @@ -1472,7 +1477,7 @@ renderParseErrors s = \case Pr.wrap $ "I was expecting some digits for the exponent," <> "for example: " - <> Pr.group (style Code (n <> "37") <> ".") + <> Pr.group (quoteCode (n <> "37") <> ".") ] L.TextLiteralMissingClosingQuote _txt -> Pr.lines @@ -1488,7 +1493,7 @@ renderParseErrors s = \case "", "I only know about the following escape characters:", "", - let s ch = style Code (fromString $ "\\" <> [ch]) + let s ch = quoteCode (fromString $ "\\" <> [ch]) in Pr.indentN 2 $ intercalateMap "," s (fst <$> L.escapeChars) ] L.LayoutError -> @@ -1719,7 +1724,7 @@ renderParseErrors s = \case let msg = mconcat [ "This looks like the start of an expression here but I was expecting a binding.", - "\nDid you mean to use a single " <> style Code ":", + "\nDid you mean to use a single " <> quoteCode ":", " here for a type signature?", "\n\n", tokenAsErrorSite s t diff --git a/unison-src/transcripts/error-messages.output.md b/unison-src/transcripts/error-messages.output.md index eb0154804..dacc86ac7 100644 --- a/unison-src/transcripts/error-messages.output.md +++ b/unison-src/transcripts/error-messages.output.md @@ -19,8 +19,8 @@ x = 1. -- missing some digits after the decimal 1 | x = 1. -- missing some digits after the decimal - I was expecting some digits after the '.', for example: 1.0 or - 1.1e37. + I was expecting some digits after the `.` , for example: `1.0` + or `1.1e37`. ``` ```unison @@ -36,7 +36,7 @@ x = 1e -- missing an exponent 1 | x = 1e -- missing an exponent I was expecting some digits for the exponent, for example: - 1e37. + `1e37`. ``` ```unison @@ -52,7 +52,7 @@ x = 1e- -- missing an exponent 1 | x = 1e- -- missing an exponent I was expecting some digits for the exponent, for example: - 1e-37. + `1e-37`. ``` ```unison @@ -68,7 +68,7 @@ x = 1E+ -- missing an exponent 1 | x = 1E+ -- missing an exponent I was expecting some digits for the exponent, for example: - 1e+37. + `1e+37`. ``` ### Hex, octal, and bytes literals @@ -343,7 +343,7 @@ use.keyword.in.namespace = 1 Loading changes detected in scratch.u. - The identifier namespace used here is a reserved keyword: + The identifier, `namespace`, used here is a reserved keyword: 1 | use.keyword.in.namespace = 1 diff --git a/unison-src/transcripts/generic-parse-errors.output.md b/unison-src/transcripts/generic-parse-errors.output.md index 039a1fb00..3a1c7b19e 100644 --- a/unison-src/transcripts/generic-parse-errors.output.md +++ b/unison-src/transcripts/generic-parse-errors.output.md @@ -30,7 +30,7 @@ namespace.blah = 1 Loading changes detected in scratch.u. - The identifier namespace used here is a reserved keyword: + The identifier, `namespace`, used here is a reserved keyword: 1 | namespace.blah = 1 From 10d26229910ac77024ba6072c80f7bf6a2af0174 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Sun, 23 Jun 2024 22:09:51 -0500 Subject: [PATCH 4/5] Remove unused error case --- parser-typechecker/src/Unison/PrintError.hs | 14 -------------- unison-syntax/src/Unison/Syntax/Lexer.hs | 3 +-- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/parser-typechecker/src/Unison/PrintError.hs b/parser-typechecker/src/Unison/PrintError.hs index 56bf11fcf..835b2c4e2 100644 --- a/parser-typechecker/src/Unison/PrintError.hs +++ b/parser-typechecker/src/Unison/PrintError.hs @@ -1370,20 +1370,6 @@ renderParseErrors s = \case <> style ErrorSite (fromString open) <> ".\n\n" <> excerpt - L.InvalidWordyId id -> - Pr.lines - [ "The identifier, " <> quoteCode id <> ", isn't valid syntax: ", - "", - excerpt, - "Here's a few examples of valid syntax: " - <> quoteCode "abba1'" - <> ", " - <> quoteCode "snake_case" - <> ", " - <> quoteCode "Foo.zoink!" - <> ", and " - <> quoteCode "🌻" - ] L.ReservedWordyId id -> Pr.lines [ "The identifier, " <> quoteCode id <> ", used here is a reserved keyword: ", diff --git a/unison-syntax/src/Unison/Syntax/Lexer.hs b/unison-syntax/src/Unison/Syntax/Lexer.hs index 9938e2e41..e708bc772 100644 --- a/unison-syntax/src/Unison/Syntax/Lexer.hs +++ b/unison-syntax/src/Unison/Syntax/Lexer.hs @@ -95,8 +95,7 @@ parseFailure :: EP.ParseError [Char] (Token Err) -> P a parseFailure e = PI.ParsecT $ \s _ _ _ eerr -> eerr e s data Err - = InvalidWordyId String - | ReservedWordyId String + = ReservedWordyId String | InvalidSymbolyId String | ReservedSymbolyId String | InvalidShortHash String From 782ac4164d54c02457bcbc10134fce22894bbd0b Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Sun, 23 Jun 2024 22:18:51 -0500 Subject: [PATCH 5/5] Remove redundant `,` from lexer errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Appositives only need to be offset by commas if there isn’t already some other punctuation. --- parser-typechecker/src/Unison/PrintError.hs | 6 +++--- unison-src/transcripts/error-messages.output.md | 2 +- unison-src/transcripts/generic-parse-errors.output.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/parser-typechecker/src/Unison/PrintError.hs b/parser-typechecker/src/Unison/PrintError.hs index 835b2c4e2..d6e50ebbb 100644 --- a/parser-typechecker/src/Unison/PrintError.hs +++ b/parser-typechecker/src/Unison/PrintError.hs @@ -1372,7 +1372,7 @@ renderParseErrors s = \case <> excerpt L.ReservedWordyId id -> Pr.lines - [ "The identifier, " <> quoteCode id <> ", used here is a reserved keyword: ", + [ "The identifier " <> quoteCode id <> " used here is a reserved keyword: ", "", excerpt, Pr.wrap $ @@ -1382,7 +1382,7 @@ renderParseErrors s = \case ] L.InvalidSymbolyId id -> Pr.lines - [ "The infix identifier, " <> quoteCode id <> ", isn’t valid syntax: ", + [ "The infix identifier " <> quoteCode id <> " isn’t valid syntax: ", "", excerpt, "Here are a few valid examples: " @@ -1394,7 +1394,7 @@ renderParseErrors s = \case ] L.ReservedSymbolyId id -> Pr.lines - [ "The identifier, " <> quoteCode id <> ", is reserved by Unison and can't be used as an operator: ", + [ "The identifier " <> quoteCode id <> " is reserved by Unison and can't be used as an operator: ", "", excerpt ] diff --git a/unison-src/transcripts/error-messages.output.md b/unison-src/transcripts/error-messages.output.md index dacc86ac7..82ae8a88b 100644 --- a/unison-src/transcripts/error-messages.output.md +++ b/unison-src/transcripts/error-messages.output.md @@ -343,7 +343,7 @@ use.keyword.in.namespace = 1 Loading changes detected in scratch.u. - The identifier, `namespace`, used here is a reserved keyword: + The identifier `namespace` used here is a reserved keyword: 1 | use.keyword.in.namespace = 1 diff --git a/unison-src/transcripts/generic-parse-errors.output.md b/unison-src/transcripts/generic-parse-errors.output.md index 3a1c7b19e..7800cbab4 100644 --- a/unison-src/transcripts/generic-parse-errors.output.md +++ b/unison-src/transcripts/generic-parse-errors.output.md @@ -30,7 +30,7 @@ namespace.blah = 1 Loading changes detected in scratch.u. - The identifier, `namespace`, used here is a reserved keyword: + The identifier `namespace` used here is a reserved keyword: 1 | namespace.blah = 1