Skip to content

Commit

Permalink
Add fix for future-required-type-annotation (#8711)
Browse files Browse the repository at this point in the history
## Summary

We already support inserting imports for `I002` -- this PR just adds the
same fix for `FA102`, which is explicitly about `from __future__ import
annotations`.

Closes #8682.
  • Loading branch information
charliermarsh authored Nov 16, 2023
1 parent cd29761 commit a591725
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 26 deletions.
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/importer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::error::Error;

use anyhow::Result;
use libcst_native::{ImportAlias, Name, NameOrAttribute};
use ruff_python_ast::{self as ast, PySourceType, Stmt, Suite};
use ruff_python_ast::{self as ast, PySourceType, Stmt};
use ruff_text_size::{Ranged, TextSize};

use ruff_diagnostics::Edit;
Expand All @@ -26,7 +26,7 @@ mod insertion;

pub(crate) struct Importer<'a> {
/// The Python AST to which we are adding imports.
python_ast: &'a Suite,
python_ast: &'a [Stmt],
/// The [`Locator`] for the Python AST.
locator: &'a Locator<'a>,
/// The [`Stylist`] for the Python AST.
Expand All @@ -39,7 +39,7 @@ pub(crate) struct Importer<'a> {

impl<'a> Importer<'a> {
pub(crate) fn new(
python_ast: &'a Suite,
python_ast: &'a [Stmt],
locator: &'a Locator<'a>,
stylist: &'a Stylist<'a>,
) -> Self {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use ruff_python_ast::Expr;
use std::fmt;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;
use ruff_python_ast::imports::{AnyImport, ImportFrom};
use ruff_python_ast::Expr;
use ruff_text_size::{Ranged, TextSize};

use crate::checkers::ast::Checker;
use crate::importer::Importer;

/// ## What it does
/// Checks for uses of PEP 585- and PEP 604-style type annotations in Python
Expand Down Expand Up @@ -42,6 +44,10 @@ use crate::checkers::ast::Checker;
/// ...
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as adding `from __future__ import annotations`
/// may change the semantics of the program.
///
/// ## Options
/// - `target-version`
#[violation]
Expand All @@ -66,18 +72,28 @@ impl fmt::Display for Reason {
}
}

impl Violation for FutureRequiredTypeAnnotation {
impl AlwaysFixableViolation for FutureRequiredTypeAnnotation {
#[derive_message_formats]
fn message(&self) -> String {
let FutureRequiredTypeAnnotation { reason } = self;
format!("Missing `from __future__ import annotations`, but uses {reason}")
}

fn fix_title(&self) -> String {
format!("Add `from __future__ import annotations`")
}
}

/// FA102
pub(crate) fn future_required_type_annotation(checker: &mut Checker, expr: &Expr, reason: Reason) {
checker.diagnostics.push(Diagnostic::new(
FutureRequiredTypeAnnotation { reason },
expr.range(),
));
let mut diagnostic = Diagnostic::new(FutureRequiredTypeAnnotation { reason }, expr.range());
if let Some(python_ast) = checker.semantic().definitions.python_ast() {
let required_import =
AnyImport::ImportFrom(ImportFrom::member("__future__", "annotations"));
diagnostic.set_fix(Fix::unsafe_edit(
Importer::new(python_ast, checker.locator(), checker.stylist())
.add_import(&required_import, TextSize::default()),
));
}
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
---
source: crates/ruff_linter/src/rules/flake8_future_annotations/mod.rs
---
no_future_import_uses_lowercase.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
no_future_import_uses_lowercase.py:2:13: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
1 | def main() -> None:
2 | a_list: list[str] = []
| ^^^^^^^^^ FA102
3 | a_list.append("hello")
|
= help: Add `from __future__ import annotations`

no_future_import_uses_lowercase.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] = []
3 4 | a_list.append("hello")

no_future_import_uses_lowercase.py:6:14: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
6 | def hello(y: dict[str, int]) -> None:
| ^^^^^^^^^^^^^^ FA102
7 | del y
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] = []
3 4 | a_list.append("hello")


Original file line number Diff line number Diff line change
@@ -1,34 +1,62 @@
---
source: crates/ruff_linter/src/rules/flake8_future_annotations/mod.rs
---
no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
no_future_import_uses_union.py:2:13: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
1 | def main() -> None:
2 | a_list: list[str] | None = []
| ^^^^^^^^^ FA102
3 | a_list.append("hello")
|
= help: Add `from __future__ import annotations`

no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] | None = []
3 4 | a_list.append("hello")

no_future_import_uses_union.py:2:13: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 604 union
|
1 | def main() -> None:
2 | a_list: list[str] | None = []
| ^^^^^^^^^^^^^^^^ FA102
3 | a_list.append("hello")
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] | None = []
3 4 | a_list.append("hello")

no_future_import_uses_union.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
no_future_import_uses_union.py:6:14: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
6 | def hello(y: dict[str, int] | None) -> None:
| ^^^^^^^^^^^^^^ FA102
7 | del y
|
= help: Add `from __future__ import annotations`

no_future_import_uses_union.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] | None = []
3 4 | a_list.append("hello")

no_future_import_uses_union.py:6:14: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 604 union
|
6 | def hello(y: dict[str, int] | None) -> None:
| ^^^^^^^^^^^^^^^^^^^^^ FA102
7 | del y
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str] | None = []
3 4 | a_list.append("hello")


Original file line number Diff line number Diff line change
@@ -1,52 +1,94 @@
---
source: crates/ruff_linter/src/rules/flake8_future_annotations/mod.rs
---
no_future_import_uses_union_inner.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
no_future_import_uses_union_inner.py:2:13: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
1 | def main() -> None:
2 | a_list: list[str | None] = []
| ^^^^^^^^^^^^^^^^ FA102
3 | a_list.append("hello")
|
= help: Add `from __future__ import annotations`

no_future_import_uses_union_inner.py:2:18: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")

no_future_import_uses_union_inner.py:2:18: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 604 union
|
1 | def main() -> None:
2 | a_list: list[str | None] = []
| ^^^^^^^^^^ FA102
3 | a_list.append("hello")
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")

no_future_import_uses_union_inner.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
no_future_import_uses_union_inner.py:6:14: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
6 | def hello(y: dict[str | None, int]) -> None:
| ^^^^^^^^^^^^^^^^^^^^^ FA102
7 | z: tuple[str, str | None, str] = tuple(y)
8 | del z
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")

no_future_import_uses_union_inner.py:6:19: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
no_future_import_uses_union_inner.py:6:19: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 604 union
|
6 | def hello(y: dict[str | None, int]) -> None:
| ^^^^^^^^^^ FA102
7 | z: tuple[str, str | None, str] = tuple(y)
8 | del z
|
= help: Add `from __future__ import annotations`

no_future_import_uses_union_inner.py:7:8: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")

no_future_import_uses_union_inner.py:7:8: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 585 collection
|
6 | def hello(y: dict[str | None, int]) -> None:
7 | z: tuple[str, str | None, str] = tuple(y)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FA102
8 | del z
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")

no_future_import_uses_union_inner.py:7:19: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
no_future_import_uses_union_inner.py:7:19: FA102 [*] Missing `from __future__ import annotations`, but uses PEP 604 union
|
6 | def hello(y: dict[str | None, int]) -> None:
7 | z: tuple[str, str | None, str] = tuple(y)
| ^^^^^^^^^^ FA102
8 | del z
|
= help: Add `from __future__ import annotations`

Unsafe fix
1 |+from __future__ import annotations
1 2 | def main() -> None:
2 3 | a_list: list[str | None] = []
3 4 | a_list.append("hello")


5 changes: 2 additions & 3 deletions crates/ruff_linter/src/rules/refurb/rules/repeated_append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_python_codegen::Generator;
use ruff_python_semantic::analyze::typing::is_list;
use ruff_python_semantic::{Binding, BindingId, DefinitionId, SemanticModel};
use ruff_python_semantic::{Binding, BindingId, SemanticModel};
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -183,8 +183,7 @@ fn match_consecutive_appends<'a>(
let siblings: &[Stmt] = if semantic.at_top_level() {
// If the statement is at the top level, we should go to the parent module.
// Module is available in the definitions list.
let module = semantic.definitions[DefinitionId::module()].as_module()?;
module.python_ast
semantic.definitions.python_ast()?
} else {
// Otherwise, go to the parent, and take its body as a sequence of siblings.
semantic
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_python_semantic/src/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,12 @@ impl<'a> Definitions<'a> {

ContextualizedDefinitions(definitions.raw)
}

/// Returns a reference to the Python AST.
pub fn python_ast(&self) -> Option<&'a [Stmt]> {
let module = self[DefinitionId::module()].as_module()?;
Some(module.python_ast)
}
}

impl<'a> Deref for Definitions<'a> {
Expand Down

0 comments on commit a591725

Please sign in to comment.