Skip to content

Commit 57f0cf4

Browse files
committed
Fix some styleCheck bugs
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.
1 parent 62b81d7 commit 57f0cf4

File tree

6 files changed

+73
-6
lines changed

6 files changed

+73
-6
lines changed

compiler/linter.nim

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ template styleCheckDef*(ctx: PContext; info: TLineInfo; sym: PSym; k: TSymKind)
9494
## Check symbol definitions adhere to NEP1 style rules.
9595
if optStyleCheck in ctx.config.options and # ignore if styleChecks are off
9696
hintName in ctx.config.notes and # ignore if name checks are not requested
97-
ctx.config.belongsToProjectPackage(ctx.module) and # ignore foreign packages
97+
ctx.config.belongsToProjectPackage(sym) and # ignore foreign packages
9898
optStyleUsages notin ctx.config.globalOptions and # ignore if requested to only check name usage
9999
sym.kind != skResult and # ignore `result`
100100
sym.kind != skTemp and # ignore temporary variables created by the compiler
@@ -135,7 +135,7 @@ template styleCheckUse*(ctx: PContext; info: TLineInfo; sym: PSym) =
135135
## Check symbol uses match their definition's style.
136136
if {optStyleHint, optStyleError} * ctx.config.globalOptions != {} and # ignore if styleChecks are off
137137
hintName in ctx.config.notes and # ignore if name checks are not requested
138-
ctx.config.belongsToProjectPackage(ctx.module) and # ignore foreign packages
138+
ctx.config.belongsToProjectPackage(sym) and # ignore foreign packages
139139
sym.kind != skTemp and # ignore temporary variables created by the compiler
140140
sym.name.s[0] in Letters and # ignore operators TODO: what about unicode symbols???
141141
sfAnon notin sym.flags: # ignore temporary variables created by the compiler
@@ -146,6 +146,10 @@ proc checkPragmaUseImpl(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragm
146146
if pragmaName != wanted:
147147
lintReport(conf, info, wanted, pragmaName)
148148

149-
template checkPragmaUse*(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragmaName: string) =
150-
if {optStyleHint, optStyleError} * conf.globalOptions != {}:
151-
checkPragmaUseImpl(conf, info, w, pragmaName)
149+
template checkPragmaUse*(ctx: PContext; info: TLineInfo; w: TSpecialWord; pragmaName: string, sym: PSym) =
150+
## Check builtin pragma uses match their definition's style.
151+
## Note: This only applies to builtin pragmas, not user pragmas.
152+
if {optStyleHint, optStyleError} * ctx.config.globalOptions != {} and # ignore if styleChecks are off
153+
hintName in ctx.config.notes and # ignore if name checks are not requested
154+
(sym != nil and ctx.config.belongsToProjectPackage(sym)): # ignore foreign packages
155+
checkPragmaUseImpl(ctx.config, info, w, pragmaName)

compiler/pragmas.nim

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,7 @@ proc processPragma(c: PContext, n: PNode, i: int) =
648648
invalidPragma(c, n)
649649

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

@@ -843,7 +844,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
843844
else:
844845
let k = whichKeyword(ident)
845846
if k in validPragmas:
846-
checkPragmaUse(c.config, key.info, k, ident.s)
847+
checkPragmaUse(c, key.info, k, ident.s, (if sym != nil: sym else: c.module))
847848
case k
848849
of wExportc, wExportCpp:
849850
makeExternExport(c, sym, getOptionalStr(c, it, "$1"), it.info)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
include ../thint
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# See `tstyleCheck`
2+
# Needed to mark `mstyleCheck` as a foreign package.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
discard """
2+
matrix: "--errorMax:0 --styleCheck:error"
3+
action: compile
4+
"""
5+
6+
import foreign_package/foreign_package
7+
8+
# This call tests that:
9+
# - an instantiation of a generic in a foreign package doesn't raise errors
10+
# when the generic body contains:
11+
# - definition and usage violations
12+
# - builtin pragma usage violations
13+
# - user pragma usage violations
14+
# - definition violations in foreign packages are ignored
15+
# - usage violations in foreign packages are ignored
16+
genericProc[int]()

tests/stylecheck/thint.nim

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
discard """
2+
matrix: "--styleCheck:hint"
3+
action: compile
4+
"""
5+
6+
# Test violating ident definition:
7+
{.pragma: user_pragma.} #[tt.Hint
8+
^ 'user_pragma' should be: 'userPragma' [Name] ]#
9+
10+
# Test violating ident usage style matches definition style:
11+
{.userPragma.} #[tt.Hint
12+
^ 'userPragma' should be: 'user_pragma' [template declared in thint.nim(7, 9)] [Name] ]#
13+
14+
# Test violating builtin pragma usage style:
15+
{.no_side_effect.}: #[tt.Hint
16+
^ 'no_side_effect' should be: 'noSideEffect' [Name] ]#
17+
discard 0
18+
19+
# Test:
20+
# - definition style violation
21+
# - user pragma usage style violation
22+
# - builtin pragma usage style violation
23+
proc generic_proc*[T] {.no_destroy, userPragma.} = #[tt.Hint
24+
^ 'generic_proc' should be: 'genericProc' [Name]; tt.Hint
25+
^ 'no_destroy' should be: 'nodestroy' [Name]; tt.Hint
26+
^ 'userPragma' should be: 'user_pragma' [template declared in thint.nim(7, 9)] [Name] ]#
27+
# Test definition style violation:
28+
let snake_case = 0 #[tt.Hint
29+
^ 'snake_case' should be: 'snakeCase' [Name] ]#
30+
# Test user pragma definition style violation:
31+
{.pragma: another_user_pragma.} #[tt.Hint
32+
^ 'another_user_pragma' should be: 'anotherUserPragma' [Name] ]#
33+
# Test user pragma usage style violation:
34+
{.anotherUserPragma.} #[tt.Hint
35+
^ 'anotherUserPragma' should be: 'another_user_pragma' [template declared in thint.nim(31, 11)] [Name] ]#
36+
# Test violating builtin pragma usage style:
37+
{.no_side_effect.}: #[tt.Hint
38+
^ 'no_side_effect' should be: 'noSideEffect' [Name] ]#
39+
# Test usage style violation:
40+
discard snakeCase #[tt.Hint
41+
^ 'snakeCase' should be: 'snake_case' [let declared in thint.nim(28, 7)] [Name] ]#
42+
43+
generic_proc[int]()

0 commit comments

Comments
 (0)