Skip to content

Commit adc818b

Browse files
committed
IdentifierHiding: Consider point of decl for local vars
Variables are only in scope from the point of declaration onwards.
1 parent 9f408a0 commit adc818b

File tree

3 files changed

+73
-1
lines changed

3 files changed

+73
-1
lines changed

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,42 @@ private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
209209
v2 = getPotentialScopeOfVariableStrict(v1) and
210210
v1.getName() = v2.getName() and
211211
// Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
212-
not (v1.isMember() or v2.isMember())
212+
not (v1.isMember() or v2.isMember()) and
213+
(
214+
// If v1 is a local variable, ensure that v1 is declared before v2
215+
(
216+
v1 instanceof LocalVariable and
217+
// Ignore variables declared in conditional expressions, as they apply to
218+
// the nested scope
219+
not v1 = any(ConditionDeclExpr cde).getVariable() and
220+
// Ignore variables declared in loops
221+
not exists(Loop l | l.getADeclaration() = v1)
222+
)
223+
implies
224+
exists(BlockStmt bs, DeclStmt v1Stmt, Stmt v2Stmt |
225+
v1 = v1Stmt.getADeclaration() and
226+
getEnclosingStmt(v2).getParentStmt*() = v2Stmt
227+
|
228+
bs.getIndexOfStmt(v1Stmt) <= bs.getIndexOfStmt(v2Stmt)
229+
)
230+
)
231+
}
232+
233+
/**
234+
* Gets the enclosing statement of the given variable, if any.
235+
*/
236+
private Stmt getEnclosingStmt(LocalScopeVariable v) {
237+
result.(DeclStmt).getADeclaration() = v
238+
or
239+
exists(ConditionDeclExpr cde |
240+
cde.getVariable() = v and
241+
result = cde.getEnclosingStmt()
242+
)
243+
or
244+
exists(CatchBlock cb |
245+
cb.getParameter() = v and
246+
result = cb.getEnclosingStmt()
247+
)
213248
}
214249

215250
/** Holds if `v2` hides `v1`. */

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,6 @@
55
| test.cpp:23:13:23:15 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
66
| test.cpp:26:12:26:14 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 |
77
| test.cpp:27:14:27:16 | id1 | Variable is hiding variable $@. | test.cpp:26:12:26:14 | id1 | id1 |
8+
| test.cpp:65:11:65:11 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
9+
| test.cpp:67:9:67:9 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |
10+
| test.cpp:70:12:70:12 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i |

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,38 @@ template <class T> constexpr T foo1 = T(1.1L);
4040
template <class T, class R> T f(T r) {
4141
T v = foo1<T> * r * r; // COMPLIANT
4242
T v1 = foo1<R> * r * r; // COMPLIANT
43+
}
44+
45+
void test_scope_order() {
46+
{
47+
{
48+
int i; // COMPLIANT
49+
}
50+
int i; // COMPLIANT
51+
}
52+
53+
for (int i = 0; i < 10; i++) { // COMPLIANT
54+
}
55+
56+
try {
57+
58+
} catch (int i) { // COMPLIANT
59+
}
60+
61+
int i; // COMPLIANT
62+
63+
{
64+
{
65+
int i; // NON_COMPLIANT
66+
}
67+
int i; // NON_COMPLIANT
68+
}
69+
70+
for (int i = 0; i < 10; i++) { // NON_COMPLIANT
71+
}
72+
73+
try {
74+
75+
} catch (int i) { // NON_COMPLIANT[FALSE_NEGATIVE]
76+
}
4377
}

0 commit comments

Comments
 (0)