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.

Sign up for GitHub

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

Jump to bottom

More colorful error messages #16356

Closed
wants to merge 5 commits into from
Closed

More colorful error messages #16356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e88afe5
Squashed commits, and rebased colour PR
beef331 Dec 16, 2020
c5b754e
Fixed tests failing due to missing quote expr
beef331 Jul 9, 2021
6872fed
Fixed failing tests
beef331 Jul 10, 2021
8598e59
Fixed double quote issue
beef331 Jul 10, 2021
a4c396e
Fixed last issue test
beef331 Jul 10, 2021
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
72 changes: 65 additions & 7 deletions compiler/msgs.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#

import
std/[strutils, os, tables, terminal, macros, times],
std/[strutils, os, tables, terminal, macros, times, colors, strformat, parseutils],
std/private/miscdollars,
options, ropes, lineinfos, pathutils, strutils2

Expand Down Expand Up @@ -477,12 +477,68 @@ proc sourceLine*(conf: ConfigRef; i: TLineInfo): string =

result = conf.m.fileInfos[i.fileIndex.int32].lines[i.line.int-1]

proc getSurroundingSrc(conf: ConfigRef; info: TLineInfo): string =
proc colorError(s: string, color: ForegroundColor, conf: ConfigRef): string =
if optUseColors in conf.globalOptions:
template isQuote(val: untyped): untyped = val == '\''
beef331 marked this conversation as resolved.
Show resolved Hide resolved
template isNotQuote(val: untyped): untyped = val != '\''
let parsable = (s.count('"').mod 2) == 0
var
pos = 0
while pos < s.len:
if s[pos].isQuote:
if pos < s.high and s[pos + 1].isQuote:
result.add s[pos..(pos + 1)]
var inDoubleQuote = false
inc pos
let start = pos
while (pos < s.len and s[pos].isNotQuote) or inDoubleQuote:
if s[pos] == '"' and parsable:
inDoubleQuote = not inDoubleQuote
inc pos
#Highlight error
if pos < s.len and parsable:
if (s[pos].isQuote):
result.add fmt"""{color.ansiForegroundColorCode}{s[start..(pos - 1)]}{ansiResetCode}"""
beef331 marked this conversation as resolved.
Show resolved Hide resolved
else:
result.add s[start..(pos - 1)]
else:
result.add s[start..s.high]
break
else:
result.add s[pos]
inc pos
else:
result = s

proc getSurroundingSrc(result: var string, conf: ConfigRef; info: TLineInfo, color: ForegroundColor) =
if conf.hasHint(hintSource) and info != unknownLineInfo:
const indent = " "
result = "\n" & indent & $sourceLine(conf, info)
if info.col >= 0:
result.add "\n" & indent & spaces(info.col) & '^'
if optUseColors in conf.globalOptions:
var msg = $sourceLine(conf, info)
let
colorEnd =
case msg[info.col]:
of IdentStartChars:
info.col + 1 + msg.skipUntil({'\0'..'\255'} - IdentChars, info.col)
of '[': # + 2 due to wanting to include the sym in the coloring
info.col + 2 + msg.skipUntil(']', info.col)
of '(': # + 2 due to wanting to include the sym in the coloring
info.col + 2 + msg.skipUntil(')', info.col)
of '.': # + 2 due to wanting to include the sym in the coloring
info.col + 2 + msg.skipUntil({'.', ' ', '(', '['}, info.col)
else: msg.high

msg.insert("'", info.col)
msg.insert("'", colorEnd)
msg.insert("\n ", 0)

if info.col >= 0:
msg.add "\n" & indent & spaces(info.col) & "'^'"
result.add msg.colorError(color, conf)
else:
result.add "\n" & indent & $sourceLine(conf, info)
if info.col >= 0:
result.add "\n" & indent & spaces(info.col) & '^'

proc formatMsg*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string): string =
let title = case msg
Expand Down Expand Up @@ -539,7 +595,8 @@ proc liMessage*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string,
color = HintColor
inc(conf.hintCounter)

let s = if isRaw: arg else: getMessageStr(msg, arg)
let strMsg = if isRaw: arg else: getMessageStr(msg, arg)
var s = colorError(strMsg, color, conf)
if not ignoreMsg:
let loc = if info != unknownLineInfo: conf.toFileLineCol(info) & " " else: ""
# we could also show `conf.cmdInput` here for `projectIsCmd`
Expand All @@ -550,8 +607,9 @@ proc liMessage*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string,
if msg == hintProcessing and conf.hintProcessingDots:
msgWrite(conf, ".")
else:
getSurroundingSrc(s, conf, info, color)
styledMsgWriteln(styleBright, loc, resetStyle, color, title, resetStyle, s, KindColor, kindmsg,
resetStyle, conf.getSurroundingSrc(info), conf.unitSep)
resetStyle, conf.unitSep)
if hintMsgOrigin in conf.mainPackageNotes:
# xxx needs a bit of refactoring to honor `conf.filenameOption`
styledMsgWriteln(styleBright, toFileLineCol(info2), resetStyle,
Expand Down
24 changes: 12 additions & 12 deletions compiler/semcall.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# included from sem.nim

from algorithm import sort

from renderer import quoteExpr
proc sameMethodDispatcher(a, b: PSym): bool =
result = false
if a.kind == skMethod and b.kind == skMethod:
Expand Down Expand Up @@ -274,8 +274,8 @@ const
errTypeMismatch = "type mismatch: got <"
errButExpected = "but expected one of:"
errUndeclaredField = "undeclared field: '$1'"
errUndeclaredRoutine = "attempting to call undeclared routine: '$1'"
errBadRoutine = "attempting to call routine: '$1'$2"
errUndeclaredRoutine = "attempting to call undeclared routine: $1"
errBadRoutine = "attempting to call routine: $1$2"
errAmbiguousCallXYZ = "ambiguous call; both $1 and $2 match for: $3"

proc notFoundError*(c: PContext, n: PNode, errors: CandidateErrors) =
Expand All @@ -287,7 +287,7 @@ proc notFoundError*(c: PContext, n: PNode, errors: CandidateErrors) =
globalError(c.config, n.info, "type mismatch")
return
if errors.len == 0:
localError(c.config, n.info, "expression '$1' cannot be called" % n[0].renderTree)
localError(c.config, n.info, "expression $1 cannot be called" % n[0].renderTree.quoteExpr)
return

let (prefer, candidates) = presentFailedCandidates(c, n, errors)
Expand Down Expand Up @@ -344,8 +344,8 @@ proc getMsgDiagnostic(c: PContext, flags: TExprFlags, n, f: PNode): string =
let suffix = if result.len > 0: " " & result else: ""
result = errUndeclaredField % ident & typeHint & suffix
else:
if result.len == 0: result = errUndeclaredRoutine % ident
else: result = errBadRoutine % [ident, result]
if result.len == 0: result = errUndeclaredRoutine % ident.quoteExpr
else: result = errBadRoutine % [ident.quoteExpr, result]

proc resolveOverloads(c: PContext, n, orig: PNode,
filter: TSymKinds, flags: TExprFlags,
Expand Down Expand Up @@ -432,8 +432,8 @@ proc resolveOverloads(c: PContext, n, orig: PNode,
return
elif result.state != csMatch:
if nfExprCall in n.flags:
localError(c.config, n.info, "expression '$1' cannot be called" %
renderTree(n, {renderNoComments}))
localError(c.config, n.info, "expression $1 cannot be called" %
renderTree(n, {renderNoComments}).quoteExpr)
else:
if {nfDotField, nfDotSetter} * n.flags != {}:
# clean up the inserted ops
Expand All @@ -457,8 +457,8 @@ proc resolveOverloads(c: PContext, n, orig: PNode,
args.add(")")

localError(c.config, n.info, errAmbiguousCallXYZ % [
getProcHeader(c.config, result.calleeSym),
getProcHeader(c.config, alt.calleeSym),
getProcHeader(c.config, result.calleeSym).quoteExpr,
getProcHeader(c.config, alt.calleeSym).quoteExpr,
args])

proc instGenericConvertersArg*(c: PContext, a: PNode, x: TCandidate) =
Expand Down Expand Up @@ -664,8 +664,8 @@ proc explicitGenericInstantiation(c: PContext, n: PNode, s: PSym): PNode =
# number of generic type parameters:
if s.ast[genericParamsPos].safeLen != n.len-1:
let expected = s.ast[genericParamsPos].safeLen
localError(c.config, getCallLineInfo(n), errGenerated, "cannot instantiate: '" & renderTree(n) &
"'; got " & $(n.len-1) & " typeof(s) but expected " & $expected)
localError(c.config, getCallLineInfo(n), errGenerated, "cannot instantiate: " & renderTree(n) &
"; got " & $(n.len-1) & " typeof(s) but expected " & $expected)
return n
result = explicitGenericSym(c, n, s)
if result == nil: result = explicitGenericInstError(c, n)
Expand Down
4 changes: 2 additions & 2 deletions compiler/types.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ proc typeToString*(typ: PType; prefer: TPreferedDesc = preferName): string

proc addTypeDeclVerboseMaybe*(result: var string, conf: ConfigRef; typ: PType) =
if optDeclaredLocs in conf.globalOptions:
result.add typeToString(typ, preferMixed)
result.add typeToString(typ, preferMixed).quoteExpr
result.addDeclaredLoc(conf, typ)
else:
result.add typeToString(typ)
result.add typeToString(typ).quoteExpr

template `$`*(typ: PType): string = typeToString(typ)

Expand Down
24 changes: 12 additions & 12 deletions tests/concepts/t3330.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,29 @@ t3330.nim(70, 4) Error: type mismatch: got <Bar[system.int]>
but expected one of:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about coloring paths consistently in 1 color? eg:
nim r --colors tests/misc/t9039.nim
shows
image

=> can the ../../lib/system/basic_types.nim(2, 3) be colored like t9039.nim(30, 22) ?

before doing anything, is that hard to do with what you have?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be done in the declared locs using the colors as a control to toggle it.

proc test(foo: Foo[int])
Copy link
Member

@timotheecour timotheecour Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you color expression using a different color than Error / warning / hint ?

eg for
image

using bold white is an option (but would still be good to distinguish expressions from paths but maybe not critical)

somehow it looks good in D:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea this could be done, though I personally dont like the boldness in the D message as i can barely notice a difference(though it may just be your font), ideally we could have a user defined string with a $color as a marker for if they want colour, so then styling could be user definable using colour.

first type mismatch at position: 1
required type for foo: Foo[int]
Copy link
Member

@timotheecour timotheecour Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we shouldn't remove the quotes when coloring an expression, so that copy pasting a msg on terminal gives the same message with --colors:on vs --colors:off (avoids un-necessary complications)

as can be seen in https://github.com/nim-lang/Nim/pull/16356/files#r670864680, the quotes are currently removed when colored

but expression 'bar' is of type: Bar[system.int]
required type for foo: 'Foo[int]'
but expression 'bar' is of type: 'Bar[system.int]'
t3330.nim(55, 8) Hint: Non-matching candidates for add(k, string, T)
proc add(x: var string; y: char)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
required type for x: 'var string'
but expression 'k' is of type: 'Alias'
proc add(x: var string; y: cstring)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
required type for x: 'var string'
but expression 'k' is of type: 'Alias'
proc add(x: var string; y: string)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
required type for x: 'var string'
but expression 'k' is of type: 'Alias'
proc add[T](x: var seq[T]; y: openArray[T])
first type mismatch at position: 1
required type for x: var seq[T]
but expression 'k' is of type: Alias
required type for x: 'var seq[T]'
but expression 'k' is of type: 'Alias'
proc add[T](x: var seq[T]; y: sink T)
first type mismatch at position: 1
required type for x: var seq[T]
but expression 'k' is of type: Alias
required type for x: 'var seq[T]'
but expression 'k' is of type: 'Alias'

t3330.nim(55, 8) template/generic instantiation of `add` from here
t3330.nim(62, 6) Foo: 'bar.value' cannot be assigned to
Expand Down
44 changes: 22 additions & 22 deletions tests/concepts/texplain.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ discard """
texplain.nim(162, 10) Hint: Non-matching candidates for e(y)
proc e(i: int): int
first type mismatch at position: 1
required type for i: int
but expression 'y' is of type: MatchingType
required type for i: 'int'
but expression 'y' is of type: 'MatchingType'

texplain.nim(165, 7) Hint: Non-matching candidates for e(10)
proc e(o: ExplainedConcept): int
first type mismatch at position: 1
required type for o: ExplainedConcept
but expression '10' is of type: int literal(10)
required type for o: 'ExplainedConcept'
but expression '10' is of type: 'int literal(10)'
texplain.nim(128, 6) ExplainedConcept: undeclared field: 'foo'
texplain.nim(128, 6) ExplainedConcept: undeclared field: '.'
texplain.nim(128, 6) ExplainedConcept: expression '.' cannot be called
Expand All @@ -26,8 +26,8 @@ texplain.nim(128, 5) ExplainedConcept: concept predicate failed
texplain.nim(168, 10) Hint: Non-matching candidates for e(10)
proc e(o: ExplainedConcept): int
first type mismatch at position: 1
required type for o: ExplainedConcept
but expression '10' is of type: int literal(10)
required type for o: 'ExplainedConcept'
but expression '10' is of type: 'int literal(10)'
texplain.nim(128, 6) ExplainedConcept: undeclared field: 'foo'
texplain.nim(128, 6) ExplainedConcept: undeclared field: '.'
texplain.nim(128, 6) ExplainedConcept: expression '.' cannot be called
Expand All @@ -43,12 +43,12 @@ texplain.nim(172, 20) Error: type mismatch: got <NonMatchingType>
but expected one of:
proc e(i: int): int
first type mismatch at position: 1
required type for i: int
but expression 'n' is of type: NonMatchingType
required type for i: 'int'
but expression 'n' is of type: 'NonMatchingType'
proc e(o: ExplainedConcept): int
first type mismatch at position: 1
required type for o: ExplainedConcept
but expression 'n' is of type: NonMatchingType
required type for o: 'ExplainedConcept'
but expression 'n' is of type: 'NonMatchingType'
texplain.nim(172, 9) template/generic instantiation of `assert` from here
texplain.nim(128, 5) ExplainedConcept: concept predicate failed

Expand All @@ -57,36 +57,36 @@ texplain.nim(173, 20) Error: type mismatch: got <NonMatchingType>
but expected one of:
proc r(i: string): int
first type mismatch at position: 1
required type for i: string
but expression 'n' is of type: NonMatchingType
required type for i: 'string'
but expression 'n' is of type: 'NonMatchingType'
proc r(o: RegularConcept): int
first type mismatch at position: 1
required type for o: RegularConcept
but expression 'n' is of type: NonMatchingType
required type for o: 'RegularConcept'
but expression 'n' is of type: 'NonMatchingType'
texplain.nim(173, 9) template/generic instantiation of `assert` from here
texplain.nim(132, 5) RegularConcept: concept predicate failed
proc r[T](a: SomeNumber; b: T; c: auto)
first type mismatch at position: 1
required type for a: SomeNumber
but expression 'n' is of type: NonMatchingType
required type for a: 'SomeNumber'
but expression 'n' is of type: 'NonMatchingType'

expression: r(n)
texplain.nim(174, 20) Hint: Non-matching candidates for r(y)
proc r(i: string): int
first type mismatch at position: 1
required type for i: string
but expression 'y' is of type: MatchingType
required type for i: 'string'
but expression 'y' is of type: 'MatchingType'
proc r[T](a: SomeNumber; b: T; c: auto)
first type mismatch at position: 1
required type for a: SomeNumber
but expression 'y' is of type: MatchingType
required type for a: 'SomeNumber'
but expression 'y' is of type: 'MatchingType'

texplain.nim(182, 2) Error: type mismatch: got <MatchingType>
but expected one of:
proc f(o: NestedConcept)
first type mismatch at position: 1
required type for o: NestedConcept
but expression 'y' is of type: MatchingType
required type for o: 'NestedConcept'
but expression 'y' is of type: 'MatchingType'
texplain.nim(132, 6) RegularConcept: undeclared field: 'foo'
texplain.nim(132, 6) RegularConcept: undeclared field: '.'
texplain.nim(132, 6) RegularConcept: expression '.' cannot be called
Expand Down
4 changes: 2 additions & 2 deletions tests/errmsgs/t8434.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ discard """
nimout: '''but expected one of:
proc fun0[T1: int | float | object | array | seq](a1: T1; a2: int)
first type mismatch at position: 1
required type for a1: T1: int or float or object or array or seq
but expression 'byte(1)' is of type: byte
required type for a1: 'T1: int or float or object or array or seq'
but expression 'byte(1)' is of type: 'byte'

expression: fun0(byte(1), 0)
'''
Expand Down
Loading