Skip to content

Commit

Permalink
rework testament diff and minor UX improvements
Browse files Browse the repository at this point in the history
- rework implementation of the message diff in testament. Now plaintext
  diff is implemented using built-in diff written in nim instead of calling
  into separate `git diff` command that produced noisy, colorless output
  and was completely unconfigurable.
- Add implementation of the structured message diff, structured compiler
  reports for the compiler. These changes had to be made togehter,
  otherwise it would've been a series incomplete PRs that depended on each
  other even for the purposes of testing/documentation.
  - Add s-expression formatter to the compiler output. Selected using
    `--msgFormat=sexp` (for now we have a `text` and `sexp`, in the future
    `json` must be added as well, since that's what people actually expect
    from their tooling - not some obscure data format).
  - Fix 'invalid value' error for cli switch - that bug was discovered when
    adding `msgFormat` option.
  - Add structural S-expression diff algorithm with formatting logic for
    mismatches.
  - Add `nimoutFormat: sexp` field for the testament, to allow it
    understand what kind of data will be generated by the compiler itself.
  - Clean up testament message diff handling a little and add checks for
    structural diffs where appropriate.
- Minor related changes
  - Split `strutils.addf` and reuse the interpolation string for colored
    text - otherwise all futher formatting logic looked a lot uglier than
    necessary (using `strformat.&` would require factoring out macro
    implementation parts)
  - Clean up testament documentation for different spec parts, add
    description of the structural messages.
  - More documentation all over testament implementation
  - Move `sexp` file into `experimental/` directory - it is no longer
    related directly to nimsuggest, @saem has already suggested moving it
    in #164
  - Testament no longer has a hard assertion on the path of the tested file
    `testament tfile.nim` can also work, and does not force user to create
    needlessly nested directories for purposes of simple file testing.
  • Loading branch information
haxscramper committed Jan 28, 2022
1 parent 006bdd3 commit cb4dbec
Show file tree
Hide file tree
Showing 24 changed files with 2,742 additions and 242 deletions.
9 changes: 7 additions & 2 deletions compiler/front/cli_reporter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ proc toStr(conf: ConfigRef, loc: TLineInfo, dropExt: bool = false): string =
## Convert location to printable string
conf.wrap(
"$1($2, $3)" % [
toFilenameOption(conf, loc.fileIndex, conf.filenameOption).dropExt(dropExt),
conf.toMsgFilename(loc.fileIndex).dropExt(dropExt),
$loc.line,
$(loc.col + ColOffset)
],
Expand Down Expand Up @@ -3083,7 +3083,12 @@ proc reportBody*(conf: ConfigRef, r: ExternalReport): string =
result = "$1 is not a valid number" % r.cmdlineProvided

of rextInvalidValue:
result = r.cmdlineError
result = ("Unexpected value for " &
"the $1. Expected one of $2, but got '$3'") % [
r.cmdlineSwitch,
r.cmdlineAllowed.mapIt("'" & it & "'").join(", "),
r.cmdlineProvided
]

of rextUnexpectedValue:
result = "Unexpected value for $1. Expected one of $2" % [
Expand Down
16 changes: 15 additions & 1 deletion compiler/front/commands.nim
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ import
front/[
condsyms,
options,
msgs
msgs,
cli_reporter,
sexp_reporter
],
backend/[
extccomp
Expand Down Expand Up @@ -1121,6 +1123,17 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
else:
conf.localReport(info, invalidSwitchValue @["abs", "canonical", "legacyRelProj"])

of "msgformat":
case arg.normalize:
of "text":
conf.setReportHook cli_reporter.reportHook

of "sexp":
conf.setReportHook sexp_reporter.reportHook

else:
conf.localReport(info, invalidSwitchValue @["text", "sexp"])

of "processing":
incl(conf, cnCurrent, rsemProcessing)
incl(conf, cnMainPackage, rsemProcessing)
Expand Down Expand Up @@ -1269,6 +1282,7 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
of "nilseqs", "nilchecks", "mainmodule", "m", "symbol", "taintmode",
"cs", "deadcodeelim":
warningOptionNoop(switch)

else:
if strutils.find(switch, '.') >= 0: options.setConfigVar(conf, switch, arg)
else: invalidCmdLineOption(conf, pass, switch, info)
Expand Down
1 change: 0 additions & 1 deletion compiler/front/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ type
) {.closure.} ## All
## textual output from the compiler goes through this callback.
writeHook*: proc(conf: ConfigRef, output: string, flags: MsgFlags) {.closure.}

structuredReportHook*: ReportHook
cppCustomNamespace*: string
vmProfileData*: ProfileData
Expand Down
185 changes: 185 additions & 0 deletions compiler/front/sexp_reporter.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
## Implementation of the structured CLI message generator. Using
## `--msgFormat=sexp` will make compiler switch to the report hook
## implemented in this module.

import
experimental/[
sexp,
diff,
colortext,
sexp_diff
],
ast/[
lineinfos,
ast,
reports
],
front/[
options,
msgs
],
std/[
strutils
]


var writeConf: ConfigRef

import std/options as std_options

proc addFields[T](s: var SexpNode, r: T, ignore: seq[string] = @[])

proc sexp[T: object | tuple](obj: T): SexpNode =
result = newSList()
addFields(result, obj)

proc sexp[T: object | tuple](obj: ref T): SexpNode =
result = newSList()
addFields(result, obj[])

proc sexp*[E: enum](e: E): SexpNode = newSSymbol($e)

proc sexpItems*[T](s: T): SexpNode =
result = newSList()
for item in items(s):
result.add sexp(item)


proc sexp*[T](s: seq[T]): SexpNode = sexpItems(s)
proc sexp*[R, T](s: array[R, T]): SexpNode = sexpItems(s)
proc sexp*[I](s: set[I]): SexpNode = sexpItems(s)
proc sexp*(s: cstring): SexpNode = sexp($s)

proc sexp*(v: SomeInteger): SexpNode = newSInt(BiggestInt(v))
proc sexp*(id: FileIndex): SexpNode =
sexp(writeConf.toMsgFilename(id))


iterator sexpFields[T](obj: T, ignore: seq[string] = @[]): SexpNode =
for name, value in fieldPairs(obj):
var pass = true
when value is ref or value is ptr:
if isNil(value):
pass = false

when value is seq or value is string:
if len(value) == 0:
pass = false

when value is TLineInfo:
if pass and value == unknownLineInfo:
pass = false

when value is ReportLineInfo:
if pass and value.isValid():
pass = false

if pass and name in ignore:
pass = false

if pass:
yield newSKeyword(name, sexp(value))


proc add*(other: var SexpNode, str: string, expr: SexpNode) =
other.add newSSymbol(":" & str)
other.add expr

proc sexp*[T](o: Option[T]): SexpNode =
if o.isNone: newSNil() else: sexp(o.get())

proc addFields[T](s: var SexpNode, r: T, ignore: seq[string] = @[]) =
for item in sexpFields(r, ignore):
s.add item

proc sexp*(i: ReportLineInfo): SexpNode =
convertSexp([
writeConf.formatPath(i.file).sexp(),
sexp(i.line),
sexp(i.col)
])

proc sexp*(i: TLineInfo): SexpNode =
convertSexp([sexp(i.fileIndex), sexp(i.line), sexp(i.col)])

proc sexp*(e: StackTraceEntry): SexpNode =
result = newSList()
result.addFields(e, @["filename"])
result.add newSKeyword(
"filename", writeConf.formatPath($e.filename).sexp())


proc sexp*(typ: PType): SexpNode =
if typ.isNil: return newSNil()
result = newSList()
result.add newSSymbol(($typ.kind)[2 ..^ 1])
if typ.sons.len > 0:
result.add("sons", sexp(typ.sons))

proc sexp*(node: PNode): SexpNode =
if node.isNil: return newSNil()

result = newSList()
result.add newSSymbol(($node.kind)[2 ..^ 1])
case node.kind:
of nkCharLit..nkUInt64Lit: result.add sexp(node.intVal)
of nkFloatLit..nkFloat128Lit: result.add sexp(node.floatVal)
of nkStrLit..nkTripleStrLit: result.add sexp(node.strVal)
of nkSym: result.add newSSymbol(node.sym.name.s)
of nkIdent: result.add newSSymbol(node.ident.s)
else:
for node in node.sons:
result.add sexp(node)

proc sexp*(t: PSym): SexpNode =
convertSexp([
substr($t.kind, 2).newSSymbol(),
name = sexp(t.name.s),
info = sexp(t.info)
])


proc reportHook*(conf: ConfigRef, r: Report): TErrorHandling =
writeConf = conf
let wkind = conf.writabilityKind(r)

if wkind == writeDisabled:
return

else:
var s = newSList()
s.add newSSymbol(multiReplace($r.kind, {
"rsem": "Sem",
"rpar": "Par",
"rlex": "Lex",
"rint": "Int",
"rext": "Ext",
"rdbg": "Dbg",
"rback": "Bck",
}))
s.add newSSymbol(":severity")
s.add sexp(conf.severity(r))

let f = @["kind"]

case r.category:
of repLexer: s.addFields(r.lexReport, f)
of repParser: s.addFields(r.parserReport, f)
of repCmd: s.addFields(r.cmdReport, f)
of repSem:
if r.kind == rsemProcessingStmt:
s.addFields(r.semReport, f & "node")

else:
s.addFields(r.semReport, f)

of repDebug: s.addFields(r.debugReport)
of repInternal: s.addFields(r.internalReport)
of repBackend: s.addFields(r.backendReport)
of repExternal: s.addFields(r.externalReport)

if wkind == writeForceEnabled:
echo s.toLine().toString(conf.useColor)

else:
conf.writeln(s.toLine().toString(conf.useColor))
2 changes: 1 addition & 1 deletion compiler/sem/semcall.nim
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ proc pickBestCandidate(c: PContext, headSymbol: PNode,
firstMismatch: z.firstMismatch,
diagnostics: z.diagnostics,
isDiagnostic: z.diagnosticsEnabled or efExplain in flags
))
))

else:
# Symbol table has been modified. Restart and pre-calculate all syms
Expand Down
1 change: 0 additions & 1 deletion compiler/vm/vm.nim
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import
parseutils
],
ast/[
astalgo,
lineinfos,
renderer, # toStrLit implementation
trees,
Expand Down
Loading

0 comments on commit cb4dbec

Please sign in to comment.