Ignore globals when checking local variable names (#800)

resources/test/fixtures/N806.py

@@ -1,8 +1,12 @@
 import collections
 from collections import namedtuple
 
+GLOBAL: str = "foo"
+
 
 def f():
+    global GLOBAL
+    GLOBAL = "bar"
     lower = 0
     Camel = 0
     CONSTANT = 0

src/ast/types.rs

@@ -79,8 +79,10 @@ pub enum BindingKind {
     Annotation,
     Argument,
     Assignment,
+    // TODO(charlie): This seems to be a catch-all.
     Binding,
     LoopVar,
+    Global,
     Builtin,
     ClassDefinition,
     Definition,

src/check_ast.rs

@@ -19,8 +19,7 @@ use crate::ast::helpers::{
 use crate::ast::operations::extract_all_names;
 use crate::ast::relocate::relocate_expr;
 use crate::ast::types::{
-    Binding, BindingContext, BindingKind, ClassScope, FunctionScope, ImportKind, Range, Scope,
+    Binding, BindingContext, BindingKind, ClassScope, ImportKind, Range, Scope, ScopeKind,
-    ScopeKind,
 };
 use crate::ast::visitor::{walk_excepthandler, Visitor};
 use crate::ast::{helpers, operations, visitor};
@@ -216,21 +215,18 @@ where
         match &stmt.node {
             StmtKind::Global { names } | StmtKind::Nonlocal { names } => {
                 let global_scope_id = self.scopes[GLOBAL_SCOPE_INDEX].id;
-
+                let scope =
-                let current_scope = self.current_scope();
+                    &mut self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
-                let current_scope_id = current_scope.id;
+                if scope.id != global_scope_id {
-                if current_scope_id != global_scope_id {
                     for name in names {
-                        for scope in self.scopes.iter_mut().skip(GLOBAL_SCOPE_INDEX + 1) {
+                        scope.values.insert(
-                            scope.values.insert(
+                            name,
-                                name,
+                            Binding {
-                                Binding {
+                                kind: BindingKind::Global,
-                                    kind: BindingKind::Assignment,
+                                used: Some((global_scope_id, Range::from_located(stmt))),
-                                    used: Some((global_scope_id, Range::from_located(stmt))),
+                                range: Range::from_located(stmt),
-                                    range: Range::from_located(stmt),
+                            },
-                                },
+                        );
-                            );
-                        }
                     }
                 }
 
@@ -732,10 +728,8 @@ where
                             ));
                         }
 
-                        let scope = &mut self.scopes[*(self
+                        let scope = &mut self.scopes
-                            .scope_stack
+                            [*(self.scope_stack.last().expect("No current scope found."))];
-                            .last_mut()
-                            .expect("No current scope found."))];
                         scope.import_starred = true;
                     } else {
                         if let Some(asname) = &alias.node.asname {
@@ -1452,15 +1446,10 @@ where
 
                 if let ExprKind::Name { id, ctx } = &func.node {
                     if id == "locals" && matches!(ctx, ExprContext::Load) {
-                        let scope = &mut self.scopes[*(self
+                        let scope = &mut self.scopes
-                            .scope_stack
+                            [*(self.scope_stack.last().expect("No current scope found."))];
-                            .last_mut()
+                        if let ScopeKind::Function(inner) = &mut scope.kind {
-                            .expect("No current scope found."))];
+                            inner.uses_locals = true;
-                        if matches!(
-                            scope.kind,
-                            ScopeKind::Function(FunctionScope { uses_locals: false })
-                        ) {
-                            scope.kind = ScopeKind::Function(FunctionScope { uses_locals: true });
                         }
                     }
                 }
@@ -2243,7 +2232,16 @@ impl<'a> Checker<'a> {
 
         if self.settings.enabled.contains(&CheckCode::N806) {
             if matches!(self.current_scope().kind, ScopeKind::Function(..)) {
-                pep8_naming::plugins::non_lowercase_variable_in_function(self, expr, parent, id)
+                // Ignore globals.
+                if !self
+                    .current_scope()
+                    .values
+                    .get(id)
+                    .map(|binding| matches!(binding.kind, BindingKind::Global))
+                    .unwrap_or(false)
+                {
+                    pep8_naming::plugins::non_lowercase_variable_in_function(self, expr, parent, id)
+                }
             }
         }
 

src/pyflakes/checks.rs

@@ -55,7 +55,10 @@ pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec<Check>
 
     if matches!(
         scope.kind,
-        ScopeKind::Function(FunctionScope { uses_locals: true })
+        ScopeKind::Function(FunctionScope {
+            uses_locals: true,
+            ..
+        })
     ) {
         return checks;
     }

src/snapshots/ruff__linter__tests__N806_N806.py.snap

@@ -5,19 +5,19 @@ expression: checks
 - kind:
     NonLowercaseVariableInFunction: Camel
   location:
-    row: 7
+    row: 11
     column: 4
   end_location:
-    row: 7
+    row: 11
     column: 9
   fix: ~
 - kind:
     NonLowercaseVariableInFunction: CONSTANT
   location:
-    row: 8
+    row: 12
     column: 4
   end_location:
-    row: 8
+    row: 12
     column: 12
   fix: ~