Implement F404 (#159)

README.md

@@ -124,7 +124,7 @@ 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 58 rules, give or take. At time of writing, ruff
-implements 28 rules. (Note that these 28 rules likely cover a disproportionate share of errors:
+implements 30 rules. (Note that these 30 rules likely cover a disproportionate share of errors:
 unused imports, undefined variables, etc.)
 
 Of the unimplemented rules, ruff is missing:
@@ -157,6 +157,7 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
 | E902 | IOError | No such file or directory: `...` |
 | F401 | UnusedImport | `...` imported but unused |
 | F403 | ImportStarUsage | Unable to detect undefined names |
+| F404 | LateFutureImport | from __future__ imports must occur at the beginning of the file |
 | F541 | FStringMissingPlaceholders | f-string without any placeholders |
 | F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal repeated |
 | F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated |

examples/generate_rules_table.rs

@@ -12,6 +12,7 @@ fn main() {
         CheckKind::IOError("...".to_string()),
         CheckKind::IfTuple,
         CheckKind::ImportStarUsage,
+        CheckKind::LateFutureImport,
         CheckKind::LineTooLong,
         CheckKind::ModuleImportNotAtTopOfFile,
         CheckKind::MultiValueRepeatedKeyLiteral,

resources/test/fixtures/F404.py

@@ -0,0 +1,7 @@
+from __future__ import print_function
+"""Docstring"""
+from __future__ import absolute_import
+
+from collections import namedtuple
+
+from __future__ import print_function

resources/test/fixtures/pyproject.toml

@@ -13,6 +13,7 @@ select = [
     "E902",
     "F401",
     "F403",
+    "F404",
     "F541",
     "F601",
     "F602",

src/check_ast.rs

@@ -44,6 +44,7 @@ struct Checker<'a> {
     in_literal: bool,
     seen_non_import: bool,
     seen_docstring: bool,
+    futures_allowed: bool,
 }
 
 impl<'a> Checker<'a> {
@@ -73,6 +74,7 @@ impl<'a> Checker<'a> {
             in_literal: false,
             seen_non_import: false,
             seen_docstring: false,
+            futures_allowed: true,
         }
     }
 }
@@ -102,32 +104,57 @@ where
 
         // Track whether we've seen docstrings, non-imports, etc.
         match &stmt.node {
-            StmtKind::Import { .. } => {}
+            StmtKind::ImportFrom { module, .. } => {
-            StmtKind::ImportFrom { .. } => {}
+                // Allow __future__ imports until we see a non-__future__ import.
-            StmtKind::Expr { value } => {
+                if self.futures_allowed {
-                if !self.seen_docstring
+                    if let Some(module) = module {
-                    && stmt.location.column() == 1
+                        if module != "__future__" {
-                    && !operations::in_nested_block(&self.parent_stack, &self.parents)
+                            self.futures_allowed = false;
-                {
+                        }
-                    if let ExprKind::Constant {
-                        value: Constant::Str(_),
-                        ..
-                    } = &value.node
-                    {
-                        self.seen_docstring = true;
                     }
                 }
-
+            }
-                if !self.seen_non_import
+            StmtKind::Import { .. } => {
-                    && stmt.location.column() == 1
+                self.futures_allowed = false;
+            }
+            StmtKind::Expr { value } => {
+                if self.seen_docstring
+                    && !self.seen_non_import
                     && !operations::in_nested_block(&self.parent_stack, &self.parents)
                 {
                     self.seen_non_import = true;
                 }
+
+                if !self.seen_docstring
+                    && !operations::in_nested_block(&self.parent_stack, &self.parents)
+                    && matches!(
+                        &value.node,
+                        ExprKind::Constant {
+                            value: Constant::Str(_),
+                            ..
+                        },
+                    )
+                {
+                    self.seen_docstring = true;
+                }
+
+                // Allow docstrings to interrupt __future__ imports.
+                if self.futures_allowed
+                    && !matches!(
+                        &value.node,
+                        ExprKind::Constant {
+                            value: Constant::Str(_),
+                            ..
+                        },
+                    )
+                {
+                    self.futures_allowed = false;
+                }
             }
             _ => {
+                self.futures_allowed = false;
+
                 if !self.seen_non_import
-                    && stmt.location.column() == 1
                     && !operations::in_nested_block(&self.parent_stack, &self.parents)
                 {
                     self.seen_non_import = true;
@@ -363,6 +390,12 @@ where
                                 location: stmt.location,
                             },
                         );
+
+                        if !self.futures_allowed && self.settings.select.contains(&CheckCode::F404)
+                        {
+                            self.checks
+                                .push(Check::new(CheckKind::LateFutureImport, stmt.location));
+                        }
                     } else if alias.node.name == "*" {
                         self.add_binding(
                             name,

src/checks.rs

@@ -19,6 +19,7 @@ pub enum CheckCode {
     E902,
     F401,
     F403,
+    F404,
     F541,
     F601,
     F602,
@@ -55,6 +56,7 @@ impl FromStr for CheckCode {
             "E902" => Ok(CheckCode::E902),
             "F401" => Ok(CheckCode::F401),
             "F403" => Ok(CheckCode::F403),
+            "F404" => Ok(CheckCode::F404),
             "F541" => Ok(CheckCode::F541),
             "F601" => Ok(CheckCode::F601),
             "F602" => Ok(CheckCode::F602),
@@ -92,6 +94,7 @@ impl CheckCode {
             CheckCode::E902 => "E902",
             CheckCode::F401 => "F401",
             CheckCode::F403 => "F403",
+            CheckCode::F404 => "F404",
             CheckCode::F541 => "F541",
             CheckCode::F601 => "F601",
             CheckCode::F602 => "F602",
@@ -127,6 +130,7 @@ impl CheckCode {
             CheckCode::E902 => &LintSource::FileSystem,
             CheckCode::F401 => &LintSource::AST,
             CheckCode::F403 => &LintSource::AST,
+            CheckCode::F404 => &LintSource::AST,
             CheckCode::F541 => &LintSource::AST,
             CheckCode::F601 => &LintSource::AST,
             CheckCode::F602 => &LintSource::AST,
@@ -173,6 +177,7 @@ pub enum CheckKind {
     IOError(String),
     IfTuple,
     ImportStarUsage,
+    LateFutureImport,
     LineTooLong,
     ModuleImportNotAtTopOfFile,
     MultiValueRepeatedKeyLiteral,
@@ -207,6 +212,7 @@ impl CheckKind {
             CheckKind::IOError(_) => "IOError",
             CheckKind::IfTuple => "IfTuple",
             CheckKind::ImportStarUsage => "ImportStarUsage",
+            CheckKind::LateFutureImport => "LateFutureImport",
             CheckKind::LineTooLong => "LineTooLong",
             CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
             CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
@@ -243,6 +249,7 @@ impl CheckKind {
             CheckKind::IOError(_) => &CheckCode::E902,
             CheckKind::IfTuple => &CheckCode::F634,
             CheckKind::ImportStarUsage => &CheckCode::F403,
+            CheckKind::LateFutureImport => &CheckCode::F404,
             CheckKind::LineTooLong => &CheckCode::E501,
             CheckKind::DoNotAssignLambda => &CheckCode::E731,
             CheckKind::AmbiguousVariableName(_) => &CheckCode::E741,
@@ -288,6 +295,9 @@ impl CheckKind {
             }
             CheckKind::IfTuple => "If test is a tuple, which is always `True`".to_string(),
             CheckKind::ImportStarUsage => "Unable to detect undefined names".to_string(),
+            CheckKind::LateFutureImport => {
+                "from __future__ imports must occur at the beginning of the file".to_string()
+            }
             CheckKind::LineTooLong => "Line too long".to_string(),
             CheckKind::DoNotAssignLambda => {
                 "Do not assign a lambda expression, use a def".to_string()
@@ -368,23 +378,24 @@ impl CheckKind {
     /// Whether the check kind is (potentially) fixable.
     pub fn fixable(&self) -> bool {
         match self {
+            CheckKind::AmbiguousVariableName(_) => false,
             CheckKind::AssertTuple => false,
             CheckKind::DefaultExceptNotLast => false,
+            CheckKind::DoNotAssignLambda => false,
             CheckKind::DuplicateArgumentName => false,
             CheckKind::FStringMissingPlaceholders => false,
             CheckKind::IOError(_) => false,
             CheckKind::IfTuple => false,
             CheckKind::ImportStarUsage => false,
-            CheckKind::DoNotAssignLambda => false,
+            CheckKind::LateFutureImport => false,
-            CheckKind::AmbiguousVariableName(_) => false,
             CheckKind::LineTooLong => false,
             CheckKind::ModuleImportNotAtTopOfFile => false,
             CheckKind::MultiValueRepeatedKeyLiteral => false,
             CheckKind::MultiValueRepeatedKeyVariable(_) => false,
             CheckKind::NoAssertEquals => true,
+            CheckKind::NoneComparison(_) => false,
             CheckKind::NotInTest => false,
             CheckKind::NotIsTest => false,
-            CheckKind::NoneComparison(_) => false,
             CheckKind::RaiseNotImplemented => false,
             CheckKind::ReturnOutsideFunction => false,
             CheckKind::TooManyExpressionsInStarredAssignment => false,

src/linter.rs

@@ -509,6 +509,32 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn f404() -> Result<()> {
+        let mut actual = check_path(
+            Path::new("./resources/test/fixtures/F404.py"),
+            &settings::Settings {
+                line_length: 88,
+                exclude: vec![],
+                select: BTreeSet::from([CheckCode::F404]),
+            },
+            &fixer::Mode::Generate,
+        )?;
+        actual.sort_by_key(|check| check.location);
+        let expected = vec![Check {
+            kind: CheckKind::LateFutureImport,
+            location: Location::new(7, 1),
+            fix: None,
+        }];
+        assert_eq!(actual.len(), expected.len());
+        for i in 0..actual.len() {
+            assert_eq!(actual[i], expected[i]);
+        }
+
+        Ok(())
+    }
+
     #[test]
     fn f541() -> Result<()> {
         let mut actual = check_path(

src/pyproject.rs

@@ -270,6 +270,7 @@ other-attribute = 1
                     CheckCode::E902,
                     CheckCode::F401,
                     CheckCode::F403,
+                    CheckCode::F404,
                     CheckCode::F541,
                     CheckCode::F601,
                     CheckCode::F602,