diff --git a/python/ql/src/Statements/ModificationOfLocals.ql b/python/ql/src/Statements/ModificationOfLocals.ql index 1a76c38c52e3..5a87d60edf2d 100644 --- a/python/ql/src/Statements/ModificationOfLocals.ql +++ b/python/ql/src/Statements/ModificationOfLocals.ql @@ -30,5 +30,11 @@ predicate modification_of_locals(ControlFlowNode f) { } from AstNode a, ControlFlowNode f -where modification_of_locals(f) and a = f.getNode() +where + modification_of_locals(f) and + a = f.getNode() and + // in module level scope `locals() == globals()` + // see https://docs.python.org/3/library/functions.html#locals + // FP report in https://github.com/github/codeql/issues/6674 + not a.getScope() instanceof ModuleScope select a, "Modification of the locals() dictionary will have no effect on the local variables." diff --git a/python/ql/test/query-tests/Statements/general/C_StyleParentheses.expected b/python/ql/test/query-tests/Statements/general/C_StyleParentheses.expected index a7fa79d219a3..44ae1710468d 100644 --- a/python/ql/test/query-tests/Statements/general/C_StyleParentheses.expected +++ b/python/ql/test/query-tests/Statements/general/C_StyleParentheses.expected @@ -1,4 +1,4 @@ -| test.py:109:5:109:8 | cond | Parenthesized condition in 'if' statement. | -| test.py:112:8:112:11 | cond | Parenthesized condition in 'while' statement. | -| test.py:115:9:115:12 | test | Parenthesized test in 'assert' statement. | -| test.py:118:13:118:13 | x | Parenthesized value in 'return' statement. | +| test.py:115:5:115:8 | cond | Parenthesized condition in 'if' statement. | +| test.py:118:8:118:11 | cond | Parenthesized condition in 'while' statement. | +| test.py:121:9:121:12 | test | Parenthesized test in 'assert' statement. | +| test.py:124:13:124:13 | x | Parenthesized value in 'return' statement. | diff --git a/python/ql/test/query-tests/Statements/general/ShouldUseWithStatement.expected b/python/ql/test/query-tests/Statements/general/ShouldUseWithStatement.expected index 41a8723db03b..d062717bbf25 100644 --- a/python/ql/test/query-tests/Statements/general/ShouldUseWithStatement.expected +++ b/python/ql/test/query-tests/Statements/general/ShouldUseWithStatement.expected @@ -1 +1 @@ -| test.py:162:9:162:17 | Attribute() | Instance of context-manager class $@ is closed in a finally block. Consider using 'with' statement. | test.py:145:1:145:17 | class CM | CM | +| test.py:168:9:168:17 | Attribute() | Instance of context-manager class $@ is closed in a finally block. Consider using 'with' statement. | test.py:151:1:151:17 | class CM | CM | diff --git a/python/ql/test/query-tests/Statements/general/test.py b/python/ql/test/query-tests/Statements/general/test.py index 0f23a0b16818..eee63fa89e88 100644 --- a/python/ql/test/query-tests/Statements/general/test.py +++ b/python/ql/test/query-tests/Statements/general/test.py @@ -18,7 +18,7 @@ def return_in_finally(seq, x): finally: return 1 return 0 - + #Break in loop in finally #This is OK def return_in_loop_in_finally(f, seq): @@ -27,7 +27,7 @@ def return_in_loop_in_finally(f, seq): finally: for i in seq: break - + #But this is not def return_in_loop_in_finally(f, seq): try: @@ -49,7 +49,7 @@ def __init__(self): for x in NonIterator(): do_something(x) - + #None in for loop def dodgy_iter(x): @@ -91,8 +91,8 @@ class D(dict): pass - - + + def modification_of_locals(): x = 0 locals()['x'] = 1 @@ -104,6 +104,12 @@ def modification_of_locals(): return x +globals()['foo'] = 42 # OK +# in module-level scope `locals() == globals()` +# FP report from https://github.com/github/codeql/issues/6674 +locals()['foo'] = 43 # technically OK + + #C-style things if (cond): @@ -128,7 +134,7 @@ def __get__(self, instance, instance_type): return self.getter(instance_type) class WithClassProperty(object): - + @classproperty def x(self): return [0] @@ -143,13 +149,13 @@ def x(self): #Should use context mamager class CM(object): - + def __enter__(self): pass - + def __exit__(self, ex, cls, tb): pass - + def write(self, data): pass @@ -168,4 +174,3 @@ def assert_ok(seq): # False positive. ODASA-8042. Fixed in PR #2401. class false_positive: e = (x for x in []) -