A2-10-1: add functions and types to identifier consideration

Author: knewbury01

change_notes/2024-02-27-identifier-hidden.md

@@ -0,0 +1,2 @@
+- `A2-10-1`, `RULE-5-3` - `IdentifierHiding.ql`, `IdentifierHidingC.ql`:
+  - Address FN reported in #118. Rule was missing detection of functions and types.

cpp/common/src/codingstandards/cpp/Scope.qll

@@ -57,9 +57,15 @@ private Element getParentScope(Element e) {
 
 /** A variable which is defined by the user, rather than being from a third party or compiler generated. */
 class UserVariable extends Variable {
-  UserVariable() {
+  UserVariable() { this instanceof UserDeclaration }
+}
+
+/** A construct which is defined by the user, rather than being from a third party or compiler generated. */
+class UserDeclaration extends Declaration {
+  UserDeclaration() {
     exists(getFile().getRelativePath()) and
-    not isCompilerGenerated() and
+    not this.(Variable).isCompilerGenerated() and
+    not this.(Function).isCompilerGenerated() and
     not this.(Parameter).getFunction().isCompilerGenerated() and
     // compiler inferred parameters have name of p#0
     not this.(Parameter).getName() = "p#0"
@@ -78,7 +84,7 @@ class Scope extends Element {
 
   Scope getStrictParent() { result = getParentScope(this) }
 
-  Declaration getADeclaration() { getParentScope(result) = this }
+  UserDeclaration getADeclaration() { getParentScope(result) = this }
 
   Expr getAnExpr() { this = getParentScope(result) }
 
@@ -122,30 +128,30 @@ class GeneratedBlockStmt extends BlockStmt {
   GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation }
 }
 
-/** Gets a variable that is in the potential scope of variable `v`. */
-private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) {
+/** Gets a Declaration that is in the potential scope of Declaration `v`. */
+private UserDeclaration getPotentialScopeOfDeclaration_candidate(UserDeclaration v) {
   exists(Scope s |
-    result = s.getAVariable() and
+    result = s.getADeclaration() and
     (
-      // Variable in an ancestor scope, but only if there are less than 100 variables in this scope
-      v = s.getAnAncestor().getAVariable() and
+      // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope
+      v = s.getAnAncestor().getADeclaration() and
       s.getNumberOfVariables() < 100
       or
-      // In the same scope, but not the same variable, and choose just one to report
-      v = s.getAVariable() and
+      // In the same scope, but not the same Declaration, and choose just one to report
+      v = s.getADeclaration() and
       not result = v and
       v.getName() <= result.getName()
     )
   )
 }
 
-/** Gets a variable that is in the potential scope of variable `v`. */
-private UserVariable getOuterScopesOfVariable_candidate(UserVariable v) {
+/** Gets a Declarationthat is in the potential scope of Declaration `v`. */
+private UserDeclaration getOuterScopesOfDeclaration_candidate(UserDeclaration v) {
   exists(Scope s |
-    result = s.getAVariable() and
+    result = s.getADeclaration() and
     (
-      // Variable in an ancestor scope, but only if there are less than 100 variables in this scope
-      v = s.getAnAncestor().getAVariable() and
+      // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope
+      v = s.getAnAncestor().getADeclaration() and
       s.getNumberOfVariables() < 100
     )
   )
@@ -161,20 +167,20 @@ predicate inSameTranslationUnit(File f1, File f2) {
 }
 
 /**
- * Gets a user variable which occurs in the "potential scope" of variable `v`.
+ * Gets a user Declaration which occurs in the "outer scope" of Declaration `v`.
  */
 cached
-UserVariable getPotentialScopeOfVariable(UserVariable v) {
-  result = getPotentialScopeOfVariable_candidate(v) and
+UserDeclaration getPotentialScopeOfDeclarationStrict(UserDeclaration v) {
+  result = getOuterScopesOfDeclaration_candidate(v) and
   inSameTranslationUnit(v.getFile(), result.getFile())
 }
 
 /**
- * Gets a user variable which occurs in the "outer scope" of variable `v`.
+ * Gets a user variable which occurs in the "potential scope" of variable `v`.
  */
 cached
-UserVariable getPotentialScopeOfVariableStrict(UserVariable v) {
-  result = getOuterScopesOfVariable_candidate(v) and
+UserDeclaration getPotentialScopeOfDeclaration(UserDeclaration v) {
+  result = getPotentialScopeOfDeclaration_candidate(v) and
   inSameTranslationUnit(v.getFile(), result.getFile())
 }
 
@@ -204,18 +210,9 @@ class TranslationUnit extends SourceFile {
 }
 
 /** Holds if `v2` may hide `v1`. */
-private predicate hides_candidate(UserVariable v1, UserVariable v2) {
-  not v1 = v2 and
-  v2 = getPotentialScopeOfVariable(v1) and
-  v1.getName() = v2.getName() and
-  // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
-  not (v1.isMember() or v2.isMember())
-}
-
-/** Holds if `v2` may hide `v1`. */
-private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
+private predicate hides_candidateStrict(UserDeclaration v1, UserDeclaration v2) {
   not v1 = v2 and
-  v2 = getPotentialScopeOfVariableStrict(v1) and
+  v2 = getPotentialScopeOfDeclarationStrict(v1) and
   v1.getName() = v2.getName() and
   // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
   not (v1.isMember() or v2.isMember()) and
@@ -239,6 +236,15 @@ private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
   )
 }
 
+/** Holds if `v2` may hide `v1`. */
+private predicate hides_candidate(UserDeclaration v1, UserDeclaration v2) {
+  not v1 = v2 and
+  v2 = getPotentialScopeOfDeclaration(v1) and
+  v1.getName() = v2.getName() and
+  // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
+  not (v1.isMember() or v2.isMember())
+}
+
 /**
  * Gets the enclosing statement of the given variable, if any.
  */
@@ -257,20 +263,20 @@ private Stmt getEnclosingStmt(LocalScopeVariable v) {
 }
 
 /** Holds if `v2` hides `v1`. */
-predicate hides(UserVariable v1, UserVariable v2) {
+predicate hides(UserDeclaration v1, UserDeclaration v2) {
   hides_candidate(v1, v2) and
   // Confirm that there's no closer candidate variable which `v2` hides
-  not exists(UserVariable mid |
+  not exists(UserDeclaration mid |
     hides_candidate(v1, mid) and
     hides_candidate(mid, v2)
   )
 }
 
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
-predicate hidesStrict(UserVariable v1, UserVariable v2) {
+predicate hidesStrict(UserDeclaration v1, UserDeclaration v2) {
   hides_candidateStrict(v1, v2) and
   // Confirm that there's no closer candidate variable which `v2` hides
-  not exists(UserVariable mid |
+  not exists(UserDeclaration mid |
     hides_candidateStrict(v1, mid) and
     hides_candidateStrict(mid, v2)
   )

cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll

@@ -11,13 +11,13 @@ abstract class IdentifierHiddenSharedQuery extends Query { }
 
 Query getQuery() { result instanceof IdentifierHiddenSharedQuery }
 
-query predicate problems(UserVariable v2, string message, UserVariable v1, string varName) {
+query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, string varName) {
   not isExcluded(v1, getQuery()) and
   not isExcluded(v2, getQuery()) and
   //ignore template variables for this rule
   not v1 instanceof TemplateVariable and
   not v2 instanceof TemplateVariable and
   hidesStrict(v1, v2) and
   varName = v1.getName() and
-  message = "Variable is hiding variable $@."
+  message = "Declaration is hiding declaration $@."
 }

cpp/common/test/rules/identifierhidden/IdentifierHidden.expected

@@ -1,11 +1,13 @@
-| test.cpp:4:5:4:7 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
-| test.cpp:8:5:8:7 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
-| test.cpp:11:5:11:7 | id1 | Variable is hiding variable $@. | test.cpp:8:5:8:7 | id1 | id1 |
-| test.cpp:20:7:20:9 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
-| test.cpp:23:13:23:15 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
-| test.cpp:26:12:26:14 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
-| test.cpp:27:14:27:16 | id1 | Variable is hiding variable $@. | test.cpp:26:12:26:14 | id1 | id1 |
-| test.cpp:65:11:65:11 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
-| test.cpp:67:9:67:9 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
-| test.cpp:70:12:70:12 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
-| test.cpp:75:16:75:16 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
+| test.cpp:4:5:4:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 |
+| test.cpp:8:5:8:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 |
+| test.cpp:11:5:11:7 | id1 | Declaration is hiding declaration $@. | test.cpp:8:5:8:7 | id1 | id1 |
+| test.cpp:20:7:20:9 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 |
+| test.cpp:23:13:23:15 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 |
+| test.cpp:26:12:26:14 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 |
+| test.cpp:27:14:27:16 | id1 | Declaration is hiding declaration $@. | test.cpp:26:12:26:14 | id1 | id1 |
+| test.cpp:65:11:65:11 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i |
+| test.cpp:67:9:67:9 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i |
+| test.cpp:70:12:70:12 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i |
+| test.cpp:75:16:75:16 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i |
+| test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a |
+| test.cpp:85:13:85:13 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a |

cpp/common/test/rules/identifierhidden/test.cpp

@@ -74,4 +74,13 @@ void test_scope_order() {
 
   } catch (int i) { // NON_COMPLIANT
   }
+}
+
+int a;
+namespace b {
+int a() {} // NON_COMPLIANT
+} // namespace b
+
+namespace b1 {
+typedef int a; // NON_COMPLIANT
 }