Implement F405 (#243)

README.md

@@ -151,14 +151,14 @@ ruff's goal is to achieve feature-parity with Flake8 when used (1) without any p
 stylistic checks; limiting to Python 3 obviates the need for certain compatibility checks.)
 
 Under those conditions, Flake8 implements about 60 rules, give or take. At time of writing, ruff
-implements 43 rules. (Note that these 43 rules likely cover a disproportionate share of errors:
+implements 44 rules. (Note that these 44 rules likely cover a disproportionate share of errors:
 unused imports, undefined variables, etc.)
 
 The unimplemented rules are tracked in #170, and include:
 
 - 14 rules related to string `.format` calls.
-- 4 logical rules.
+- 1 logical rule.
-- 1 rule related to parsing.
+- 1 rule related to docstring parsing.
 
 Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
 
@@ -182,12 +182,13 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
 | E741 | AmbiguousVariableName | ambiguous variable name '...' |
 | E742 | AmbiguousClassName | ambiguous class name '...' |
 | E743 | AmbiguousFunctionName | ambiguous function name '...' |
-| E902 | IOError | No such file or directory: `...` |
+| E902 | IOError | ... |
 | E999 | SyntaxError | SyntaxError: ... |
 | F401 | UnusedImport | `...` imported but unused |
 | F402 | ImportShadowedByLoopVar | import '...' from line 1 shadowed by loop variable |
-| F403 | ImportStarUsage | `from ... import *` used; unable to detect undefined names |
+| F403 | ImportStarUsed | `from ... import *` used; unable to detect undefined names |
 | F404 | LateFutureImport | from __future__ imports must occur at the beginning of the file |
+| F405 | ImportStarUsage | '...' may be undefined, or defined from star imports: ... |
 | F406 | ImportStarNotPermitted | `from ... import *` only allowed at module level |
 | F407 | FutureFeatureNotDefined | future feature '...' is not defined |
 | F541 | FStringMissingPlaceholders | f-string without any placeholders |

resources/test/fixtures/F405.py

@@ -0,0 +1,11 @@
+from mymodule import *
+
+
+def print_name():
+    print(name)
+
+
+def print_name(name):
+    print(name)
+
+__all__ = ['a']

src/ast/types.rs

@@ -25,6 +25,7 @@ pub enum ScopeKind {
 pub struct Scope {
     pub id: usize,
     pub kind: ScopeKind,
+    pub import_starred: bool,
     pub values: BTreeMap<String, Binding>,
 }
 
@@ -33,6 +34,7 @@ impl Scope {
         Scope {
             id: id(),
             kind,
+            import_starred: false,
             values: BTreeMap::new(),
         }
     }

src/check_ast.rs

@@ -391,7 +391,11 @@ where
                     }
                 }
             }
-            StmtKind::ImportFrom { names, module, .. } => {
+            StmtKind::ImportFrom {
+                names,
+                module,
+                level,
+            } => {
                 if self
                     .settings
                     .select
@@ -447,8 +451,14 @@ where
                                 .push(Check::new(CheckKind::LateFutureImport, stmt.location));
                         }
                     } else if alias.node.name == "*" {
+                        let module_name = format!(
+                            "{}{}",
+                            ".".repeat(level.unwrap_or_default()),
+                            module.clone().unwrap_or_else(|| "module".to_string()),
+                        );
+
                         self.add_binding(
-                            name,
+                            module_name.to_string(),
                             Binding {
                                 kind: BindingKind::StarImportation,
                                 used: None,
@@ -456,27 +466,29 @@ where
                             },
                         );
 
-                        if self.settings.select.contains(&CheckCode::F403) {
-                            self.checks.push(Check::new(
-                                CheckKind::ImportStarUsage(
-                                    module.clone().unwrap_or_else(|| "module".to_string()),
-                                ),
-                                stmt.location,
-                            ));
-                        }
-
                         if self.settings.select.contains(&CheckCode::F406) {
                             let scope = &self.scopes
                                 [*(self.scope_stack.last().expect("No current scope found."))];
                             if !matches!(scope.kind, ScopeKind::Module) {
                                 self.checks.push(Check::new(
-                                    CheckKind::ImportStarNotPermitted(
+                                    CheckKind::ImportStarNotPermitted(module_name.to_string()),
-                                        module.clone().unwrap_or_else(|| "module".to_string()),
-                                    ),
                                     stmt.location,
                                 ));
                             }
                         }
+
+                        if self.settings.select.contains(&CheckCode::F403) {
+                            self.checks.push(Check::new(
+                                CheckKind::ImportStarUsed(module_name.to_string()),
+                                stmt.location,
+                            ));
+                        }
+
+                        let scope = &mut self.scopes[*(self
+                            .scope_stack
+                            .last_mut()
+                            .expect("No current scope found."))];
+                        scope.import_starred = true;
                     } else {
                         let binding = Binding {
                             kind: BindingKind::Importation(match module {
@@ -1138,6 +1150,7 @@ impl<'a> Checker<'a> {
 
             let mut first_iter = true;
             let mut in_generator = false;
+            let mut import_starred = false;
             for scope_index in self.scope_stack.iter().rev() {
                 let scope = &mut self.scopes[*scope_index];
                 if matches!(scope.kind, ScopeKind::Class) {
@@ -1154,6 +1167,28 @@ impl<'a> Checker<'a> {
 
                 first_iter = false;
                 in_generator = matches!(scope.kind, ScopeKind::Generator);
+                import_starred = import_starred || scope.import_starred;
+            }
+
+            if import_starred {
+                if self.settings.select.contains(&CheckCode::F405) {
+                    let mut from_list = vec![];
+                    for scope_index in self.scope_stack.iter().rev() {
+                        let scope = &self.scopes[*scope_index];
+                        for (name, binding) in scope.values.iter() {
+                            if matches!(binding.kind, BindingKind::StarImportation) {
+                                from_list.push(name.as_str());
+                            }
+                        }
+                    }
+                    from_list.sort();
+
+                    self.checks.push(Check::new(
+                        CheckKind::ImportStarUsage(id.clone(), from_list.join(", ")),
+                        expr.location,
+                    ));
+                }
+                return;
             }
 
             if self.settings.select.contains(&CheckCode::F821) {
@@ -1364,8 +1399,9 @@ impl<'a> Checker<'a> {
     }
 
     fn check_dead_scopes(&mut self) {
-        if !self.settings.select.contains(&CheckCode::F822)
+        if !self.settings.select.contains(&CheckCode::F401)
-            && !self.settings.select.contains(&CheckCode::F401)
+            && !self.settings.select.contains(&CheckCode::F405)
+            && !self.settings.select.contains(&CheckCode::F822)
         {
             return;
         }
@@ -1380,15 +1416,39 @@ impl<'a> Checker<'a> {
             });
 
             if self.settings.select.contains(&CheckCode::F822)
+                && !scope.import_starred
                 && !self.path.ends_with("__init__.py")
             {
-                if let Some(binding) = all_binding {
+                if let Some(all_binding) = all_binding {
                     if let Some(names) = all_names {
                         for name in names {
                             if !scope.values.contains_key(name) {
                                 self.checks.push(Check::new(
                                     CheckKind::UndefinedExport(name.to_string()),
-                                    binding.location,
+                                    all_binding.location,
+                                ));
+                            }
+                        }
+                    }
+                }
+            }
+
+            if self.settings.select.contains(&CheckCode::F405) && scope.import_starred {
+                if let Some(all_binding) = all_binding {
+                    if let Some(names) = all_names {
+                        let mut from_list = vec![];
+                        for (name, binding) in scope.values.iter() {
+                            if matches!(binding.kind, BindingKind::StarImportation) {
+                                from_list.push(name.as_str());
+                            }
+                        }
+                        from_list.sort();
+
+                        for name in names {
+                            if !scope.values.contains_key(name) {
+                                self.checks.push(Check::new(
+                                    CheckKind::ImportStarUsage(name.clone(), from_list.join(", ")),
+                                    all_binding.location,
                                 ));
                             }
                         }

src/checks.rs

@@ -6,7 +6,7 @@ use regex::Regex;
 use rustpython_parser::ast::Location;
 use serde::{Deserialize, Serialize};
 
-pub const ALL_CHECK_CODES: [CheckCode; 43] = [
+pub const ALL_CHECK_CODES: [CheckCode; 44] = [
     CheckCode::E402,
     CheckCode::E501,
     CheckCode::E711,
@@ -25,6 +25,7 @@ pub const ALL_CHECK_CODES: [CheckCode; 43] = [
     CheckCode::F402,
     CheckCode::F403,
     CheckCode::F404,
+    CheckCode::F405,
     CheckCode::F406,
     CheckCode::F407,
     CheckCode::F541,
@@ -72,6 +73,7 @@ pub enum CheckCode {
     F402,
     F403,
     F404,
+    F405,
     F406,
     F407,
     F541,
@@ -122,6 +124,7 @@ impl FromStr for CheckCode {
             "F402" => Ok(CheckCode::F402),
             "F403" => Ok(CheckCode::F403),
             "F404" => Ok(CheckCode::F404),
+            "F405" => Ok(CheckCode::F405),
             "F406" => Ok(CheckCode::F406),
             "F407" => Ok(CheckCode::F407),
             "F541" => Ok(CheckCode::F541),
@@ -173,6 +176,7 @@ impl CheckCode {
             CheckCode::F402 => "F402",
             CheckCode::F403 => "F403",
             CheckCode::F404 => "F404",
+            CheckCode::F405 => "F405",
             CheckCode::F406 => "F406",
             CheckCode::F407 => "F407",
             CheckCode::F541 => "F541",
@@ -213,49 +217,50 @@ impl CheckCode {
     /// A placeholder representation of the CheckKind for the check.
     pub fn kind(&self) -> CheckKind {
         match self {
-            CheckCode::E742 => CheckKind::AmbiguousClassName("...".to_string()),
-            CheckCode::E743 => CheckKind::AmbiguousFunctionName("...".to_string()),
-            CheckCode::E741 => CheckKind::AmbiguousVariableName("...".to_string()),
-            CheckCode::F631 => CheckKind::AssertTuple,
-            CheckCode::F701 => CheckKind::BreakOutsideLoop,
-            CheckCode::F702 => CheckKind::ContinueOutsideLoop,
-            CheckCode::F707 => CheckKind::DefaultExceptNotLast,
-            CheckCode::E731 => CheckKind::DoNotAssignLambda,
-            CheckCode::E722 => CheckKind::DoNotUseBareExcept,
-            CheckCode::F831 => CheckKind::DuplicateArgumentName,
-            CheckCode::F541 => CheckKind::FStringMissingPlaceholders,
-            CheckCode::F722 => CheckKind::ForwardAnnotationSyntaxError("...".to_string()),
-            CheckCode::F407 => CheckKind::FutureFeatureNotDefined("...".to_string()),
-            CheckCode::E902 => CheckKind::IOError("...".to_string()),
-            CheckCode::F634 => CheckKind::IfTuple,
-            CheckCode::F402 => CheckKind::ImportShadowedByLoopVar("...".to_string(), 1),
-            CheckCode::F406 => CheckKind::ImportStarNotPermitted("...".to_string()),
-            CheckCode::F403 => CheckKind::ImportStarUsage("...".to_string()),
-            CheckCode::F633 => CheckKind::InvalidPrintSyntax,
-            CheckCode::F632 => CheckKind::IsLiteral,
-            CheckCode::F404 => CheckKind::LateFutureImport,
-            CheckCode::E501 => CheckKind::LineTooLong(89, 88),
             CheckCode::E402 => CheckKind::ModuleImportNotAtTopOfFile,
-            CheckCode::F601 => CheckKind::MultiValueRepeatedKeyLiteral,
+            CheckCode::E501 => CheckKind::LineTooLong(89, 88),
-            CheckCode::F602 => CheckKind::MultiValueRepeatedKeyVariable("...".to_string()),
-            CheckCode::R002 => CheckKind::NoAssertEquals,
             CheckCode::E711 => CheckKind::NoneComparison(RejectedCmpop::Eq),
+            CheckCode::E712 => CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq),
             CheckCode::E713 => CheckKind::NotInTest,
             CheckCode::E714 => CheckKind::NotIsTest,
-            CheckCode::F901 => CheckKind::RaiseNotImplemented,
-            CheckCode::F706 => CheckKind::ReturnOutsideFunction,
-            CheckCode::E999 => CheckKind::SyntaxError("...".to_string()),
-            CheckCode::F621 => CheckKind::TooManyExpressionsInStarredAssignment,
-            CheckCode::E712 => CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq),
-            CheckCode::F622 => CheckKind::TwoStarredExpressions,
             CheckCode::E721 => CheckKind::TypeComparison,
+            CheckCode::E722 => CheckKind::DoNotUseBareExcept,
+            CheckCode::E731 => CheckKind::DoNotAssignLambda,
+            CheckCode::E741 => CheckKind::AmbiguousVariableName("...".to_string()),
+            CheckCode::E742 => CheckKind::AmbiguousClassName("...".to_string()),
+            CheckCode::E743 => CheckKind::AmbiguousFunctionName("...".to_string()),
+            CheckCode::E902 => CheckKind::IOError("...".to_string()),
+            CheckCode::E999 => CheckKind::SyntaxError("...".to_string()),
+            CheckCode::F401 => CheckKind::UnusedImport("...".to_string()),
+            CheckCode::F402 => CheckKind::ImportShadowedByLoopVar("...".to_string(), 1),
+            CheckCode::F403 => CheckKind::ImportStarUsed("...".to_string()),
+            CheckCode::F404 => CheckKind::LateFutureImport,
+            CheckCode::F405 => CheckKind::ImportStarUsage("...".to_string(), "...".to_string()),
+            CheckCode::F406 => CheckKind::ImportStarNotPermitted("...".to_string()),
+            CheckCode::F407 => CheckKind::FutureFeatureNotDefined("...".to_string()),
+            CheckCode::F541 => CheckKind::FStringMissingPlaceholders,
+            CheckCode::F601 => CheckKind::MultiValueRepeatedKeyLiteral,
+            CheckCode::F602 => CheckKind::MultiValueRepeatedKeyVariable("...".to_string()),
+            CheckCode::F621 => CheckKind::TooManyExpressionsInStarredAssignment,
+            CheckCode::F622 => CheckKind::TwoStarredExpressions,
+            CheckCode::F631 => CheckKind::AssertTuple,
+            CheckCode::F632 => CheckKind::IsLiteral,
+            CheckCode::F633 => CheckKind::InvalidPrintSyntax,
+            CheckCode::F634 => CheckKind::IfTuple,
+            CheckCode::F701 => CheckKind::BreakOutsideLoop,
+            CheckCode::F702 => CheckKind::ContinueOutsideLoop,
+            CheckCode::F704 => CheckKind::YieldOutsideFunction,
+            CheckCode::F706 => CheckKind::ReturnOutsideFunction,
+            CheckCode::F707 => CheckKind::DefaultExceptNotLast,
+            CheckCode::F722 => CheckKind::ForwardAnnotationSyntaxError("...".to_string()),
+            CheckCode::F821 => CheckKind::UndefinedName("...".to_string()),
             CheckCode::F822 => CheckKind::UndefinedExport("...".to_string()),
             CheckCode::F823 => CheckKind::UndefinedLocal("...".to_string()),
-            CheckCode::F821 => CheckKind::UndefinedName("...".to_string()),
+            CheckCode::F831 => CheckKind::DuplicateArgumentName,
-            CheckCode::F401 => CheckKind::UnusedImport("...".to_string()),
             CheckCode::F841 => CheckKind::UnusedVariable("...".to_string()),
+            CheckCode::F901 => CheckKind::RaiseNotImplemented,
             CheckCode::R001 => CheckKind::UselessObjectInheritance("...".to_string()),
-            CheckCode::F704 => CheckKind::YieldOutsideFunction,
+            CheckCode::R002 => CheckKind::NoAssertEquals,
         }
     }
 }
@@ -292,7 +297,8 @@ pub enum CheckKind {
     IfTuple,
     ImportShadowedByLoopVar(String, usize),
     ImportStarNotPermitted(String),
-    ImportStarUsage(String),
+    ImportStarUsage(String, String),
+    ImportStarUsed(String),
     InvalidPrintSyntax,
     IsLiteral,
     LateFutureImport,
@@ -341,7 +347,8 @@ impl CheckKind {
             CheckKind::IfTuple => "IfTuple",
             CheckKind::ImportShadowedByLoopVar(_, _) => "ImportShadowedByLoopVar",
             CheckKind::ImportStarNotPermitted(_) => "ImportStarNotPermitted",
-            CheckKind::ImportStarUsage(_) => "ImportStarUsage",
+            CheckKind::ImportStarUsage(_, _) => "ImportStarUsage",
+            CheckKind::ImportStarUsed(_) => "ImportStarUsed",
             CheckKind::InvalidPrintSyntax => "InvalidPrintSyntax",
             CheckKind::IsLiteral => "IsLiteral",
             CheckKind::LateFutureImport => "LateFutureImport",
@@ -392,7 +399,8 @@ impl CheckKind {
             CheckKind::IfTuple => &CheckCode::F634,
             CheckKind::ImportShadowedByLoopVar(_, _) => &CheckCode::F402,
             CheckKind::ImportStarNotPermitted(_) => &CheckCode::F406,
-            CheckKind::ImportStarUsage(_) => &CheckCode::F403,
+            CheckKind::ImportStarUsed(_) => &CheckCode::F403,
+            CheckKind::ImportStarUsage(_, _) => &CheckCode::F405,
             CheckKind::InvalidPrintSyntax => &CheckCode::F633,
             CheckKind::IsLiteral => &CheckCode::F632,
             CheckKind::LateFutureImport => &CheckCode::F404,
@@ -466,9 +474,12 @@ impl CheckKind {
             CheckKind::ImportStarNotPermitted(name) => {
                 format!("`from {name} import *` only allowed at module level")
             }
-            CheckKind::ImportStarUsage(name) => {
+            CheckKind::ImportStarUsed(name) => {
                 format!("`from {name} import *` used; unable to detect undefined names")
             }
+            CheckKind::ImportStarUsage(name, sources) => {
+                format!("'{name}' may be undefined, or defined from star imports: {sources}")
+            }
             CheckKind::IsLiteral => "use ==/!= to compare constant literals".to_string(),
             CheckKind::LateFutureImport => {
                 "from __future__ imports must occur at the beginning of the file".to_string()

src/linter.rs

@@ -383,6 +383,23 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn f405() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/F405.py"),
+            &settings::Settings {
+                line_length: 88,
+                exclude: vec![],
+                extend_exclude: vec![],
+                select: BTreeSet::from([CheckCode::F405]),
+            },
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     #[test]
     fn f406() -> Result<()> {
         let mut checks = check_path(

src/snapshots/ruff__linter__tests__f405.snap

@@ -0,0 +1,21 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    ImportStarUsed:
+      - name
+      - mymodule
+  location:
+    row: 5
+    column: 11
+  fix: ~
+- kind:
+    ImportStarUsed:
+      - a
+      - mymodule
+  location:
+    row: 11
+    column: 1
+  fix: ~
+