Change styleCheck to ignore foreign packages

* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456

compiler/ic/bitabs.nim

@@ -93,13 +93,13 @@ proc getOrIncl*[T](t: var BiTable[T]; v: T): LitId =
   t.vals.add v
 
 
-proc `[]`*[T](t: var BiTable[T]; LitId: LitId): var T {.inline.} =
-  let idx = idToIdx LitId
+proc `[]`*[T](t: var BiTable[T]; litId: LitId): var T {.inline.} =
+  let idx = idToIdx litId
   assert idx < t.vals.len
   result = t.vals[idx]
 
-proc `[]`*[T](t: BiTable[T]; LitId: LitId): lent T {.inline.} =
-  let idx = idToIdx LitId
+proc `[]`*[T](t: BiTable[T]; litId: LitId): lent T {.inline.} =
+  let idx = idToIdx litId
   assert idx < t.vals.len
   result = t.vals[idx]
 

compiler/linter.nim

@@ -12,7 +12,8 @@
 import std/strutils
 from std/sugar import dup
 
-import options, ast, msgs, idents, lineinfos, wordrecg, astmsgs
+import options, ast, msgs, idents, lineinfos, wordrecg, astmsgs, semdata, packages
+export packages
 
 const
   Letters* = {'a'..'z', 'A'..'Z', '0'..'9', '\x80'..'\xFF', '_'}
@@ -85,24 +86,33 @@ proc differ*(line: string, a, b: int, x: string): string =
       result = y
 
 proc nep1CheckDefImpl(conf: ConfigRef; info: TLineInfo; s: PSym; k: TSymKind) =
-  # operators stay as they are:
-  if k in {skResult, skTemp} or s.name.s[0] notin Letters: return
-  if k in {skType, skGenericParam} and sfAnon in s.flags: return
-  if s.typ != nil and s.typ.kind == tyTypeDesc: return
-  if {sfImportc, sfExportc} * s.flags != {}: return
-  if optStyleCheck notin s.options: return
   let beau = beautifyName(s.name.s, k)
   if s.name.s != beau:
     lintReport(conf, info, beau, s.name.s)
 
-template styleCheckDef*(conf: ConfigRef; info: TLineInfo; s: PSym; k: TSymKind) =
-  if {optStyleHint, optStyleError} * conf.globalOptions != {} and optStyleUsages notin conf.globalOptions:
-    nep1CheckDefImpl(conf, info, s, k)
-
-template styleCheckDef*(conf: ConfigRef; info: TLineInfo; s: PSym) =
-  styleCheckDef(conf, info, s, s.kind)
-template styleCheckDef*(conf: ConfigRef; s: PSym) =
-  styleCheckDef(conf, s.info, s, s.kind)
+template styleCheckDef*(ctx: PContext; info: TLineInfo; sym: PSym; k: TSymKind) =
+  ## Check symbol definitions adhere to NEP1 style rules.
+  if optStyleCheck in ctx.config.options 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
+     optStyleUsages notin ctx.config.globalOptions and # ignore if requested to only check name usage
+     sym.kind != skField and # ignore object/tuple fields
+     sym.kind != skResult and # ignore `result`
+     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???
+     k notin {skType, skGenericParam} and # ignore types and generic params
+     (sym.typ == nil or sym.typ.kind != tyTypeDesc) and # ignore `typedesc`
+     {sfImportc, sfExportc} * sym.flags == {} and # ignore FFI
+     sfAnon notin sym.flags: # ignore if created by compiler
+    nep1CheckDefImpl(ctx.config, info, sym, k)
+
+template styleCheckDef*(ctx: PContext; info: TLineInfo; s: PSym) =
+  ## Check symbol definitions adhere to NEP1 style rules.
+  styleCheckDef(ctx, info, s, s.kind)
+
+template styleCheckDef*(ctx: PContext; s: PSym) =
+  ## Check symbol definitions adhere to NEP1 style rules.
+  styleCheckDef(ctx, s.info, s, s.kind)
 
 proc differs(conf: ConfigRef; info: TLineInfo; newName: string): string =
   let line = sourceLine(conf, info)
@@ -116,23 +126,32 @@ proc differs(conf: ConfigRef; info: TLineInfo; newName: string): string =
   let last = first+identLen(line, first)-1
   result = differ(line, first, last, newName)
 
-proc styleCheckUse*(conf: ConfigRef; info: TLineInfo; s: PSym) =
-  if info.fileIndex.int < 0: return
-  # we simply convert it to what it looks like in the definition
-  # for consistency
-
-  # operators stay as they are:
-  if s.kind == skTemp or s.name.s[0] notin Letters or sfAnon in s.flags:
-    return
-
+proc styleCheckUseImpl(conf: ConfigRef; info: TLineInfo; s: PSym) =
   let newName = s.name.s
   let badName = differs(conf, info, newName)
   if badName.len > 0:
     # special rules for historical reasons
-    let forceHint = badName == "nnkArgList" and newName == "nnkArglist" or badName == "nnkArglist" and newName == "nnkArgList"
-    lintReport(conf, info, newName, badName, forceHint = forceHint, extraMsg = "".dup(addDeclaredLoc(conf, s)))
-
-proc checkPragmaUse*(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragmaName: string) =
+    let forceHint = # TODO: PR Reviewer: Any clue if this is still needed? (This comment needs removed before commit.)
+      badName == "nnkArgList" and newName == "nnkArglist" or
+      badName == "nnkArglist" and newName == "nnkArgList"
+    lintReport(conf, info, newName, badName, forceHint, "".dup(addDeclaredLoc(conf, s)))
+
+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
+     sym.kind != skField and # ignore object/tuple fields
+     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
+    styleCheckUseImpl(ctx.config, info, sym)
+
+proc checkPragmaUseImpl(conf: ConfigRef; info: TLineInfo; w: TSpecialWord; pragmaName: string) =
   let wanted = $w
   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)

compiler/pragmas.nim

@@ -829,8 +829,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
   let ident = considerQuotedIdent(c, key)
   var userPragma = strTableGet(c.userPragmas, ident)
   if userPragma != nil:
-    if {optStyleHint, optStyleError} * c.config.globalOptions != {}:
-      styleCheckUse(c.config, key.info, userPragma)
+    styleCheckUse(c, key.info, userPragma)
 
     # number of pragmas increase/decrease with user pragma expansion
     inc c.instCounter
@@ -844,8 +843,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
   else:
     let k = whichKeyword(ident)
     if k in validPragmas:
-      if {optStyleHint, optStyleError} * c.config.globalOptions != {}:
-        checkPragmaUse(c.config, key.info, k, ident.s)
+      checkPragmaUse(c.config, key.info, k, ident.s)
       case k
       of wExportc, wExportCpp:
         makeExternExport(c, sym, getOptionalStr(c, it, "$1"), it.info)

compiler/semexprs.nim

@@ -2586,7 +2586,7 @@ proc semBlock(c: PContext, n: PNode; flags: TExprFlags): PNode =
       labl.owner = c.p.owner
     n[0] = newSymNode(labl, n[0].info)
     suggestSym(c.graph, n[0].info, labl, c.graph.usageSym)
-    styleCheckDef(c.config, labl)
+    styleCheckDef(c, labl)
     onDef(n[0].info, labl)
   n[1] = semExpr(c, n[1], flags)
   n.typ = n[1].typ

compiler/semgnrc.nim

@@ -179,7 +179,7 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags,
 proc addTempDecl(c: PContext; n: PNode; kind: TSymKind) =
   let s = newSymS(skUnknown, getIdentNode(c, n), c)
   addPrelimDecl(c, s)
-  styleCheckDef(c.config, n.info, s, kind)
+  styleCheckDef(c, n.info, s, kind)
   onDef(n.info, s)
 
 proc semGenericStmt(c: PContext, n: PNode,

compiler/semmagic.nim

@@ -428,7 +428,7 @@ proc semQuantifier(c: PContext; n: PNode): PNode =
       let op = considerQuotedIdent(c, it[0])
       if op.id == ord(wIn):
         let v = newSymS(skForVar, it[1], c)
-        styleCheckDef(c.config, v)
+        styleCheckDef(c, v)
         onDef(it[1].info, v)
         let domain = semExprWithType(c, it[2], {efWantIterator})
         v.typ = domain.typ

compiler/semstmts.nim

@@ -373,7 +373,7 @@ proc semUsing(c: PContext; n: PNode): PNode =
       let typ = semTypeNode(c, a[^2], nil)
       for j in 0..<a.len-2:
         let v = semIdentDef(c, a[j], skParam)
-        styleCheckDef(c.config, v)
+        styleCheckDef(c, v)
         onDef(a[j].info, v)
         v.typ = typ
         strTableIncl(c.signatures, v)
@@ -662,7 +662,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
         addToVarSection(c, result, n, a)
         continue
       var v = semIdentDef(c, a[j], symkind, false)
-      styleCheckDef(c.config, v)
+      styleCheckDef(c, v)
       onDef(a[j].info, v)
       if sfGenSym notin v.flags:
         if not isDiscardUnderscore(v): addInterfaceDecl(c, v)
@@ -792,7 +792,7 @@ proc semConst(c: PContext, n: PNode): PNode =
       var v = semIdentDef(c, a[j], skConst)
       if sfGenSym notin v.flags: addInterfaceDecl(c, v)
       elif v.owner == nil: v.owner = getCurrOwner(c)
-      styleCheckDef(c.config, v)
+      styleCheckDef(c, v)
       onDef(a[j].info, v)
 
       if a.kind != nkVarTuple:
@@ -817,7 +817,7 @@ include semfields
 proc symForVar(c: PContext, n: PNode): PSym =
   let m = if n.kind == nkPragmaExpr: n[0] else: n
   result = newSymG(skForVar, m, c)
-  styleCheckDef(c.config, result)
+  styleCheckDef(c, result)
   onDef(n.info, result)
   if n.kind == nkPragmaExpr:
     pragma(c, result, n[1], forVarPragmas)
@@ -1443,7 +1443,7 @@ proc typeSectionFinalPass(c: PContext, n: PNode) =
     let name = typeSectionTypeName(c, a[0])
     var s = name.sym
     # check the style here after the pragmas have been processed:
-    styleCheckDef(c.config, s)
+    styleCheckDef(c, s)
     # compute the type's size and check for illegal recursions:
     if a[1].kind == nkEmpty:
       var x = a[2]
@@ -2041,7 +2041,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
       ("'" & proto.name.s & "' from " & c.config$proto.info &
         " '" & s.name.s & "' from " & c.config$s.info))
 
-  styleCheckDef(c.config, s)
+  styleCheckDef(c, s)
   if hasProto:
     onDefResolveForward(n[namePos].info, proto)
   else:
@@ -2118,7 +2118,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
         trackProc(c, s, s.ast[bodyPos])
       else:
         if (s.typ[0] != nil and s.kind != skIterator):
-          addDecl(c, newSym(skUnknown, getIdent(c.cache, "result"), nextSymId c.idgen, nil, n.info))
+          addDecl(c, newSym(skUnknown, getIdent(c.cache, "result"), nextSymId c.idgen, s, n.info))
 
         openScope(c)
         n[bodyPos] = semGenericStmt(c, n[bodyPos])

compiler/semtempl.nim

@@ -230,7 +230,7 @@ proc addLocalDecl(c: var TemplCtx, n: var PNode, k: TSymKind) =
       if n.kind != nkSym:
         let local = newGenSym(k, ident, c)
         addPrelimDecl(c.c, local)
-        styleCheckDef(c.c.config, n.info, local)
+        styleCheckDef(c.c, n.info, local)
         onDef(n.info, local)
         replaceIdentBySym(c.c, n, newSymNode(local, n.info))
         if k == skParam and c.inTemplateHeader > 0:
@@ -271,8 +271,7 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
     when defined(nimsuggest):
       suggestSym(c.graph, n.info, s, c.graph.usageSym, false)
     # field access (dot expr) will be handled by builtinFieldAccess
-    if not isField and {optStyleHint, optStyleError} * c.config.globalOptions != {}:
-      styleCheckUse(c.config, n.info, s)
+    styleCheckUse(c, n.info, s)
 
 proc semRoutineInTemplName(c: var TemplCtx, n: PNode): PNode =
   result = n
@@ -297,7 +296,7 @@ proc semRoutineInTemplBody(c: var TemplCtx, n: PNode, k: TSymKind): PNode =
       var s = newGenSym(k, ident, c)
       s.ast = n
       addPrelimDecl(c.c, s)
-      styleCheckDef(c.c.config, n.info, s)
+      styleCheckDef(c.c, n.info, s)
       onDef(n.info, s)
       n[namePos] = newSymNode(s, n[namePos].info)
     else:
@@ -431,7 +430,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
         # labels are always 'gensym'ed:
         let s = newGenSym(skLabel, n[0], c)
         addPrelimDecl(c.c, s)
-        styleCheckDef(c.c.config, s)
+        styleCheckDef(c.c, s)
         onDef(n[0].info, s)
         n[0] = newSymNode(s, n[0].info)
     n[1] = semTemplBody(c, n[1])
@@ -624,7 +623,7 @@ proc semTemplateDef(c: PContext, n: PNode): PNode =
        s.owner.name.s == "vm" and s.name.s == "stackTrace":
       incl(s.flags, sfCallsite)
 
-  styleCheckDef(c.config, s)
+  styleCheckDef(c, s)
   onDef(n[namePos].info, s)
   # check parameter list:
   #s.scope = c.currentScope

compiler/semtypes.nim

@@ -139,7 +139,7 @@ proc semEnum(c: PContext, n: PNode, prev: PType): PType =
       e.flags.incl {sfUsed, sfExported}
 
     result.n.add symNode
-    styleCheckDef(c.config, e)
+    styleCheckDef(c, e)
     onDef(e.info, e)
     if sfGenSym notin e.flags:
       if not isPure:
@@ -476,7 +476,7 @@ proc semTuple(c: PContext, n: PNode, prev: PType): PType =
       else:
         result.n.add newSymNode(field)
         addSonSkipIntLit(result, typ, c.idgen)
-      styleCheckDef(c.config, a[j].info, field)
+      styleCheckDef(c, a[j].info, field)
       onDef(field.info, field)
   if result.n.len == 0: result.n = nil
   if isTupleRecursive(result):
@@ -808,7 +808,7 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
         localError(c.config, info, "attempt to redefine: '" & f.name.s & "'")
       if a.kind == nkEmpty: father.add newSymNode(f)
       else: a.add newSymNode(f)
-      styleCheckDef(c.config, f)
+      styleCheckDef(c, f)
       onDef(f.info, f)
     if a.kind != nkEmpty: father.add a
   of nkSym:
@@ -1315,7 +1315,7 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
       result.n.add newSymNode(arg)
       rawAddSon(result, finalType)
       addParamOrResult(c, arg, kind)
-      styleCheckDef(c.config, a[j].info, arg)
+      styleCheckDef(c, a[j].info, arg)
       onDef(a[j].info, arg)
       if {optNimV1Emulation, optNimV12Emulation} * c.config.globalOptions == {}:
         a[j] = newSymNode(arg)

compiler/suggest.nim

@@ -596,8 +596,7 @@ proc markUsed(c: PContext; info: TLineInfo; s: PSym) =
     if sfError in s.flags: userError(conf, info, s)
   when defined(nimsuggest):
     suggestSym(c.graph, info, s, c.graph.usageSym, false)
-  if {optStyleHint, optStyleError} * conf.globalOptions != {}:
-    styleCheckUse(conf, info, s)
+  styleCheckUse(c, info, s)
   markOwnerModuleAsUsed(c, s)
 
 proc safeSemExpr*(c: PContext, n: PNode): PNode =

compiler/vm.nim

@@ -119,13 +119,13 @@ template move(a, b: untyped) {.dirty.} = system.shallowCopy(a, b)
 
 proc derefPtrToReg(address: BiggestInt, typ: PType, r: var TFullReg, isAssign: bool): bool =
   # nim bug: `isAssign: static bool` doesn't work, giving odd compiler error
-  template fun(field, T, rkind) =
+  template fun(field, typ, rkind) =
     if isAssign:
-      cast[ptr T](address)[] = T(r.field)
+      cast[ptr typ](address)[] = typ(r.field)
     else:
       r.ensureKind(rkind)
-      let val = cast[ptr T](address)[]
-      when T is SomeInteger | char:
+      let val = cast[ptr typ](address)[]
+      when typ is SomeInteger | char:
         r.field = BiggestInt(val)
       else:
         r.field = val