From 365a753eed70f817b43fd8c76bdfaf28ab001561 Mon Sep 17 00:00:00 2001 From: quantimnot <54247259+quantimnot@users.noreply.github.com> Date: Sat, 6 May 2023 13:10:13 -0400 Subject: [PATCH] Fix some `styleCheck` bugs (#20095) refs #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 --- compiler/linter.nim | 14 +++--- compiler/pragmas.nim | 3 +- .../foreign_package/foreign_package.nim | 1 + .../foreign_package/foreign_package.nimble | 2 + tests/stylecheck/tforeign_package.nim | 16 +++++++ tests/stylecheck/thint.nim | 43 +++++++++++++++++++ 6 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 tests/stylecheck/foreign_package/foreign_package.nim create mode 100644 tests/stylecheck/foreign_package/foreign_package.nimble create mode 100644 tests/stylecheck/tforeign_package.nim create mode 100644 tests/stylecheck/thint.nim diff --git a/compiler/linter.nim b/compiler/linter.nim index 0c2aaef792..f3e2d62071 100644 --- a/compiler/linter.nim +++ b/compiler/linter.nim @@ -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 @@ -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 @@ -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) diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 9f2eeb002f..10d77a17e9 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -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) @@ -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) diff --git a/tests/stylecheck/foreign_package/foreign_package.nim b/tests/stylecheck/foreign_package/foreign_package.nim new file mode 100644 index 0000000000..f95be006c6 --- /dev/null +++ b/tests/stylecheck/foreign_package/foreign_package.nim @@ -0,0 +1 @@ +include ../thint \ No newline at end of file diff --git a/tests/stylecheck/foreign_package/foreign_package.nimble b/tests/stylecheck/foreign_package/foreign_package.nimble new file mode 100644 index 0000000000..a2c49e3898 --- /dev/null +++ b/tests/stylecheck/foreign_package/foreign_package.nimble @@ -0,0 +1,2 @@ +# See `tstyleCheck` +# Needed to mark `mstyleCheck` as a foreign package. diff --git a/tests/stylecheck/tforeign_package.nim b/tests/stylecheck/tforeign_package.nim new file mode 100644 index 0000000000..8594ad802e --- /dev/null +++ b/tests/stylecheck/tforeign_package.nim @@ -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]() diff --git a/tests/stylecheck/thint.nim b/tests/stylecheck/thint.nim new file mode 100644 index 0000000000..c19aac1b89 --- /dev/null +++ b/tests/stylecheck/thint.nim @@ -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]()