Merge pull request github#6688 from RasmusWL/small-fix

Python: Fix `globals() == locals()` FP

Author: tausbn

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."

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. |

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 |

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 [])
-