Try to detect non-gloabl unused imports.

We do so by tracking the imports on exist of each scopestack; and on
exist, if there is an import that has not been checked; we mark it as
unused.

This seem to have side effects as the SourceToSource transformer is
aware it cannot remove non-global imports and emit a error message.

Note that this also affects conditional imports:

```
if sys.version_info > X,y:
    from foo import bar
else:
    from foo.new import bar
```

The first import will be marked as unused as shoadowed by the second one
(pyflyby does not understand the if/else).

This also does some refactor and type anotations while I am at it.

lib/python/pyflyby/_autoimp.py

@@ -4,6 +4,8 @@
 
 
 
+from __future__ import print_function, annotations
+
 import ast
 import builtins
 from   collections.abc          import Sequence
@@ -18,13 +20,13 @@
 from   pyflyby._importstmt      import Import
 from   pyflyby._log             import logger
 from   pyflyby._modules         import ModuleHandle
-from   pyflyby._parse           import (PythonBlock, _is_ast_str,
-                                        infer_compile_mode, MatchAs)
+from   pyflyby._parse           import (MatchAs, PythonBlock, _is_ast_str,
+                                        infer_compile_mode)
 
 from   six                      import reraise
 import sys
 import types
-from   typing                   import Any, Set, Optional, List, Tuple
+from typing import Any, List, Optional, Set, Tuple, Union, Dict
 
 if sys.version_info >= (3, 12):
     ATTRIBUTE_NAME = "value"
@@ -130,9 +132,13 @@ def __getitem__(self, item):
     def __len__(self):
         return len(self._tup)
 
-    def with_new_scope(
-        self, include_class_scopes=False, new_class_scope=False, unhide_classdef=False
-    ):
+    def _with_new_scope(
+        self,
+        *,
+        include_class_scopes: bool,
+        new_class_scope: bool,
+        unhide_classdef: bool,
+    ) -> ScopeStack:
         """
         Return a new ``ScopeStack`` with an additional empty scope.
 
@@ -148,8 +154,8 @@ def with_new_scope(
         if include_class_scopes:
             scopes = tuple(self)
         else:
-            scopes = tuple(s for s in self
-                           if not isinstance(s, _ClassScope))
+            scopes = tuple(s for s in self if not isinstance(s, _ClassScope))
+        new_scope: Union[_ClassScope, Dict[str, Any]]
         if new_class_scope:
             new_scope = _ClassScope()
         else:
@@ -199,7 +205,7 @@ def merged_to_two(self):
         return (d, self[-1])
 
 
-    def has_star_import(self):
+    def has_star_import(self) -> bool:
         """
         Return whether there are any star-imports in this ScopeStack.
         Only relevant in AST-based static analysis mode.
@@ -355,9 +361,10 @@ def __init__(self, name: str, source: str, lineno: int):
         self.name = name
         self.source = source # generally an Import
         self.lineno = lineno
+        logger.debug("Create _UseChecker : %r", self)
 
     def __repr__(self):
-        return f"<{type(self).__name__}: name:{self.name} source:{self.source!r} lineno:{self.lineno} used:{self.used}>"
+        return f"<{type(self).__name__}: name:{self.name!r} source:{self.source!r} lineno:{self.lineno} used:{self.used}>"
 
 
 class _MissingImportFinder:
@@ -397,13 +404,13 @@ def __init__(self, scopestack, *, find_unused_imports:bool, parse_docstrings:boo
         # Create a stack of namespaces.  The caller should pass in a list that
         # includes the globals dictionary.  ScopeStack() will make sure this
         # includes builtins.
-        scopestack = ScopeStack(scopestack)
+        _scopestack = ScopeStack(scopestack)
 
         # Add an empty namespace to the stack.  This facilitates adding stuff
         # to scopestack[-1] without ever modifying user globals.
-        scopestack = scopestack.with_new_scope()
-
-        self.scopestack = scopestack
+        self.scopestack = _scopestack._with_new_scope(
+            include_class_scopes=False, new_class_scope=False, unhide_classdef=False
+        )
 
         # Create data structure to hold the result.
         # missing_imports is a list of (lineno, DottedIdentifier) tuples.
@@ -450,6 +457,7 @@ def scan_for_import_issues(self, codeblock: PythonBlock):
         # references in doctests to be noted as missing-imports.  For now we
         # just let the code accumulate into self.missing_imports and ignore
         # the result.
+        logger.debug("unused: %r", self.unused_imports)
         missing_imports = sorted(self.missing_imports)
         if self.parse_docstrings and self.unused_imports is not None:
             doctest_blocks = codeblock.get_doctests()
@@ -467,10 +475,8 @@ def scan_for_import_issues(self, codeblock: PythonBlock):
                 # Currently we don't support the 'global' keyword anyway so
                 # this doesn't matter yet, and it's uncommon to use 'global'
                 # in a doctest, so this is low priority to fix.
-                oldstack = self.scopestack
-                self.scopestack = self.scopestack.with_new_scope()
-                self._scan_node(block.ast_node)
-                self.scopestack = oldstack
+                with self._NewScopeCtx(check_unused_imports=False):
+                    self._scan_node(block.ast_node)
             # Find literal brace identifiers like "... `Foo` ...".
             # TODO: Do this inline: (1) faster; (2) can use proper scope of vars
             # Once we do that, use _check_load() with new args
@@ -553,17 +559,36 @@ def generic_visit(self, node):
 
 
     @contextlib.contextmanager
-    def _NewScopeCtx(self, **kwargs):
+    def _NewScopeCtx(
+        self,
+        include_class_scopes=False,
+        new_class_scope=False,
+        unhide_classdef=False,
+        check_unused_imports=True,
+    ):
         """
         Context manager that temporarily pushes a new empty namespace onto the
         stack of namespaces.
         """
         prev_scopestack = self.scopestack
-        new_scopestack = prev_scopestack.with_new_scope(**kwargs)
+        new_scopestack = prev_scopestack._with_new_scope(
+            include_class_scopes=include_class_scopes,
+            new_class_scope=new_class_scope,
+            unhide_classdef=unhide_classdef,
+        )
         self.scopestack = new_scopestack
         try:
             yield
         finally:
+            logger.debug("throwing last scope from scopestack: %r", new_scopestack[-1])
+            for name, use_checker in new_scopestack[-1].items():
+                if use_checker and use_checker.used == False and check_unused_imports:
+                    logger.debug(
+                        "unused checker %r scopestack_depth %r",
+                        use_checker,
+                        len(self.scopestack),
+                    )
+                    self.unused_imports.append((use_checker.lineno, use_checker.source))
             assert self.scopestack is new_scopestack
             self.scopestack = prev_scopestack
 
@@ -693,7 +718,7 @@ def visit_Lambda(self, node):
                 self.visit(node.body)
             self._in_FunctionDef = old_in_FunctionDef
 
-    def _visit_typecomment(self, typecomment):
+    def _visit_typecomment(self, typecomment: str) -> None:
         """
         Warning, when a type comment the node is a string, not an ast node.
         We also get two types of type comments:
@@ -716,14 +741,15 @@ def foo(a #type: int
         """
         if typecomment is None:
             return
+        node: Union[ast.Module, ast.FunctionType]
         if '->' in typecomment:
             node = ast.parse(typecomment, mode='func_type')
         else:
             node = ast.parse(typecomment)
 
         self.visit(node)
 
-    def visit_arguments(self, node):
+    def visit_arguments(self, node) -> None:
         assert node._fields == ('posonlyargs', 'args', 'vararg', 'kwonlyargs', 'kw_defaults', 'kwarg', 'defaults'), node._fields
         # Argument/parameter list.  Note that the defaults should be
         # considered "Load"s from the upper scope, and the argument names are
@@ -754,7 +780,7 @@ def visit_arguments(self, node):
         else:
             self._visit_Store(node.kwarg)
 
-    def visit_ExceptHandler(self, node):
+    def visit_ExceptHandler(self, node) -> None:
         assert node._fields == ('type', 'name', 'body')
         if node.type:
             self.visit(node.type)
@@ -948,7 +974,12 @@ def _visit_StoreImport(self, node, modulename):
             value = _UseChecker(name, imp, self._lineno)
         self._visit_Store(name, value)
 
-    def _visit_Store(self, fullname:str, value=None):
+    def _visit_Store(self, fullname: str, value: Optional[_UseChecker] = None):
+        """
+        Visit a Store action, check for unused import
+        and add current value to the last scope.
+        """
+        assert isinstance(value, (_UseChecker, type(None)))
         logger.debug("_visit_Store(%r)", fullname)
         if fullname is None:
             return

lib/python/pyflyby/_importdb.py

@@ -2,6 +2,8 @@
 # Copyright (C) 2011, 2012, 2013, 2014, 2015 Karl Chen.
 # License: MIT http://opensource.org/licenses/MIT
 
+from __future__ import annotations
+
 
 
 from   collections              import defaultdict
@@ -417,7 +419,7 @@ def get_default(cls, target_filename: Union[Filename, str], /):
         return result
 
     @classmethod
-    def interpret_arg(cls, arg, target_filename):
+    def interpret_arg(cls, arg, target_filename) -> ImportDB:
         if arg is None:
             return cls.get_default(target_filename)
         else:

lib/python/pyflyby/_imports2s.py

@@ -13,12 +13,12 @@
 from   pyflyby._util            import ImportPathCtx, Inf, NullCtx, memoize
 import re
 
-from typing import Union
+from typing import Union, Optional, Literal
 
 from textwrap import indent
 
 
-class SourceToSourceTransformationBase(object):
+class SourceToSourceTransformationBase:
 
     input: PythonBlock
 
@@ -121,7 +121,7 @@ def pretty_print(self, params=None):
         result = [block.pretty_print(params=params) for block in self.blocks]
         return FileText.concatenate(result)
 
-    def find_import_block_by_lineno(self, lineno):
+    def find_import_block_by_lineno(self, lineno: int):
         """
         Find the import block containing the given line number.
 
@@ -312,11 +312,11 @@ def ImportPathForRelativeImportsCtx(codeblock):
 
 
 def fix_unused_and_missing_imports(
-    codeblock: Union[PythonBlock, str],
-    add_missing=True,
-    remove_unused="AUTOMATIC",
-    add_mandatory=True,
-    db=None,
+    codeblock: Union[PythonBlock, str, Filename],
+    add_missing: bool = True,
+    remove_unused: Union[Literal["AUTOMATIC"], bool] = "AUTOMATIC",
+    add_mandatory: bool = True,
+    db: Optional[ImportDB] = None,
     params=None,
 ) -> PythonBlock:
     r"""
@@ -345,6 +345,7 @@ def fix_unused_and_missing_imports(
     :rtype:
       `PythonBlock`
     """
+    _codeblock: PythonBlock
     if isinstance(codeblock, Filename):
         _codeblock = PythonBlock(codeblock)
     if not isinstance(codeblock, PythonBlock):
@@ -393,7 +394,7 @@ def fix_unused_and_missing_imports(
                 logger.error(
                     "%s: couldn't remove import %r", filename, imp,)
             except LineNumberNotFoundError as e:
-                logger.error(
+                logger.debug(
                     "%s: unused import %r on line %d not global",
                     filename, str(imp), e.args[0])
             else: