Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
nimsuggest: fix and re-enable old tests (nim-lang#16401)
Browse files Browse the repository at this point in the history
A number of nimsuggest tests were disabled for various reasons, sometimes due
to brittleness. These tests have been fixed where needed and most have are now
enabled -- details below. The updates are meant to provide better regression
coverage for future nimsuggest improvements. To avoid brittleness some tests
were refactored.

Impact:
* test coverage has now increased
* faster execution of the test suite
* tests are less likely to break due to stdlib changes

Re-enabled Test & Test Description:
* `tchk1.nim`: check (chk) via nimsuggest works at end of file
* `tdot4.nim`: prioritize already used completion
* `tinclude.nim`: definition lookup (def) with includes
* `tstrutils.nim` -> `tdef2.nim`: test template definition lookup (def)
* `tsug_regression.nim`: regression test for [nimsuggest nim-lang#52](nim-lang/nimsuggest#52)
* `ttemplate_highlight.nim`: per the file name
* `twithin_macro_prefix.nim`: suggest within a macro with a prefix

Tests Not Re-Enabled:
* `twithin_macro.nim` still disabled as it doesn't provide a good test signal
* EPC highlight tests remain disabled -- requires out of scope tester changes

Additional Notes:
* todos added in comments for follow-up work
saem authored and mildred committed Jan 11, 2021
1 parent 9a3be54 commit 7fcc6d7
Showing 19 changed files with 292 additions and 376 deletions.
164 changes: 164 additions & 0 deletions nimsuggest/tests/fixtures/mclass_macro.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@

import macros

macro class*(head, body: untyped): untyped =
# The macro is immediate, since all its parameters are untyped.
# This means, it doesn't resolve identifiers passed to it.

var typeName, baseName: NimNode

# flag if object should be exported
var exported: bool

if head.kind == nnkInfix and head[0].kind == nnkIdent and $head[0] == "of":
# `head` is expression `typeName of baseClass`
# echo head.treeRepr
# --------------------
# Infix
# Ident !"of"
# Ident !"Animal"
# Ident !"RootObj"
typeName = head[1]
baseName = head[2]

elif head.kind == nnkInfix and head[0].kind == nnkIdent and
$head[0] == "*" and head[2].kind == nnkPrefix and
head[2][0].kind == nnkIdent and $head[2][0] == "of":
# `head` is expression `typeName* of baseClass`
# echo head.treeRepr
# --------------------
# Infix
# Ident !"*"
# Ident !"Animal"
# Prefix
# Ident !"of"
# Ident !"RootObj"
typeName = head[1]
baseName = head[2][1]
exported = true

else:
quit "Invalid node: " & head.lispRepr

# The following prints out the AST structure:
#
# import macros
# dumptree:
# type X = ref object of Y
# z: int
# --------------------
# StmtList
# TypeSection
# TypeDef
# Ident !"X"
# Empty
# RefTy
# ObjectTy
# Empty
# OfInherit
# Ident !"Y"
# RecList
# IdentDefs
# Ident !"z"
# Ident !"int"
# Empty

# create a type section in the result
result = newNimNode(nnkStmtList)
result.add(
if exported:
# mark `typeName` with an asterisk
quote do:
type `typeName`* = ref object of `baseName`
else:
quote do:
type `typeName` = ref object of `baseName`
)

# echo treeRepr(body)
# --------------------
# StmtList
# VarSection
# IdentDefs
# Ident !"name"
# Ident !"string"
# Empty
# IdentDefs
# Ident !"age"
# Ident !"int"
# Empty
# MethodDef
# Ident !"vocalize"
# Empty
# Empty
# FormalParams
# Ident !"string"
# Empty
# Empty
# StmtList
# StrLit ...
# MethodDef
# Ident !"age_human_yrs"
# Empty
# Empty
# FormalParams
# Ident !"int"
# Empty
# Empty
# StmtList
# DotExpr
# Ident !"this"
# Ident !"age"

# var declarations will be turned into object fields
var recList = newNimNode(nnkRecList)

# expected name of constructor
let ctorName = newIdentNode("new" & $typeName)

# Iterate over the statements, adding `this: T`
# to the parameters of functions, unless the
# function is a constructor
for node in body.children:
case node.kind:

of nnkMethodDef, nnkProcDef:
# check if it is the ctor proc
if node.name.kind != nnkAccQuoted and node.name.basename == ctorName:
# specify the return type of the ctor proc
node.params[0] = typeName
else:
# inject `self: T` into the arguments
node.params.insert(1, newIdentDefs(ident("self"), typeName))
result.add(node)

of nnkVarSection:
# variables get turned into fields of the type.
for n in node.children:
recList.add(n)

else:
result.add(node)

# Inspect the tree structure:
#
# echo result.treeRepr
# --------------------
# StmtList
# TypeSection
# TypeDef
# Ident !"Animal"
# Empty
# RefTy
# ObjectTy
# Empty
# OfInherit
# Ident !"RootObj"
# Empty <= We want to replace this
# MethodDef
# ...

result[0][0][2][0][2] = recList

# Lets inspect the human-readable version of the output
#echo repr(result)
File renamed without changes.
File renamed without changes.
5 changes: 5 additions & 0 deletions nimsuggest/tests/fixtures/mfakeassert.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Template for testing defs

template fakeAssert*(cond: untyped, msg: string = "") =
## template to allow def lookup testing
if not cond: quit(1)
15 changes: 15 additions & 0 deletions nimsuggest/tests/fixtures/minclude_import.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Creates an awkward set of dependencies between this, import, and include.
# This pattern appears in the compiler, compiler/(sem|ast|semexprs).nim.

import mfakeassert
import minclude_types

proc say*(g: Greet): string =
fakeAssert(true, "always works")
g.greeting & ", " & g.subject & "!"

include minclude_include

proc say*(): string =
fakeAssert(1 + 1 == 2, "math works")
say(create())
4 changes: 4 additions & 0 deletions nimsuggest/tests/fixtures/minclude_include.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# this file is included and relies on imports within the include

proc create*(greeting: string = "Hello", subject: string = "World"): Greet =
Greet(greeting: greeting, subject: subject)
6 changes: 6 additions & 0 deletions nimsuggest/tests/fixtures/minclude_types.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# types used by minclude_* (import or include), to find with def in include

type
Greet* = object
greeting*: string
subject*: string
19 changes: 19 additions & 0 deletions nimsuggest/tests/fixtures/mstrutils.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import mfakeassert

func rereplace*(s, sub: string; by: string = ""): string {.used.} =
## competes for priority in suggestion, here first, but never used in test

fakeAssert(true, "always works")
result = by

func replace*(s, sub: string; by: string = ""): string =
## this is a test version of strutils.replace, it simply returns `by`

fakeAssert("".len == 0, "empty string is empty")
result = by

func rerereplace*(s, sub: string; by: string = ""): string {.used.} =
## isn't used and appears last, lowest priority

fakeAssert(false, "never works")
result = by
9 changes: 4 additions & 5 deletions nimsuggest/tests/tchk1.nim
Original file line number Diff line number Diff line change
@@ -15,14 +15,13 @@ proc main =

#[!]#
discard """
disabled:true
$nimsuggest --tester $file
>chk $1
chk;;skUnknown;;;;Hint;;???;;-1;;-1;;"tchk1 [Processing]";;0
chk;;skUnknown;;;;Error;;$file;;12;;0;;"identifier expected, but found \'keyword template\'";;0
chk;;skUnknown;;;;Error;;$file;;14;;0;;"complex statement requires indentation";;0
chk;;skUnknown;;;;Hint;;???;;0;;-1;;"tchk1 [Processing]";;0
chk;;skUnknown;;;;Error;;$file;;12;;0;;"identifier expected, but got \'keyword template\'";;0
chk;;skUnknown;;;;Error;;$file;;14;;0;;"nestable statement requires indentation";;0
chk;;skUnknown;;;;Error;;$file;;12;;0;;"implementation of \'foo\' expected";;0
chk;;skUnknown;;;;Error;;$file;;17;;0;;"invalid indentation";;0
chk;;skUnknown;;;;Hint;;$file;;12;;9;;"\'foo\' is declared but not used [XDeclaredButNotUsed]";;0
chk;;skUnknown;;;;Hint;;$file;;14;;5;;"\'tchk1.main()[declared in tchk1.nim(14, 5)]\' is declared but not used [XDeclaredButNotUsed]";;0
chk;;skUnknown;;;;Hint;;$file;;14;;5;;"\'main\' is declared but not used [XDeclaredButNotUsed]";;0
"""
13 changes: 13 additions & 0 deletions nimsuggest/tests/tdef2.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Test def with template and boundaries for the cursor

import fixtures/mstrutils

discard """
$nimsuggest --tester $file
>def $path/fixtures/mstrutils.nim:6:4
def;;skTemplate;;mfakeassert.fakeAssert;;template (cond: untyped, msg: string);;*fixtures/mfakeassert.nim;;3;;9;;"template to allow def lookup testing";;100
>def $path/fixtures/mstrutils.nim:12:3
def;;skTemplate;;mfakeassert.fakeAssert;;template (cond: untyped, msg: string);;*fixtures/mfakeassert.nim;;3;;9;;"template to allow def lookup testing";;100
>def $path/fixtures/mstrutils.nim:18:11
def;;skTemplate;;mfakeassert.fakeAssert;;template (cond: untyped, msg: string);;*fixtures/mfakeassert.nim;;3;;9;;"template to allow def lookup testing";;100
"""
4 changes: 2 additions & 2 deletions nimsuggest/tests/tdot3.nim
Original file line number Diff line number Diff line change
@@ -9,14 +9,14 @@ proc main(f: Foo) =
# this way, the line numbers more often stay the same

discard """
!copy dep_v1.nim dep.nim
!copy fixtures/mdep_v1.nim dep.nim
$nimsuggest --tester $file
>sug $1
sug;;skField;;x;;int;;*dep.nim;;8;;4;;"";;100;;None
sug;;skField;;y;;int;;*dep.nim;;8;;8;;"";;100;;None
sug;;skProc;;tdot3.main;;proc (f: Foo);;$file;;5;;5;;"";;100;;None
!copy dep_v2.nim dep.nim
!copy fixtures/mdep_v2.nim dep.nim
>mod $path/dep.nim
>sug $1
sug;;skField;;x;;int;;*dep.nim;;8;;4;;"";;100;;None
22 changes: 13 additions & 9 deletions nimsuggest/tests/tdot4.nim
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
discard """
disabled:true
$nimsuggest --tester --maxresults:2 $file
>sug $1
sug;;skProc;;tdot4.main;;proc (inp: string): string;;$file;;10;;5;;"";;100;;None
sug;;skProc;;strutils.replace;;proc (s: string, sub: string, by: string): string{.noSideEffect, gcsafe, locks: 0.};;$lib/pure/strutils.nim;;1506;;5;;"Replaces `sub` in `s` by the string `by`.";;100;;None
"""
# Test that already used suggestions are prioritized

import strutils
from system import string, echo
import fixtures/mstrutils

proc main(inp: string): string =
# use replace here and see if it occurs in the result, it should gain
# priority:
result = inp.replace(" ", "a").replace("b", "c")


echo "string literal here".#[!]#

# priority still tested, but limit results to avoid failures from other output
discard """
$nimsuggest --tester --maxresults:2 $file
>sug $1
sug;;skProc;;tdot4.main;;proc (inp: string): string;;$file;;6;;5;;"";;100;;None
sug;;skFunc;;mstrutils.replace;;proc (s: string, sub: string, by: string): string{.noSideEffect, gcsafe, locks: 0.};;*fixtures/mstrutils.nim;;9;;5;;"this is a test version of strutils.replace, it simply returns `by`";;100;;None
"""

# TODO - determine appropriate behaviour for further suggest output and test it
23 changes: 17 additions & 6 deletions nimsuggest/tests/tinclude.nim
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
# import that has an include, def calls must work into and out of includes
import fixtures/minclude_import

proc go() =
discard create().say()

go()

discard """
disabled:true
$nimsuggest --tester compiler/nim.nim
>def compiler/semexprs.nim:25:50
def;;skType;;ast.PSym;;PSym;;*ast.nim;;707;;2;;"";;100
>def compiler/semexprs.nim:25:50
def;;skType;;ast.PSym;;PSym;;*ast.nim;;707;;2;;"";;100
$nimsuggest --tester $file
>def $path/tinclude.nim:5:14
def;;skProc;;minclude_import.create;;proc (greeting: string, subject: string): Greet{.noSideEffect, gcsafe, locks: 0.};;*fixtures/minclude_include.nim;;3;;5;;"";;100
>def $path/fixtures/minclude_include.nim:3:71
def;;skType;;minclude_types.Greet;;Greet;;*fixtures/minclude_types.nim;;4;;2;;"";;100
>def $path/fixtures/minclude_include.nim:3:71
def;;skType;;minclude_types.Greet;;Greet;;*fixtures/minclude_types.nim;;4;;2;;"";;100
"""

# TODO test/fix if the first `def` is not first or repeated we get no results
10 changes: 0 additions & 10 deletions nimsuggest/tests/tstrutils.nim

This file was deleted.

13 changes: 7 additions & 6 deletions nimsuggest/tests/tsug_regression.nim
Original file line number Diff line number Diff line change
@@ -17,13 +17,14 @@ proc main =
map0.#[!]#

discard """
disabled:true
$nimsuggest --tester $file
>sug $1
sug;;skProc;;tables.getOrDefault;;proc (t: Table[getOrDefault.A, getOrDefault.B], key: A): B;;$lib/pure/collections/tables.nim;;178;;5;;"";;100;;None
sug;;skProc;;tables.hasKey;;proc (t: Table[hasKey.A, hasKey.B], key: A): bool;;$lib/pure/collections/tables.nim;;233;;5;;"returns true iff `key` is in the table `t`.";;100;;None
sug;;skProc;;tables.add;;proc (t: var Table[add.A, add.B], key: A, val: B);;$lib/pure/collections/tables.nim;;309;;5;;"puts a new (key, value)-pair into `t` even if ``t[key]`` already exists.";;100;;None
sug;;skIterator;;tables.allValues;;iterator (t: Table[allValues.A, allValues.B], key: A): B{.inline.};;$lib/pure/collections/tables.nim;;225;;9;;"iterates over any value in the table `t` that belongs to the given `key`.";;100;;None
sug;;skProc;;tables.clear;;proc (t: var Table[clear.A, clear.B]);;$lib/pure/collections/tables.nim;;121;;5;;"Resets the table so that it is empty.";;100;;None
sug;;skProc;;tables.hasKey;;proc (t: Table[hasKey.A, hasKey.B], key: A): bool;;*/lib/pure/collections/tables.nim;;374;;5;;"Returns true if*";;100;;None
sug;;skProc;;tables.add;;proc (t: var Table[add.A, add.B], key: A, val: sink B);;*/lib/pure/collections/tables.nim;;505;;5;;"Puts a new*";;100;;None
sug;;skIterator;;tables.allValues;;iterator (t: Table[allValues.A, allValues.B], key: A): B{.inline.};;*/lib/pure/collections/tables.nim;;769;;9;;"Iterates over any*";;100;;None
sug;;skProc;;tables.clear;;proc (t: var Table[clear.A, clear.B]);;*/lib/pure/collections/tables.nim;;567;;5;;"Resets the table so that it is empty.*";;100;;None
sug;;skProc;;tables.contains;;proc (t: Table[contains.A, contains.B], key: A): bool;;*/lib/pure/collections/tables.nim;;392;;5;;"Alias of `hasKey*";;100;;None
*
"""

# TODO: test/fix suggestion sorting - deprecated suggestions should rank lower
File renamed without changes.
7 changes: 3 additions & 4 deletions nimsuggest/tests/ttype_decl.nim
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
discard """
disabled:true
$nimsuggest --tester --maxresults:3 $file
>sug $1
sug;;skType;;ttype_decl.Other;;Other;;$file;;11;;2;;"";;0;;None
sug;;skType;;system.int;;int;;$lib/system/basic_types.nim;;2;;2;;"";;0;;None
sug;;skType;;system.string;;string;;$lib/system.nim;;34;;2;;"";;0;;None
sug;;skType;;ttype_decl.Other;;Other;;$file;;10;;2;;"";;0;;None
sug;;skType;;system.int;;int;;*/lib/system/basic_types.nim;;2;;2;;"";;0;;None
sug;;skType;;system.string;;string;;*/lib/system.nim;;34;;2;;"";;0;;None
"""
import strutils
type
Loading

0 comments on commit 7fcc6d7

Please sign in to comment.