Fix FP for issue 214

exclude variable templates and
fix case where same scope identifiers
are considered

Author: knewbury01

change_notes/2023-03-14-fp-a12-10-1.md

@@ -0,0 +1 @@
+* `A12-10-1` and `RULE-5-3` - reduce false positives reported for identifiers in same scope, and (relevant for `A12-10-1` only) omitted false positives for template variables

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

@@ -130,6 +130,18 @@ private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) {
   )
 }
 
+/** Gets a variable that is in the potential scope of variable `v`. */
+private UserVariable getOuterScopesOfVariable_candidate(UserVariable v) {
+  exists(Scope s |
+    result = s.getAVariable() and
+    (
+      // Variable in an ancestor scope, but only if there are less than 100 variables in this scope
+      v = s.getAnAncestor().getAVariable() and
+      s.getNumberOfVariables() < 100
+    )
+  )
+}
+
 /** Holds if there exists a translation unit that includes both `f1` and `f2`. */
 pragma[noinline]
 predicate inSameTranslationUnit(File f1, File f2) {
@@ -148,6 +160,15 @@ UserVariable getPotentialScopeOfVariable(UserVariable v) {
   inSameTranslationUnit(v.getFile(), result.getFile())
 }
 
+/**
+ * Gets a user variable which occurs in the "outer scope" of variable `v`.
+ */
+cached
+UserVariable getPotentialScopeOfVariableStrict(UserVariable v) {
+  result = getOuterScopesOfVariable_candidate(v) and
+  inSameTranslationUnit(v.getFile(), result.getFile())
+}
+
 /** A file that is a C/C++ source file */
 class SourceFile extends File {
   SourceFile() {
@@ -182,6 +203,15 @@ private predicate hides_candidate(UserVariable v1, UserVariable v2) {
   not (v1.isMember() or v2.isMember())
 }
 
+/** Holds if `v2` may hide `v1`. */
+private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
+  not v1 = v2 and
+  v2 = getPotentialScopeOfVariableStrict(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` hides `v1`. */
 predicate hides(UserVariable v1, UserVariable v2) {
   hides_candidate(v1, v2) and
@@ -192,6 +222,16 @@ predicate hides(UserVariable v1, UserVariable v2) {
   )
 }
 
+/** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
+predicate hidesStrict(UserVariable v1, UserVariable v2) {
+  hides_candidateStrict(v1, v2) and
+  // Confirm that there's no closer candidate variable which `v2` hides
+  not exists(UserVariable mid |
+    hides_candidateStrict(v1, mid) and
+    hides_candidateStrict(mid, v2)
+  )
+}
+
 /** Holds if `decl` has namespace scope. */
 predicate hasNamespaceScope(Declaration decl) {
   // getNamespace always returns a namespace (e.g. the global namespace).

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

@@ -14,7 +14,10 @@ Query getQuery() { result instanceof IdentifierHiddenSharedQuery }
 query predicate problems(UserVariable v2, string message, UserVariable v1, string varName) {
   not isExcluded(v1, getQuery()) and
   not isExcluded(v2, getQuery()) and
-  hides(v1, v2) 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 $@."
 }

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

@@ -27,4 +27,10 @@ void f3() {
     for (int id1; id1 < 1; id1++) {
     } // NON_COMPLIANT
   }
+}
+
+template <typename T> constexpr bool foo = false; // COMPLIANT
+
+namespace {
+template <typename T> bool foo<T> = true; // COMPLIANT
 }

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

@@ -0,0 +1,2 @@
+template <typename T> constexpr bool foo = false;   // COMPLIANT
+template <typename T> constexpr bool foo<T> = true; // COMPLIANT