[Testament] Extend and document message testing aids

[quantimnot]

Inline Messages

After discovering the inline messages feature of testament, I realized how it would remove
annoyances I've had testing messages. The inline error messages are very useful when building
up a suite of tests, because you don't have to keep changing the line location as your tests get
changed around. However, inline error messages don't support anything other than error messages
or when the test action is reject. That behavior doesn't allow testing desired hints and warnings
with a test action of compile or run.

I extended the inline message feature by:

• Supporting actions other than reject. This affects these current tests:

  • Inline message not checked because the action is not reject:

    Nim/tests/pragmas/tvar_macro.nim

    Lines 87 to 89 in 8fb4d7e

    	doAssert importedFooBar == fooBar #[tt.Warning
    	^ fooBar is deprecated
    	]#

  • Nim/tests/strictnotnil/tnilcheck.nim

    Lines 1 to 32 in 528b6d1

    	discard """
    	cmd: "nim check $file"
    	action: "reject"
    	"""
    	import tables
    	{.experimental: "strictNotNil".}
    	type
    	Nilable* = ref object
    	a*: int
    	field*: Nilable
    	NonNilable* = Nilable not nil
    	Nilable2* = nil NonNilable
    	# proc `[]`(a: Nilable, b: int): Nilable =
    	# nil
    	# Nilable tests
    	# test deref
    	proc testDeref(a: Nilable) =
    	echo a.a > 0 #[tt.Warning
    	^ can't deref a, it might be nil
    	]#

• Supporting more than one message for a line. I'm making use of this in Fix some styleCheck bugs #20095:

  Nim/tests/stylecheck/thint.nim

  Lines 23 to 26 in f7cfcfe

  	proc generic_proc*[T] {.no_destroy, userPragma.} = #[tt.Hint
  	^ 'generic_proc' should be: 'genericProc' [Name]; tt.Hint
  	^ 'no_destroy' should be: 'nodestroy' [Name]; tt.Hint
  	^ 'userPragma' should be: 'user_pragma' [template declared in thint.nim(7, 9)] [Name] ]#

  
  This change is enabled by:

  • Adding a message delimiter re";\s*tt\." (regex not actually used).

  • Changing the message line determination from the caret's (^) line - 1, to that of the opening inline message pattern #[tt..

• Supporting using it along with nimout. This is useful for simultaneously testing inline messages and expected messages from imported modules. I make use of this in Add module patching (mocking) feature to compiler #19925 where I'm testing inline messages, messages raised in a nimscript, and messages raised in another module:

  Nim/tests/modules/tpatchModule.nim

  Lines 1 to 23 in 1df0600

  	discard """
  	nimout: '''
  	tpatchModule.nims(32, 12) Warning: cannot open: missingTarget_uasdygf8a7fg8uq23vfquoevfqo8ef [CannotOpen]
  	tpatchModule.nims(35, 12) Warning: cannot open: missingPatch_uasdygf8a7fg8uq23vfquoevfqo8ef [CannotOpen]
  	tpatchModule.nims(38, 12) Warning: cannot open: missingTarget_uasdygf8a7fg8uq23vfquoevfqo8ef [CannotOpen]
  	tpatchModule.nims(41, 12) Warning: cannot open: [CannotOpen]
  	tpatchModule.nims(44, 12) Warning: cannot open: [CannotOpen]
  	tpatchModule.nims(47, 12) Warning: cannot open: [CannotOpen]
  	a${/}module_name_clashes.nim(3, 12) Hint: tests/modules/b/module_name_clashes patched with tests/modules/mpatchModule [Patch]
  	'''
  	"""
  	# Test `nimscript.patchModule`
  	#
  	# The other components of this test are:
  	# * `tpatchModule.nims` is the config script to configure the patch.
  	# * `mpatchModule.nim` is the module that patches the target modules.
  	# Test patching foreign and `stdlib` modules:
  	import std/httpclient #[tt.Hint
  	^ std/httpclient patched with tests/modules/mpatchModule [Patch] ]#
  	var client = newHttpClient()
  	doAssert client.getContent("https://example.com") == "patched!"

• Fixed inline message parsing bug that broke multi-line messages. The bug is that newlines are dropped from inline messages:
  

  Nim/testament/specs.nim

  Lines 162 to 165 in 528b6d1

  	if s[result] == '\n':
  	inc result
  	inc line
  	col = 1

  
  There is an existing test that is affected by this:

  Nim/tests/effects/teffects1.nim

  Lines 1 to 44 in 528b6d1

  	discard """
  	cmd: "nim check $file"
  	nimout: '''teffects1.nim(22, 28) template/generic instantiation from here
  	teffects1.nim(23, 13) Error: can raise an unlisted exception: ref IOError
  	teffects1.nim(22, 29) Hint: 'lier' cannot raise 'IO2Error' [XCannotRaiseY]
  	teffects1.nim(38, 21) Error: type mismatch: got <proc (x: int): string{.noSideEffect, gcsafe, locks: 0.}> but expected 'MyProcType = proc (x: int): string{.closure.}'
  	.raise effects differ'''
  	"""
  	{.push warningAsError[Effect]: on.}
  	type
  	TObj {.pure, inheritable.} = object
  	TObjB = object of TObj
  	a, b, c: string
  	IO2Error = ref object of IOError
  	proc forw: int {. .}
  	proc lier(): int {.raises: [IO2Error].} =
  	#[tt.Hint ^ 'lier' cannot raise 'IO2Error' [XCannotRaiseY] ]#
  	writeLine stdout, "arg" #[tt.Error
  	^ can raise an unlisted exception: ref IOError
  	]#
  	proc forw: int =
  	raise newException(IOError, "arg")
  	{.push raises: [Defect].}
  	type
  	MyProcType* = proc(x: int): string #{.raises: [ValueError, Defect].}
  	proc foo(x: int): string {.raises: [ValueError].} =
  	if x > 9:
  	raise newException(ValueError, "Use single digit")
  	$x
  	var p: MyProcType = foo #[tt.Error
  	^
  	type mismatch: got <proc (x: int): string{.noSideEffect, gcsafe, locks: 0.}> but expected 'MyProcType = proc (x: int): string{.closure.}'
  	]#
  	{.pop.}
  	{.pop.}

  What actually happens in this case is:

  • The test spec and inline messages are parsed in testament/specs:
    

    Nim/testament/specs.nim

    Line 181 in 528b6d1

    proc extractSpec(filename: string; spec: var TSpec): string =

  • The test action is internally set to reject, because at least one of the inline messages is an Error message:
    

    Nim/testament/specs.nim

    Line 172 in 528b6d1

    if kind == "Error": spec.action = actionReject

  • nimout is ignored because the test action is reject and there are inline messages:
    

    Nim/testament/testament.nim

    Lines 545 to 547 in 528b6d1

    	of actionReject:
    	let given = callNimCompilerImpl()
    	cmpMsgs(r, expected, given, test, target, extraOptions)

    
    

    Nim/testament/testament.nim

    Lines 390 to 397 in 528b6d1

    	proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest,
    	target: TTarget, extraOptions: string) =
    	if expected.inlineErrors.len > 0:
    	checkForInlineErrors(r, expected, given, test, target, extraOptions)
    	elif strip(expected.msg) notin strip(given.msg):
    	r.addResult(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer)
    	elif not nimoutCheck(expected, given):
    	r.addResult(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer)

  • Compiler messages are compared with inline messages by checking if the actual message contains the inline message instead of if they are identical:

    Nim/testament/testament.nim

    Line 358 in 528b6d1

    x.kind == kind and x.msg in msg:

    
    The message of concern in this test is:

    Nim/tests/effects/teffects1.nim

    Lines 6 to 7 in 528b6d1

    	teffects1.nim(38, 21) Error: type mismatch: got <proc (x: int): string{.noSideEffect, gcsafe, locks: 0.}> but expected 'MyProcType = proc (x: int): string{.closure.}'
    	.raise effects differ'''

    
    The actual message has a newline and .raise effects differ, but the inline message is missing that part:
    

    Nim/tests/effects/teffects1.nim

    Lines 38 to 42 in 528b6d1

    	var p: MyProcType = foo #[tt.Error
    	^
    	type mismatch: got <proc (x: int): string{.noSideEffect, gcsafe, locks: 0.}> but expected 'MyProcType = proc (x: int): string{.closure.}'
    	]#

    
    You can change the inline message to this and it will still pass:

    var p: MyProcType = foo #[tt.Error
                        ^]#

Variable substitution for nimout, errormsg and inline messages

Path separator and test filename variable substitution in nimout, errormsg and inline messages. The path separator is needed to handle messages that contain native filesystem paths. I use this in #19925:

Nim/tests/modules/tpatchModule.nim

Line 9 in 1df0600

a${/}module_name_clashes.nim(3, 12) Hint: tests/modules/b/module_name_clashes patched with tests/modules/mpatchModule [Patch]

Other Bugs

nimout, ccodecheck and maxcodesize are not handled when output or outputsub are defined.
It's useful to be able to write a single test than can:

• Check a subset of compiler messages in nimout.
• Check for a output substring with outputsub.
• Check code gen with ccodecheck and maxcodesize.

The changes for this were done here instead of another PR, because the changes to facilitate using inline messages with actions other than reject also cleanly enabled this.

Breaking Changes

Since this PR changes the behavior of nimout, errormsg and inline messages to be interpolated by means of strutils.%, then the variable marker ($) needs escaped in messages that don't intend any variable substitution.

I think this is a minor breaking change, but a new boolean spec field could be added (ex. subMsg) that would only format those values when it is true.

[quantimnot]

errormsg currently overrides nimout, so I got rid of both in this test and inlined the messages.

[quantimnot]

Again, errormsg currently overrides nimout, so I got rid of both in this test and inlined the messages.

[quantimnot]

@ringabout, thank you for letting me know this PR is confusing. I've tried to clarify it in the first comment.

[quantimnot]

This is ready for review.

Any questions or comments?

[Varriount]

Personally, I'd merge this in now, but @Araq will need to give the final approval.

[quantimnot]

@Varriount Thank you for taking the time to review it.

[github-actions]

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 6289b00

Hint: mm: orc; threads: on; opt: speed; options: -d:release
163925 lines; 12.197s; 842.105MiB peakmem