Implement isort's `reverse_relative` setting (#1826)

This PR implements `reverse-relative`, from isort, but renames it to `relative-imports-order` with the respected value `closest-to-furthest` and `furthest-to-closest`, and the latter being the default. Closes #1813.
2023-01-12 15:48:40 -05:00 · 2023-01-12 15:48:40 -05:00 · 3110d342c7
parent 39aae28eb4
commit 3110d342c7
9 changed files with 205 additions and 34 deletions
--- a/README.md
+++ b/README.md
@ -3022,6 +3022,30 @@ order-by-type = true

 ---

+#### [`relative-imports-order`](#relative-imports-order)
+
+Whether to place "closer" imports (fewer `.` characters, most local)
+before "further" imports (more `.` characters, least local), or vice
+versa.
+
+The default ("furthest-to-closest") is equivalent to isort's
+`reverse-relative` default (`reverse-relative = false`); setting
+this to "closest-to-furthest" is equivalent to isort's `reverse-relative
+= true`.
+
+**Default value**: `furthest-to-closest`
+
+**Type**: `RelatveImportsOrder`
+
+**Example usage**:
+
+```toml
+[tool.ruff.isort]
+relative-imports-order = "closest-to-furthest"
+```
+
+---
+
 #### [`required-imports`](#required-imports)

 Add the specified import line to all files.
--- a/resources/test/fixtures/isort/relative_imports_order.py
+++ b/resources/test/fixtures/isort/relative_imports_order.py
@ -0,0 +1,3 @@
+from ... import a
+from .. import b
+from . import c
--- a/ruff.schema.json
+++ b/ruff.schema.json
@ -820,6 +820,17 @@
            "null"
          ]
        },
+        "relative-imports-order": {
+          "description": "Whether to place \"closer\" imports (fewer `.` characters, most local) before \"further\" imports (more `.` characters, least local), or vice versa.\n\nThe default (\"furthest-to-closest\") is equivalent to isort's `reverse-relative` default (`reverse-relative = false`); setting this to \"closest-to-furthest\" is equivalent to isort's `reverse-relative = true`.",
+          "anyOf": [
+            {
+              "$ref": "#/definitions/RelatveImportsOrder"
+            },
+            {
+              "type": "null"
+            }
+          ]
+        },
        "required-imports": {
          "description": "Add the specified import line to all files.",
          "type": [
@ -1007,6 +1018,24 @@
        }
      ]
    },
+    "RelatveImportsOrder": {
+      "oneOf": [
+        {
+          "description": "Place \"closer\" imports (fewer `.` characters, most local) before \"further\" imports (more `.` characters, least local).",
+          "type": "string",
+          "enum": [
+            "closest-to-further"
+          ]
+        },
+        {
+          "description": "Place \"further\" imports (more `.` characters, least local) imports before \"closer\" imports (fewer `.` characters, most local).",
+          "type": "string",
+          "enum": [
+            "furthest-to-closest"
+          ]
+        }
+      ]
+    },
    "RuleCodePrefix": {
      "type": "string",
      "enum": [
--- a/src/isort/mod.rs
+++ b/src/isort/mod.rs
@ -11,6 +11,7 @@ use rustpython_ast::{Stmt, StmtKind};
 use crate::isort::categorize::{categorize, ImportType};
 use crate::isort::comments::Comment;
 use crate::isort::helpers::trailing_comma;
+use crate::isort::settings::RelatveImportsOrder;
 use crate::isort::sorting::{cmp_either_import, cmp_import_from, cmp_members, cmp_modules};
 use crate::isort::track::{Block, Trailer};
 use crate::isort::types::EitherImport::{Import, ImportFrom};
@ -382,7 +383,11 @@ fn categorize_imports<'a>(
    block_by_type
 }

-fn order_imports(block: ImportBlock, order_by_type: bool) -> OrderedImportBlock {
+fn order_imports(
+    block: ImportBlock,
+    order_by_type: bool,
+    relative_imports_order: RelatveImportsOrder,
+) -> OrderedImportBlock {
    let mut ordered = OrderedImportBlock::default();

    // Sort `StmtKind::Import`.
@ -468,16 +473,16 @@ fn order_imports(block: ImportBlock, order_by_type: bool) -> OrderedImportBlock
            })
            .sorted_by(
                |(import_from1, _, _, aliases1), (import_from2, _, _, aliases2)| {
-                    cmp_import_from(import_from1, import_from2).then_with(|| {
-                        match (aliases1.first(), aliases2.first()) {
+                    cmp_import_from(import_from1, import_from2, relative_imports_order).then_with(
+                        || match (aliases1.first(), aliases2.first()) {
                            (None, None) => Ordering::Equal,
                            (None, Some(_)) => Ordering::Less,
                            (Some(_), None) => Ordering::Greater,
                            (Some((alias1, _)), Some((alias2, _))) => {
                                cmp_members(alias1, alias2, order_by_type)
                            }
-                        }
-                    })
+                        },
+                    )
                },
            ),
    );
@ -540,16 +545,17 @@ pub fn format_imports(
    stylist: &Stylist,
    src: &[PathBuf],
    package: Option<&Path>,
+    combine_as_imports: bool,
+    extra_standard_library: &BTreeSet<String>,
+    force_single_line: bool,
+    force_sort_within_sections: bool,
+    force_wrap_aliases: bool,
    known_first_party: &BTreeSet<String>,
    known_third_party: &BTreeSet<String>,
-    extra_standard_library: &BTreeSet<String>,
-    combine_as_imports: bool,
-    force_wrap_aliases: bool,
-    split_on_trailing_comma: bool,
-    force_single_line: bool,
-    single_line_exclusions: &BTreeSet<String>,
    order_by_type: bool,
-    force_sort_within_sections: bool,
+    relative_imports_order: RelatveImportsOrder,
+    single_line_exclusions: &BTreeSet<String>,
+    split_on_trailing_comma: bool,
 ) -> String {
    let trailer = &block.trailer;
    let block = annotate_imports(&block.imports, comments, locator, split_on_trailing_comma);
@ -572,7 +578,7 @@ pub fn format_imports(
    // Generate replacement source code.
    let mut is_first_block = true;
    for import_block in block_by_type.into_values() {
-        let mut imports = order_imports(import_block, order_by_type);
+        let mut imports = order_imports(import_block, order_by_type, relative_imports_order);

        if force_single_line {
            imports = force_single_line_imports(imports, single_line_exclusions);
@ -586,7 +592,9 @@ pub fn format_imports(
                .chain(imports.import_from.into_iter().map(ImportFrom))
                .collect::<Vec<EitherImport>>();
            if force_sort_within_sections {
-                imports.sort_by(cmp_either_import);
+                imports.sort_by(|import1, import2| {
+                    cmp_either_import(import1, import2, relative_imports_order)
+                });
            };
            imports
        };
@ -647,6 +655,7 @@ mod tests {
    use test_case::test_case;

    use crate::isort;
+    use crate::isort::settings::RelatveImportsOrder;
    use crate::linter::test_path;
    use crate::registry::RuleCode;
    use crate::settings::Settings;
@ -678,6 +687,7 @@ mod tests {
    #[test_case(Path::new("preserve_import_star.py"))]
    #[test_case(Path::new("preserve_indentation.py"))]
    #[test_case(Path::new("reorder_within_section.py"))]
+    #[test_case(Path::new("relative_imports_order.py"))]
    #[test_case(Path::new("separate_first_party_imports.py"))]
    #[test_case(Path::new("separate_future_imports.py"))]
    #[test_case(Path::new("separate_local_folder_imports.py"))]
@ -923,4 +933,24 @@ mod tests {
        insta::assert_yaml_snapshot!(snapshot, diagnostics);
        Ok(())
    }
+
+    #[test_case(Path::new("relative_imports_order.py"))]
+    fn closest_to_furthest(path: &Path) -> Result<()> {
+        let snapshot = format!("closest_to_furthest_{}", path.to_string_lossy());
+        let diagnostics = test_path(
+            Path::new("./resources/test/fixtures/isort")
+                .join(path)
+                .as_path(),
+            &Settings {
+                isort: isort::settings::Settings {
+                    relative_imports_order: RelatveImportsOrder::ClosestToFurther,
+                    ..isort::settings::Settings::default()
+                },
+                src: vec![Path::new("resources/test/fixtures/isort").to_path_buf()],
+                ..Settings::for_rule(RuleCode::I001)
+            },
+        )?;
+        insta::assert_yaml_snapshot!(snapshot, diagnostics);
+        Ok(())
+    }
 }
--- a/src/isort/rules/organize_imports.rs
+++ b/src/isort/rules/organize_imports.rs
@ -73,16 +73,17 @@ pub fn organize_imports(
        stylist,
        &settings.src,
        package,
+        settings.isort.combine_as_imports,
+        &settings.isort.extra_standard_library,
+        settings.isort.force_single_line,
+        settings.isort.force_sort_within_sections,
+        settings.isort.force_wrap_aliases,
        &settings.isort.known_first_party,
        &settings.isort.known_third_party,
-        &settings.isort.extra_standard_library,
-        settings.isort.combine_as_imports,
-        settings.isort.force_wrap_aliases,
-        settings.isort.split_on_trailing_comma,
-        settings.isort.force_single_line,
-        &settings.isort.single_line_exclusions,
        settings.isort.order_by_type,
-        settings.isort.force_sort_within_sections,
+        settings.isort.relative_imports_order,
+        &settings.isort.single_line_exclusions,
+        settings.isort.split_on_trailing_comma,
    );

    // Expand the span the entire range, including leading and trailing space.
--- a/src/isort/settings.rs
+++ b/src/isort/settings.rs
@ -6,6 +6,23 @@ use ruff_macros::ConfigurationOptions;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};

+#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Hash, JsonSchema)]
+#[serde(deny_unknown_fields, rename_all = "kebab-case")]
+pub enum RelatveImportsOrder {
+    /// Place "closer" imports (fewer `.` characters, most local) before
+    /// "further" imports (more `.` characters, least local).
+    ClosestToFurther,
+    /// Place "further" imports (more `.` characters, least local) imports
+    /// before "closer" imports (fewer `.` characters, most local).
+    FurthestToClosest,
+}
+
+impl Default for RelatveImportsOrder {
+    fn default() -> Self {
+        Self::FurthestToClosest
+    }
+}
+
 #[derive(
    Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema,
 )]
@ -129,6 +146,22 @@ pub struct Options {
    /// A list of modules to consider standard-library, in addition to those
    /// known to Ruff in advance.
    pub extra_standard_library: Option<Vec<String>>,
+    #[option(
+        default = r#"furthest-to-closest"#,
+        value_type = "RelatveImportsOrder",
+        example = r#"
+            relative-imports-order = "closest-to-furthest"
+        "#
+    )]
+    /// Whether to place "closer" imports (fewer `.` characters, most local)
+    /// before "further" imports (more `.` characters, least local), or vice
+    /// versa.
+    ///
+    /// The default ("furthest-to-closest") is equivalent to isort's
+    /// `reverse-relative` default (`reverse-relative = false`); setting
+    /// this to "closest-to-furthest" is equivalent to isort's `reverse-relative
+    /// = true`.
+    pub relative_imports_order: Option<RelatveImportsOrder>,
    #[option(
        default = r#"[]"#,
        value_type = "Vec<String>",
@ -152,6 +185,7 @@ pub struct Settings {
    pub known_first_party: BTreeSet<String>,
    pub known_third_party: BTreeSet<String>,
    pub order_by_type: bool,
+    pub relative_imports_order: RelatveImportsOrder,
    pub single_line_exclusions: BTreeSet<String>,
    pub split_on_trailing_comma: bool,
 }
@ -168,6 +202,7 @@ impl Default for Settings {
            known_first_party: BTreeSet::new(),
            known_third_party: BTreeSet::new(),
            order_by_type: true,
+            relative_imports_order: RelatveImportsOrder::default(),
            single_line_exclusions: BTreeSet::new(),
            split_on_trailing_comma: true,
        }
@ -188,6 +223,7 @@ impl From<Options> for Settings {
            known_first_party: BTreeSet::from_iter(options.known_first_party.unwrap_or_default()),
            known_third_party: BTreeSet::from_iter(options.known_third_party.unwrap_or_default()),
            order_by_type: options.order_by_type.unwrap_or(true),
+            relative_imports_order: options.relative_imports_order.unwrap_or_default(),
            single_line_exclusions: BTreeSet::from_iter(
                options.single_line_exclusions.unwrap_or_default(),
            ),
@ -208,6 +244,7 @@ impl From<Settings> for Options {
            known_first_party: Some(settings.known_first_party.into_iter().collect()),
            known_third_party: Some(settings.known_third_party.into_iter().collect()),
            order_by_type: Some(settings.order_by_type),
+            relative_imports_order: Some(settings.relative_imports_order),
            single_line_exclusions: Some(settings.single_line_exclusions.into_iter().collect()),
            split_on_trailing_comma: Some(settings.split_on_trailing_comma),
        }
--- a/src/isort/snapshots/ruffisorttests__closest_to_furthest_relative_imports_order.py.snap
+++ b/src/isort/snapshots/ruffisorttests__closest_to_furthest_relative_imports_order.py.snap
@ -0,0 +1,22 @@
+---
+source: src/isort/mod.rs
+expression: diagnostics
+---
+- kind:
+    UnsortedImports: ~
+  location:
+    row: 1
+    column: 0
+  end_location:
+    row: 4
+    column: 0
+  fix:
+    content: "from . import c\nfrom .. import b\nfrom ... import a\n"
+    location:
+      row: 1
+      column: 0
+    end_location:
+      row: 4
+      column: 0
+  parent: ~
+
--- a/src/isort/snapshots/ruffisorttests__relative_imports_order.py.snap
+++ b/src/isort/snapshots/ruffisorttests__relative_imports_order.py.snap
@ -0,0 +1,6 @@
+---
+source: src/isort/mod.rs
+expression: diagnostics
+---
+[]
+
--- a/src/isort/sorting.rs
+++ b/src/isort/sorting.rs
@ -1,6 +1,7 @@
 /// See: <https://github.com/PyCQA/isort/blob/12cc5fbd67eebf92eb2213b03c07b138ae1fb448/isort/sorting.py#L13>
 use std::cmp::Ordering;

+use crate::isort::settings::RelatveImportsOrder;
 use crate::isort::types::EitherImport::{Import, ImportFrom};
 use crate::isort::types::{AliasData, EitherImport, ImportFromData};
 use crate::python::string;
@ -49,31 +50,49 @@ pub fn cmp_members(alias1: &AliasData, alias2: &AliasData, order_by_type: bool)
 }

 /// Compare two relative import levels.
-pub fn cmp_levels(level1: Option<&usize>, level2: Option<&usize>) -> Ordering {
+pub fn cmp_levels(
+    level1: Option<&usize>,
+    level2: Option<&usize>,
+    relative_imports_order: RelatveImportsOrder,
+) -> Ordering {
    match (level1, level2) {
        (None, None) => Ordering::Equal,
        (None, Some(_)) => Ordering::Less,
        (Some(_), None) => Ordering::Greater,
-        (Some(level1), Some(level2)) => level2.cmp(level1),
+        (Some(level1), Some(level2)) => match relative_imports_order {
+            RelatveImportsOrder::ClosestToFurther => level1.cmp(level2),
+            RelatveImportsOrder::FurthestToClosest => level2.cmp(level1),
+        },
    }
 }

 /// Compare two `StmtKind::ImportFrom` blocks.
-pub fn cmp_import_from(import_from1: &ImportFromData, import_from2: &ImportFromData) -> Ordering {
-    cmp_levels(import_from1.level, import_from2.level).then_with(|| {
-        match (&import_from1.module, import_from2.module) {
-            (None, None) => Ordering::Equal,
-            (None, Some(_)) => Ordering::Less,
-            (Some(_), None) => Ordering::Greater,
-            (Some(module1), Some(module2)) => natord::compare_ignore_case(module1, module2)
-                .then_with(|| natord::compare(module1, module2)),
-        }
+pub fn cmp_import_from(
+    import_from1: &ImportFromData,
+    import_from2: &ImportFromData,
+    relative_imports_order: RelatveImportsOrder,
+) -> Ordering {
+    cmp_levels(
+        import_from1.level,
+        import_from2.level,
+        relative_imports_order,
+    )
+    .then_with(|| match (&import_from1.module, import_from2.module) {
+        (None, None) => Ordering::Equal,
+        (None, Some(_)) => Ordering::Less,
+        (Some(_), None) => Ordering::Greater,
+        (Some(module1), Some(module2)) => natord::compare_ignore_case(module1, module2)
+            .then_with(|| natord::compare(module1, module2)),
    })
 }

 /// Compare two `EitherImport` enums which may be `Import` or `ImportFrom`
 /// structs.
-pub fn cmp_either_import(a: &EitherImport, b: &EitherImport) -> Ordering {
+pub fn cmp_either_import(
+    a: &EitherImport,
+    b: &EitherImport,
+    relative_imports_order: RelatveImportsOrder,
+) -> Ordering {
    match (a, b) {
        (Import((alias1, _)), Import((alias2, _))) => cmp_modules(alias1, alias2),
        (ImportFrom((import_from, ..)), Import((alias, _))) => {
@ -83,7 +102,7 @@ pub fn cmp_either_import(a: &EitherImport, b: &EitherImport) -> Ordering {
            natord::compare_ignore_case(alias.name, import_from.module.unwrap_or_default())
        }
        (ImportFrom((import_from1, ..)), ImportFrom((import_from2, ..))) => {
-            cmp_import_from(import_from1, import_from2)
+            cmp_import_from(import_from1, import_from2, relative_imports_order)
        }
    }
 }