diff --git a/change_notes/2024-12-08-identifier-hiding.md b/change_notes/2024-12-08-identifier-hiding.md new file mode 100644 index 0000000000..b769b16e57 --- /dev/null +++ b/change_notes/2024-12-08-identifier-hiding.md @@ -0,0 +1,7 @@ + - `A2-10-1` - `IdentifierHiding.ql`: + - Improved evaluation performance. + - Addressed false negatives where nested loops used the same variable name. + - Exclude cases where a variable declared in a lambda expression shadowed a global or namespace variable that did not appear in the same translation unit. + - `RULE-5-3` - `IdentifierHidingC.ql`: + - Improved evaluation performance. + - Addressed false negatives where nested loops used the same variable name. \ No newline at end of file diff --git a/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql b/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql index 237ebbe985..f6fe18a3db 100644 --- a/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql +++ b/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql @@ -20,6 +20,6 @@ from UserVariable v, UserVariable hidden where not isExcluded(v, ScopePackage::localConstructorInitializedObjectHidesIdentifierQuery()) and v.getInitializer().getExpr() instanceof ConstructorCall and - hides(hidden, v) + hidesStrict(hidden, v) select v, "The declaration declares variable " + v.getName() + " that hides $@", hidden, hidden.getName() diff --git a/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql b/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql index 3f91530c84..368c0a05e4 100644 --- a/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql +++ b/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql @@ -13,7 +13,6 @@ import cpp import codingstandards.cpp.cert -import codingstandards.cpp.Scope class LocalUserFunctionDeclarationEntry extends FunctionDeclarationEntry { DeclStmt ds; diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index d9a81b98e3..5438c17133 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -3,56 +3,86 @@ */ import cpp +import codingstandards.cpp.ConstHelpers /** - * Gets the parent scope of this `Element`, if any. - * A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`, - * or certain kinds of `Statement`. - * Differs from `Element::getParentScope` when `e` is a `LoopControlVariable` + * a `Variable` that is nonvolatile and const + * and of type `IntegralOrEnumType` */ -private Element getParentScope(Element e) { - /* - * A `Block` cannot have a `ForStmt` as its parent scope so we have to special case - * for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope. - * If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because - * the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include - * outer scope. Blocks can only have blocks as parent scopes. - * void f() { - * for( int i1; ... ) { - * for (int i2; ...) { - * } - * } - * } - */ +class NonVolatileConstIntegralOrEnumVariable extends Variable { + NonVolatileConstIntegralOrEnumVariable() { + not this.isVolatile() and + this.isConst() and + this.getUnspecifiedType() instanceof IntegralOrEnumType + } +} - exists(Loop loop | loop.getStmt() = e and result = loop) - or - exists(IfStmt ifStmt | - (ifStmt.getThen() = e or ifStmt.getElse() = e) and - result = ifStmt - ) - or - exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt) - or - not result.(Loop).getStmt() = e and - not result.(IfStmt).getThen() = e and - not result.(IfStmt).getElse() = e and - not result.(SwitchStmt).getStmt() = e and - if exists(e.getParentScope()) - then result = e.getParentScope() - else ( - // Statements do no have a parent scope, so return the enclosing block. - result = e.(Stmt).getEnclosingBlock() +/** + * Internal module, exposed for testing. + */ +module Internal { + /** + * Gets the parent scope of this `Element`, if any. + * A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`, + * or certain kinds of `Statement`. + * Differs from `Element::getParentScope` when `e` is a `LoopControlVariable` + */ + Element getParentScope(Element e) { + /* + * A `Block` cannot have a `ForStmt` as its parent scope so we have to special case + * for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope. + * If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because + * the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include + * outer scope. Blocks can only have blocks as parent scopes. + * void f() { + * for( int i1; ... ) { + * for (int i2; ...) { + * } + * } + * } + */ + + exists(Loop loop | loop.getAChild() = e and result = loop) + or + exists(IfStmt ifStmt | + (ifStmt.getThen() = e or ifStmt.getElse() = e) and + result = ifStmt + ) + or + exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt) + or + // A catch-block parameter, whose parent is the `Handler` + exists(CatchBlock c | c.getParameter() = e and result = c.getParent()) or - result = e.(Expr).getEnclosingBlock() + // A catch-block `Handler`, whose parent is the `TryStmt` + e.(Handler).getParent() = result or - // Catch block parameters don't have an enclosing scope, so attach them to the - // the block itself - exists(CatchBlock cb | - e = cb.getParameter() and - result = cb + // The parent scope of a lambda is the scope in which the lambda expression is defined. + // + // Lambda functions are defined in a generated `Closure` class, as the `operator()` function. We choose the + // enclosing statement of the lambda expression as the parent scope of the lambda function. This is so we can + // determine the order of definition if a variable is defined in the same scope as the lambda expression. + exists(Closure lambdaClosure | + lambdaClosure.getLambdaFunction() = e and + lambdaClosure.getLambdaExpression().getEnclosingStmt() = result ) - ) + or + not exists(Loop loop | loop.getAChild() = e) and + not exists(IfStmt ifStmt | ifStmt.getThen() = e or ifStmt.getElse() = e) and + not exists(SwitchStmt switchStmt | switchStmt.getStmt() = e) and + not exists(CatchBlock c | c.getParameter() = e) and + not e instanceof Handler and + not exists(Closure lambdaClosure | lambdaClosure.getLambdaFunction() = e) and + if exists(e.getParentScope()) + then result = e.getParentScope() + else ( + // Statements do not have a parent scope, so return the enclosing block. + result = e.(Stmt).getEnclosingBlock() + or + // Expressions do not have a parent scope, so return the enclosing block. + result = e.(Expr).getEnclosingBlock() + ) + } } /** A variable which is defined by the user, rather than being from a third party or compiler generated. */ @@ -68,19 +98,29 @@ class UserVariable extends Variable { /** An element which is the parent scope of at least one other element in the program. */ class Scope extends Element { - Scope() { this = getParentScope(_) } + Scope() { this = Internal::getParentScope(_) } - UserVariable getAVariable() { getParentScope(result) = this } + UserVariable getAVariable() { Internal::getParentScope(result) = this } + + /** + * Gets the `Variable` with the given `name` that is declared in this scope. + */ + UserVariable getVariable(string name) { + result = getAVariable() and + result.getName() = name + } int getNumberOfVariables() { result = count(getAVariable()) } Scope getAnAncestor() { result = this.getStrictParent+() } - Scope getStrictParent() { result = getParentScope(this) } + Scope getAChildScope() { result.getStrictParent() = this } + + Scope getStrictParent() { result = Internal::getParentScope(this) } - Declaration getADeclaration() { getParentScope(result) = this } + Declaration getADeclaration() { Internal::getParentScope(result) = this } - Expr getAnExpr() { this = getParentScope(result) } + Expr getAnExpr() { this = Internal::getParentScope(result) } private predicate getLocationInfo( PreprocessorBranchDirective pbd, string file1, string file2, int startline1, int startline2 @@ -112,43 +152,136 @@ class Scope extends Element { predicate isGenerated() { this instanceof GeneratedBlockStmt } int getDepth() { - exists(Scope parent | parent = getParentScope(this) and result = 1 + parent.getDepth()) + exists(Scope parent | + parent = Internal::getParentScope(this) and result = 1 + parent.getDepth() + ) or - not exists(getParentScope(this)) and result = 0 + not exists(Internal::getParentScope(this)) and result = 0 } -} -class GeneratedBlockStmt extends BlockStmt { - GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation } -} + /** + * Holds if `name` is declared in this scope, or in a nested scope. + */ + private predicate isNameDeclaredInThisOrNestedScope(string name) { + name = getAVariable().getName() + or + isNameDeclaredInNestedScope(name) + } + + /** + * Holds if `name` is declared in a nested scope. + */ + private predicate isNameDeclaredInNestedScope(string name) { + this.getAChildScope().isNameDeclaredInThisOrNestedScope(name) + } + + /** + * Holds if `name` is declared in this scope and is hidden in a child scope. + */ + private predicate isDeclaredNameHiddenByChild(string name) { + isNameDeclaredInNestedScope(name) and + name = getAVariable().getName() + } + + /** + * Gets a variable with `name` which is potentially hidden in at least one nested scope. + */ + private UserVariable getAPotentiallyHiddenVariable(string name) { + result = getAVariable() and + result.getName() = name and + isDeclaredNameHiddenByChild(name) + } -/** Gets a variable that is in the potential scope of variable `v`. */ -private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) { - exists(Scope s | - result = s.getAVariable() and + /** + * Holds if `name` is declared above this scope and hidden by this or a nested scope. + */ + UserVariable getAVariableHiddenByThisOrNestedScope(string name) { + exists(Scope parent | parent = this.getStrictParent() | + result = parent.getAPotentiallyHiddenVariable(name) or + result = parent.getAVariableHiddenByThisOrNestedScope(name) + ) and + isNameDeclaredInThisOrNestedScope(name) + } + + /** + * Holds if `hiddenVariable` and `hidingVariable` are a candidate hiding pair at this scope. + */ + private predicate hidesCandidate( + UserVariable hiddenVariable, UserVariable hidingVariable, string name + ) { ( - // 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 + // Declared in this scope + hidingVariable = getVariable(name) and + hiddenVariable = getAVariableHiddenByThisOrNestedScope(name) or - // In the same scope, but not the same variable, and choose just one to report - v = s.getAVariable() and - not result = v and - v.getName() <= result.getName() + // Declared in a child scope + exists(Scope child | + getAChildScope() = child and + child.hidesCandidate(hiddenVariable, hidingVariable, name) + ) ) - ) + } + + /** + * Holds if `hiddenVariable` is declared in this scope and hidden by `hidingVariable`. + */ + predicate hides(UserVariable hiddenVariable, UserVariable hidingVariable, Scope childScope) { + exists(string name | + hiddenVariable = getAPotentiallyHiddenVariable(name) and + childScope = getAChildScope() and + childScope.hidesCandidate(hiddenVariable, hidingVariable, name) + ) + } } -/** 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 +/** + * A scope representing the generated `operator()` of a lambda function. + */ +class LambdaScope extends Scope { + Closure closure; + + LambdaScope() { this = closure.getLambdaFunction() } + + override UserVariable getAVariableHiddenByThisOrNestedScope(string name) { + // Handle special cases for lambdas + exists(UserVariable hiddenVariable, LambdaExpression lambdaExpr | + // Find the variable that is potentially hidden inside the lambda + hiddenVariable = super.getAVariableHiddenByThisOrNestedScope(name) and + result = hiddenVariable and + lambdaExpr = closure.getLambdaExpression() + | + // A definition can be hidden if it is in scope and it is captured by the lambda, + exists(LambdaCapture cap | + lambdaExpr.getACapture() = cap and + // The outer declaration is captured by the lambda + hiddenVariable.getAnAccess() = cap.getInitializer() + ) + or + // it is is non-local, + hiddenVariable instanceof GlobalVariable + or + // it has static or thread local storage duration, + (hiddenVariable.isThreadLocal() or hiddenVariable.isStatic()) + or + //it is a reference that has been initialized with a constant expression. + hiddenVariable.getType().stripTopLevelSpecifiers() instanceof ReferenceType and + hiddenVariable.getInitializer().getExpr() instanceof Literal + or + // //it const non-volatile integral or enumeration type and has been initialized with a constant expression + hiddenVariable instanceof NonVolatileConstIntegralOrEnumVariable and + hiddenVariable.getInitializer().getExpr() instanceof Literal + or + //it is constexpr and has no mutable members + hiddenVariable.isConstexpr() and + not exists(Class c | + c = hiddenVariable.getType() and not c.getAMember() instanceof MutableVariable + ) ) - ) + } +} + +class GeneratedBlockStmt extends BlockStmt { + GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation } } /** Holds if there exists a translation unit that includes both `f1` and `f2`. */ @@ -161,21 +294,17 @@ predicate inSameTranslationUnit(File f1, File f2) { } /** - * Gets a user variable which occurs in the "potential scope" of variable `v`. - */ -cached -UserVariable getPotentialScopeOfVariable(UserVariable v) { - result = getPotentialScopeOfVariable_candidate(v) and - inSameTranslationUnit(v.getFile(), result.getFile()) -} - -/** - * Gets a user variable which occurs in the "outer scope" of variable `v`. + * Holds if there exists a translation unit that includes both `f1` and `f2`. + * + * This version is late bound. */ -cached -UserVariable getPotentialScopeOfVariableStrict(UserVariable v) { - result = getOuterScopesOfVariable_candidate(v) and - inSameTranslationUnit(v.getFile(), result.getFile()) +bindingset[f1, f2] +pragma[inline_late] +predicate inSameTranslationUnitLate(File f1, File f2) { + exists(TranslationUnit c | + c.getAUserFile() = f1 and + c.getAUserFile() = f2 + ) } /** A file that is a C/C++ source file */ @@ -203,67 +332,27 @@ 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) { - 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()) and - ( - // If v1 is a local variable, ensure that v1 is declared before v2 +/** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ +predicate hides_candidateStrict(UserVariable v1, UserVariable v2) { + exists(Scope parentScope, Scope childScope | parentScope.hides(v1, v2, childScope) | + // If v1 is a local variable defined in a `DeclStmt` ensure that it is declared before `v2`, + // otherwise it would not be hidden ( - v1 instanceof LocalVariable and - // Ignore variables declared in conditional expressions, as they apply to - // the nested scope - not v1 = any(ConditionDeclExpr cde).getVariable() and - // Ignore variables declared in loops - not exists(Loop l | l.getADeclaration() = v1) + parentScope instanceof BlockStmt and + exists(DeclStmt ds | ds.getADeclaration() = v1) and + exists(parentScope.(BlockStmt).getIndexOfStmt(childScope)) ) implies exists(BlockStmt bs, DeclStmt v1Stmt, Stmt v2Stmt | - v1 = v1Stmt.getADeclaration() and - getEnclosingStmt(v2).getParentStmt*() = v2Stmt + bs = parentScope and + v2Stmt = childScope and + v1Stmt.getADeclaration() = v1 | bs.getIndexOfStmt(v1Stmt) <= bs.getIndexOfStmt(v2Stmt) ) - ) -} - -/** - * Gets the enclosing statement of the given variable, if any. - */ -private Stmt getEnclosingStmt(LocalScopeVariable v) { - result.(DeclStmt).getADeclaration() = v - or - exists(ConditionDeclExpr cde | - cde.getVariable() = v and - result = cde.getEnclosingStmt() - ) - or - exists(CatchBlock cb | - cb.getParameter() = v and - result = cb.getEnclosingStmt() - ) -} - -/** Holds if `v2` hides `v1`. */ -predicate hides(UserVariable v1, UserVariable v2) { - hides_candidate(v1, v2) and - // Confirm that there's no closer candidate variable which `v2` hides - not exists(UserVariable mid | - hides_candidate(v1, mid) and - hides_candidate(mid, v2) - ) + ) and + inSameTranslationUnitLate(v1.getFile(), v2.getFile()) and + not (v1.isMember() or v2.isMember()) } /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ @@ -291,6 +380,8 @@ predicate hasBlockScope(Declaration decl) { exists(BlockStmt b | b.getADeclarati /** * identifiers in nested (named/nonglobal) namespaces are exceptions to hiding due to being able access via fully qualified ids */ +bindingset[outerDecl, innerDecl] +pragma[inline_late] predicate excludedViaNestedNamespaces(UserVariable outerDecl, UserVariable innerDecl) { exists(Namespace inner, Namespace outer | outer.getAChildNamespace+() = inner and diff --git a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll index 87a4580ab3..5c7475883e 100644 --- a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll +++ b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll @@ -46,16 +46,32 @@ string step1(string s) { string step2(string s) { s = "m_" and result = "rn" } -predicate violation(UserVariable v1, UserVariable v2) { - v2 = getPotentialScopeOfVariable(v1) and +class VariableName extends string { + VariableName() { exists(UserVariable uv | uv.getName() = this) } + + string getCanon() { + result = + this.toLowerCase() + .replaceAll("_", "") + .regexpReplaceAll("[il]", "1") + .replaceAll("s", "5") + .replaceAll("z", "2") + .replaceAll("b", "8") + .replaceAll("h", "n") + .replaceAll("m", "rn") + .replaceAll("o", "0") + } +} + +predicate isConflictingName(VariableName name1, VariableName name2) { exists(string s1, string s2 | // over-approximate a match, because it is cheaper to compute - getCanon(v1) = getCanon(v2) and - v1 != v2 and - not v1.getName() = v2.getName() and + name1.getCanon() = name2.getCanon() and + // Exclude identical names + not name1 = name2 and // expand 'm' to 'm_' to match either 'm_' or 'rn' - s1 = v1.getName().replaceAll("_", "").replaceAll("m", "m_") and - s2 = v2.getName().replaceAll("_", "").replaceAll("m", "m_") and + s1 = name1.replaceAll("_", "").replaceAll("m", "m_") and + s2 = name2.replaceAll("_", "").replaceAll("m", "m_") and // at this point the strings must have equal length, the substitutions do not expand nor contract the string s1.length() = s2.length() and forall(int i | i in [0 .. s1.length() - 1] | @@ -87,6 +103,23 @@ predicate violation(UserVariable v1, UserVariable v2) { ) } +predicate violation(UserVariable v1, UserVariable v2) { + exists(string name1, string name2 | + isConflictingName(name1, name2) and + exists(Scope parentScope, Scope childScope | + parentScope.getVariable(name1) = v1 and + childScope.getVariable(name2) = v2 + | + childScope.getStrictParent+() = parentScope + or + // Disambiguate names in the same scope by name order + childScope = parentScope and + name1 < name2 + ) and + inSameTranslationUnitLate(v1.getFile(), v2.getFile()) + ) +} + query predicate problems( UserVariable v, string message, UserVariable v1, string v1Description, UserVariable v2, string v2Description diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 9534c2f78a..39d24299b8 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -9,75 +9,11 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.Scope -import codingstandards.cpp.ConstHelpers abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } -/** - * a `Variable` that is nonvolatile and const - * and of type `IntegralOrEnumType` - */ -class NonVolatileConstIntegralOrEnumVariable extends Variable { - NonVolatileConstIntegralOrEnumVariable() { - not this.isVolatile() and - this.isConst() and - this.getUnspecifiedType() instanceof IntegralOrEnumType - } -} - -/** - * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` by the lambda. - */ -predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { - exists( - Scope innerScope, LambdaExpression lambdaExpr, Scope lambdaExprScope, Scope outerScope, - Closure lambdaClosure - | - // The variable `innerDecl` is declared inside of the lambda. - innerScope.getADeclaration() = innerDecl and - // Because a lambda is compiled down to a closure, we need to use the closure to determine if the declaration - // is part of the lambda. - innerScope.getAnAncestor() = lambdaClosure and - // Next we determine the scope of the lambda expression to determine if `outerDecl` is visible in the scope of the lambda. - lambdaClosure.getLambdaExpression() = lambdaExpr and - lambdaExprScope.getAnExpr() = lambdaExpr and - outerScope.getADeclaration() = outerDecl and - lambdaExprScope.getStrictParent*() = outerScope and - ( - // A definition can be hidden if it is in scope and it is captured by the lambda, - exists(LambdaCapture cap | - lambdaExpr.getACapture() = cap and - // The outer declaration is captured by the lambda - outerDecl.getAnAccess() = cap.getInitializer() - ) - or - // it is is non-local, - outerDecl instanceof GlobalVariable - or - // it has static or thread local storage duration, - (outerDecl.isThreadLocal() or outerDecl.isStatic()) - or - //it is a reference that has been initialized with a constant expression. - outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and - outerDecl.getInitializer().getExpr() instanceof Literal - or - // //it const non-volatile integral or enumeration type and has been initialized with a constant expression - outerDecl instanceof NonVolatileConstIntegralOrEnumVariable and - outerDecl.getInitializer().getExpr() instanceof Literal - or - //it is constexpr and has no mutable members - outerDecl.isConstexpr() and - not exists(Class c | - c = outerDecl.getType() and not c.getAMember() instanceof MutableVariable - ) - ) and - // Finally, the variables must have the same names. - innerDecl.getName() = outerDecl.getName() - ) -} - query predicate problems( UserVariable innerDecl, string message, UserVariable outerDecl, string varName ) { @@ -86,7 +22,7 @@ query predicate problems( //ignore template variables for this rule not outerDecl instanceof TemplateVariable and not innerDecl instanceof TemplateVariable and - (hidesStrict(outerDecl, innerDecl) or hiddenInLambda(outerDecl, innerDecl)) and + hidesStrict(outerDecl, innerDecl) and not excludedViaNestedNamespaces(outerDecl, innerDecl) and varName = outerDecl.getName() and message = "Variable is hiding variable $@." diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected new file mode 100644 index 0000000000..90aa3b30c8 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected @@ -0,0 +1,91 @@ +| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= | +| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= | +| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= | +| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= | +| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) | +| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) | +| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | operator= | +| file://:0:0:0:0 | __va_list_tag | file://:0:0:0:0 | (global namespace) | +| file://:0:0:0:0 | fp_offset | file://:0:0:0:0 | __va_list_tag | +| file://:0:0:0:0 | gp_offset | file://:0:0:0:0 | __va_list_tag | +| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag | +| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag | +| file://:0:0:0:0 | overflow_arg_area | file://:0:0:0:0 | __va_list_tag | +| file://:0:0:0:0 | reg_save_area | file://:0:0:0:0 | __va_list_tag | +| test.cpp:1:5:1:7 | id1 | file://:0:0:0:0 | (global namespace) | +| test.cpp:3:11:3:13 | ns1 | file://:0:0:0:0 | (global namespace) | +| test.cpp:4:5:4:7 | id1 | test.cpp:3:11:3:13 | ns1 | +| test.cpp:6:11:6:13 | ns1::ns2 | test.cpp:3:11:3:13 | ns1 | +| test.cpp:7:5:7:7 | id1 | test.cpp:6:11:6:13 | ns1::ns2 | +| test.cpp:8:7:8:7 | C1 | test.cpp:8:7:8:8 | C1 | +| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 | +| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 | +| test.cpp:8:7:8:8 | C1 | test.cpp:6:11:6:13 | ns1::ns2 | +| test.cpp:9:7:9:9 | id1 | test.cpp:8:7:8:8 | C1 | +| test.cpp:10:8:10:17 | test_scope | test.cpp:8:7:8:8 | C1 | +| test.cpp:10:23:10:25 | id1 | test.cpp:10:8:10:17 | test_scope | +| test.cpp:10:28:34:3 | { ... } | test.cpp:10:8:10:17 | test_scope | +| test.cpp:11:5:33:5 | for(...;...;...) ... | test.cpp:10:28:34:3 | { ... } | +| test.cpp:11:10:11:17 | declaration | test.cpp:11:5:33:5 | for(...;...;...) ... | +| test.cpp:11:14:11:16 | id1 | test.cpp:11:5:33:5 | for(...;...;...) ... | +| test.cpp:11:19:11:21 | id1 | test.cpp:10:28:34:3 | { ... } | +| test.cpp:11:19:11:25 | ... < ... | test.cpp:11:5:33:5 | for(...;...;...) ... | +| test.cpp:11:25:11:25 | 1 | test.cpp:10:28:34:3 | { ... } | +| test.cpp:11:28:11:30 | id1 | test.cpp:10:28:34:3 | { ... } | +| test.cpp:11:28:11:32 | ... ++ | test.cpp:11:5:33:5 | for(...;...;...) ... | +| test.cpp:11:35:33:5 | { ... } | test.cpp:11:5:33:5 | for(...;...;...) ... | +| test.cpp:12:7:32:7 | for(...;...;...) ... | test.cpp:11:35:33:5 | { ... } | +| test.cpp:12:12:12:19 | declaration | test.cpp:12:7:32:7 | for(...;...;...) ... | +| test.cpp:12:16:12:18 | id1 | test.cpp:12:7:32:7 | for(...;...;...) ... | +| test.cpp:12:21:12:23 | id1 | test.cpp:11:35:33:5 | { ... } | +| test.cpp:12:21:12:27 | ... < ... | test.cpp:12:7:32:7 | for(...;...;...) ... | +| test.cpp:12:27:12:27 | 1 | test.cpp:11:35:33:5 | { ... } | +| test.cpp:12:30:12:32 | id1 | test.cpp:11:35:33:5 | { ... } | +| test.cpp:12:30:12:34 | ... ++ | test.cpp:12:7:32:7 | for(...;...;...) ... | +| test.cpp:12:37:32:7 | { ... } | test.cpp:12:7:32:7 | for(...;...;...) ... | +| test.cpp:13:9:31:9 | { ... } | test.cpp:12:37:32:7 | { ... } | +| test.cpp:14:11:14:18 | declaration | test.cpp:13:9:31:9 | { ... } | +| test.cpp:14:15:14:17 | id1 | test.cpp:13:9:31:9 | { ... } | +| test.cpp:16:11:20:11 | if (...) ... | test.cpp:13:9:31:9 | { ... } | +| test.cpp:16:15:16:17 | id1 | test.cpp:13:9:31:9 | { ... } | +| test.cpp:16:15:16:22 | ... == ... | test.cpp:13:9:31:9 | { ... } | +| test.cpp:16:22:16:22 | 0 | test.cpp:13:9:31:9 | { ... } | +| test.cpp:16:25:18:11 | { ... } | test.cpp:16:11:20:11 | if (...) ... | +| test.cpp:17:13:17:20 | declaration | test.cpp:16:25:18:11 | { ... } | +| test.cpp:17:17:17:19 | id1 | test.cpp:16:25:18:11 | { ... } | +| test.cpp:18:18:20:11 | { ... } | test.cpp:16:11:20:11 | if (...) ... | +| test.cpp:19:13:19:20 | declaration | test.cpp:18:18:20:11 | { ... } | +| test.cpp:19:17:19:19 | id1 | test.cpp:18:18:20:11 | { ... } | +| test.cpp:21:11:25:11 | switch (...) ... | test.cpp:13:9:31:9 | { ... } | +| test.cpp:21:19:21:21 | id1 | test.cpp:13:9:31:9 | { ... } | +| test.cpp:21:24:25:11 | { ... } | test.cpp:21:11:25:11 | switch (...) ... | +| test.cpp:22:11:22:17 | case ...: | test.cpp:21:24:25:11 | { ... } | +| test.cpp:22:16:22:16 | 0 | test.cpp:21:24:25:11 | { ... } | +| test.cpp:23:13:23:20 | declaration | test.cpp:21:24:25:11 | { ... } | +| test.cpp:23:17:23:19 | id1 | test.cpp:21:24:25:11 | { ... } | +| test.cpp:24:13:24:18 | break; | test.cpp:21:24:25:11 | { ... } | +| test.cpp:25:11:25:11 | label ...: | test.cpp:13:9:31:9 | { ... } | +| test.cpp:26:11:28:11 | try { ... } | test.cpp:13:9:31:9 | { ... } | +| test.cpp:26:15:28:11 | { ... } | test.cpp:13:9:31:9 | { ... } | +| test.cpp:27:13:27:53 | declaration | test.cpp:26:15:28:11 | { ... } | +| test.cpp:27:18:27:24 | lambda1 | test.cpp:26:15:28:11 | { ... } | +| test.cpp:27:27:27:52 | [...](...){...} | test.cpp:26:15:28:11 | { ... } | +| test.cpp:27:27:27:52 | {...} | test.cpp:26:15:28:11 | { ... } | +| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) | +| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) | +| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) | +| test.cpp:27:28:27:28 | operator= | file://:0:0:0:0 | decltype([...](...){...}) | +| test.cpp:27:29:27:29 | id1 | file://:0:0:0:0 | decltype([...](...){...}) | +| test.cpp:27:29:27:31 | id1 | test.cpp:26:15:28:11 | { ... } | +| test.cpp:27:33:27:33 | operator() | test.cpp:27:13:27:53 | declaration | +| test.cpp:27:36:27:52 | { ... } | test.cpp:27:33:27:33 | operator() | +| test.cpp:27:38:27:50 | declaration | test.cpp:27:36:27:52 | { ... } | +| test.cpp:27:42:27:44 | id1 | test.cpp:27:36:27:52 | { ... } | +| test.cpp:27:47:27:49 | 10 | test.cpp:27:36:27:52 | { ... } | +| test.cpp:27:52:27:52 | return ... | test.cpp:27:36:27:52 | { ... } | +| test.cpp:28:24:28:26 | id1 | test.cpp:28:29:30:11 | | +| test.cpp:28:29:30:11 | | test.cpp:26:11:28:11 | try { ... } | +| test.cpp:28:29:30:11 | { ... } | test.cpp:13:9:31:9 | { ... } | +| test.cpp:29:13:29:20 | declaration | test.cpp:28:29:30:11 | { ... } | +| test.cpp:29:17:29:19 | id1 | test.cpp:28:29:30:11 | { ... } | +| test.cpp:34:3:34:3 | return ... | test.cpp:10:28:34:3 | { ... } | diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql new file mode 100644 index 0000000000..47d27fb0f0 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql @@ -0,0 +1,5 @@ +import codingstandards.cpp.Scope + +from Element e, Element parent +where Internal::getParentScope(e) = parent +select e, parent diff --git a/cpp/common/test/library/codingstandards/cpp/scope/test.cpp b/cpp/common/test/library/codingstandards/cpp/scope/test.cpp new file mode 100644 index 0000000000..a0b617916d --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/scope/test.cpp @@ -0,0 +1,37 @@ +int id1; + +namespace ns1 { +int id1; // COMPLIANT + +namespace ns2 { +int id1; // COMPLIANT +class C1 { + int id1; + void test_scope(int id1) { + for (int id1; id1 < 1; id1++) { + for (int id1; id1 < 1; id1++) { + { + int id1; + + if (id1 == 0) { + int id1; + } else { + int id1; + } + switch (id1) { + case 0: + int id1; + break; + } + try { + auto lambda1 = [id1]() { int id1 = 10; }; + } catch (int id1) { + int id1; + } + } + } + } + } +}; +} // namespace ns2 +} // namespace ns1