Skip to content

Commit

Permalink
Fix some styleCheck bugs (nim-lang#20095)
Browse files Browse the repository at this point in the history
refs nim-lang#19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
  • Loading branch information
2 people authored and bung87 committed Jul 29, 2023
1 parent 8146286 commit a0b78ae
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 6 deletions.
14 changes: 9 additions & 5 deletions compiler/linter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ template styleCheckDef*(ctx: PContext; info: TLineInfo; sym: PSym; k: TSymKind)
if optStyleCheck in ctx.config.options and # ignore if styleChecks are off
{optStyleHint, optStyleError} * ctx.config.globalOptions != {} and # check only if hint/error is enabled
hintName in ctx.config.notes and # ignore if name checks are not requested
ctx.config.belongsToProjectPackage(ctx.module) and # ignore foreign packages
ctx.config.belongsToProjectPackage(sym) and # ignore foreign packages
optStyleUsages notin ctx.config.globalOptions and # ignore if requested to only check name usage
sym.kind != skResult and # ignore `result`
sym.kind != skTemp and # ignore temporary variables created by the compiler
Expand Down Expand Up @@ -136,7 +136,7 @@ template styleCheckUse*(ctx: PContext; info: TLineInfo; sym: PSym) =
## Check symbol uses match their definition's style.
if {optStyleHint, optStyleError} * ctx.config.globalOptions != {} and # ignore if styleChecks are off
hintName in ctx.config.notes and # ignore if name checks are not requested
ctx.config.belongsToProjectPackage(ctx.module) and # ignore foreign packages
ctx.config.belongsToProjectPackage(sym) and # ignore foreign packages
sym.kind != skTemp and # ignore temporary variables created by the compiler
sym.name.s[0] in Letters and # ignore operators TODO: what about unicode symbols???
sfAnon notin sym.flags: # ignore temporary variables created by the compiler
Expand All @@ -147,6 +147,10 @@ proc checkPragmaUseImpl(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragm
if pragmaName != wanted:
lintReport(conf, info, wanted, pragmaName)

template checkPragmaUse*(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragmaName: string) =
if {optStyleHint, optStyleError} * conf.globalOptions != {}:
checkPragmaUseImpl(conf, info, w, pragmaName)
template checkPragmaUse*(ctx: PContext; info: TLineInfo; w: TSpecialWord; pragmaName: string, sym: PSym) =
## Check builtin pragma uses match their definition's style.
## Note: This only applies to builtin pragmas, not user pragmas.
if {optStyleHint, optStyleError} * ctx.config.globalOptions != {} and # ignore if styleChecks are off
hintName in ctx.config.notes and # ignore if name checks are not requested
(sym != nil and ctx.config.belongsToProjectPackage(sym)): # ignore foreign packages
checkPragmaUseImpl(ctx.config, info, w, pragmaName)
3 changes: 2 additions & 1 deletion compiler/pragmas.nim
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ proc processPragma(c: PContext, n: PNode, i: int) =
invalidPragma(c, n)

var userPragma = newSym(skTemplate, it[1].ident, c.idgen, c.module, it.info, c.config.options)
styleCheckDef(c, userPragma)
userPragma.ast = newTreeI(nkPragma, n.info, n.sons[i+1..^1])
strTableAdd(c.userPragmas, userPragma)

Expand Down Expand Up @@ -863,7 +864,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
else:
let k = whichKeyword(ident)
if k in validPragmas:
checkPragmaUse(c.config, key.info, k, ident.s)
checkPragmaUse(c, key.info, k, ident.s, (if sym != nil: sym else: c.module))
case k
of wExportc, wExportCpp:
makeExternExport(c, sym, getOptionalStr(c, it, "$1"), it.info)
Expand Down
1 change: 1 addition & 0 deletions tests/stylecheck/foreign_package/foreign_package.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../thint
2 changes: 2 additions & 0 deletions tests/stylecheck/foreign_package/foreign_package.nimble
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# See `tstyleCheck`
# Needed to mark `mstyleCheck` as a foreign package.
16 changes: 16 additions & 0 deletions tests/stylecheck/tforeign_package.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
discard """
matrix: "--errorMax:0 --styleCheck:error"
action: compile
"""

import foreign_package/foreign_package

# This call tests that:
# - an instantiation of a generic in a foreign package doesn't raise errors
# when the generic body contains:
# - definition and usage violations
# - builtin pragma usage violations
# - user pragma usage violations
# - definition violations in foreign packages are ignored
# - usage violations in foreign packages are ignored
genericProc[int]()
43 changes: 43 additions & 0 deletions tests/stylecheck/thint.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
discard """
matrix: "--styleCheck:hint"
action: compile
"""

# Test violating ident definition:
{.pragma: user_pragma.} #[tt.Hint
^ 'user_pragma' should be: 'userPragma' [Name] ]#

# Test violating ident usage style matches definition style:
{.userPragma.} #[tt.Hint
^ 'userPragma' should be: 'user_pragma' [template declared in thint.nim(7, 9)] [Name] ]#

# Test violating builtin pragma usage style:
{.no_side_effect.}: #[tt.Hint
^ 'no_side_effect' should be: 'noSideEffect' [Name] ]#
discard 0

# Test:
# - definition style violation
# - user pragma usage style violation
# - builtin pragma usage style violation
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] ]#
# Test definition style violation:
let snake_case = 0 #[tt.Hint
^ 'snake_case' should be: 'snakeCase' [Name] ]#
# Test user pragma definition style violation:
{.pragma: another_user_pragma.} #[tt.Hint
^ 'another_user_pragma' should be: 'anotherUserPragma' [Name] ]#
# Test user pragma usage style violation:
{.anotherUserPragma.} #[tt.Hint
^ 'anotherUserPragma' should be: 'another_user_pragma' [template declared in thint.nim(31, 11)] [Name] ]#
# Test violating builtin pragma usage style:
{.no_side_effect.}: #[tt.Hint
^ 'no_side_effect' should be: 'noSideEffect' [Name] ]#
# Test usage style violation:
discard snakeCase #[tt.Hint
^ 'snakeCase' should be: 'snake_case' [let declared in thint.nim(28, 7)] [Name] ]#

generic_proc[int]()

0 comments on commit a0b78ae

Please sign in to comment.