Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-future-annotations] Implement FA102 #4702

Merged
merged 7 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 38 additions & 9 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2184,19 +2184,19 @@ where
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
// Ex) Optional[...], Union[...]
if self.any_enabled(&[
Rule::MissingFutureAnnotationsImport,
Rule::MissingFutureAnnotationsImportOldStyle,
Rule::NonPEP604Annotation,
]) {
if let Some(operator) =
analyze::typing::to_pep604_operator(value, slice, &self.semantic_model)
{
if self.enabled(Rule::MissingFutureAnnotationsImport) {
if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) {
if self.settings.target_version < PythonVersion::Py310
&& self.settings.target_version >= PythonVersion::Py37
&& !self.semantic_model.future_annotations()
&& self.semantic_model.in_annotation()
{
flake8_future_annotations::rules::missing_future_annotations(
flake8_future_annotations::rules::missing_future_annotations_old_style(
self, value,
);
}
Expand All @@ -2215,6 +2215,21 @@ where
}
}

// Ex) list[...]
if self.enabled(Rule::MissingFutureAnnotationsImportNewStyle) {
if self.settings.target_version < PythonVersion::Py39
&& !self.semantic_model.future_annotations()
&& self.semantic_model.in_annotation()
&& analyze::typing::is_pep585_generic(value, &self.semantic_model)
{
flake8_future_annotations::rules::missing_future_annotations_new_style(
self,
expr,
flake8_future_annotations::rules::Reason::PEP585,
);
}
}

if self.semantic_model.match_typing_expr(value, "Literal") {
self.semantic_model.flags |= SemanticModelFlags::LITERAL;
}
Expand Down Expand Up @@ -2271,19 +2286,19 @@ where

// Ex) List[...]
if self.any_enabled(&[
Rule::MissingFutureAnnotationsImport,
Rule::MissingFutureAnnotationsImportOldStyle,
Rule::NonPEP585Annotation,
]) {
if let Some(replacement) =
analyze::typing::to_pep585_generic(expr, &self.semantic_model)
{
if self.enabled(Rule::MissingFutureAnnotationsImport) {
if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) {
if self.settings.target_version < PythonVersion::Py39
&& self.settings.target_version >= PythonVersion::Py37
&& !self.semantic_model.future_annotations()
&& self.semantic_model.in_annotation()
{
flake8_future_annotations::rules::missing_future_annotations(
flake8_future_annotations::rules::missing_future_annotations_old_style(
self, expr,
);
}
Expand Down Expand Up @@ -2349,19 +2364,19 @@ where
Expr::Attribute(ast::ExprAttribute { attr, value, .. }) => {
// Ex) typing.List[...]
if self.any_enabled(&[
Rule::MissingFutureAnnotationsImport,
Rule::MissingFutureAnnotationsImportOldStyle,
Rule::NonPEP585Annotation,
]) {
if let Some(replacement) =
analyze::typing::to_pep585_generic(expr, &self.semantic_model)
{
if self.enabled(Rule::MissingFutureAnnotationsImport) {
if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) {
if self.settings.target_version < PythonVersion::Py39
&& self.settings.target_version >= PythonVersion::Py37
&& !self.semantic_model.future_annotations()
&& self.semantic_model.in_annotation()
{
flake8_future_annotations::rules::missing_future_annotations(
flake8_future_annotations::rules::missing_future_annotations_old_style(
self, expr,
);
}
Expand Down Expand Up @@ -3198,6 +3213,20 @@ where
op: Operator::BitOr,
..
}) => {
// Ex) `str | None`
if self.enabled(Rule::MissingFutureAnnotationsImportNewStyle) {
if self.settings.target_version < PythonVersion::Py310
&& !self.semantic_model.future_annotations()
&& self.semantic_model.in_annotation()
{
flake8_future_annotations::rules::missing_future_annotations_new_style(
self,
expr,
flake8_future_annotations::rules::Reason::PEP604,
);
}
}

if self.is_stub {
if self.enabled(Rule::DuplicateUnionMember)
&& self.semantic_model.in_type_definition()
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Annotations, "401") => (RuleGroup::Unspecified, Rule::AnyType),

// flake8-future-annotations
(Flake8FutureAnnotations, "100") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImport),
(Flake8FutureAnnotations, "100") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImportOldStyle),
(Flake8FutureAnnotations, "102") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImportNewStyle),

// flake8-2020
(Flake82020, "101") => (RuleGroup::Unspecified, Rule::SysVersionSlice3),
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ ruff_macros::register_rules!(
rules::flake8_annotations::rules::MissingReturnTypeClassMethod,
rules::flake8_annotations::rules::AnyType,
// flake8-future-annotations
rules::flake8_future_annotations::rules::MissingFutureAnnotationsImport,
rules::flake8_future_annotations::rules::MissingFutureAnnotationsImportOldStyle,
rules::flake8_future_annotations::rules::MissingFutureAnnotationsImportNewStyle,
// flake8-2020
rules::flake8_2020::rules::SysVersionSlice3,
rules::flake8_2020::rules::SysVersion2,
Expand Down
22 changes: 20 additions & 2 deletions crates/ruff/src/rules/flake8_future_annotations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,31 @@ mod tests {
#[test_case(Path::new("ok_non_simplifiable_types.py"))]
#[test_case(Path::new("ok_uses_future.py"))]
#[test_case(Path::new("ok_variable_name.py"))]
fn rules(path: &Path) -> Result<()> {
fn fa100(path: &Path) -> Result<()> {
let snapshot = path.to_string_lossy().into_owned();
let diagnostics = test_path(
Path::new("flake8_future_annotations").join(path).as_path(),
&settings::Settings {
target_version: PythonVersion::Py37,
..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImport)
..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImportOldStyle)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Path::new("no_future_import_uses_lowercase.py"))]
#[test_case(Path::new("no_future_import_uses_union.py"))]
#[test_case(Path::new("no_future_import_uses_union_inner.py"))]
#[test_case(Path::new("ok_no_types.py"))]
#[test_case(Path::new("ok_uses_future.py"))]
fn fa102(path: &Path) -> Result<()> {
let snapshot = format!("fa102_{}", path.to_string_lossy());
let diagnostics = test_path(
Path::new("flake8_future_annotations").join(path).as_path(),
&settings::Settings {
target_version: PythonVersion::Py37,
..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImportNewStyle)
},
)?;
assert_messages!(snapshot, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use rustpython_parser::ast::{Expr, Ranged};
use std::fmt;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of PEP 585- and PEP 604-style type annotations in Python
/// modules that lack the required `from __future__ import annotations` import
/// for compatibility with older Python versions.
///
/// ## Why is this bad?
/// Using PEP 585 and PEP 604 style annotations without a `from __future__ import
/// annotations` import will cause runtime errors on Python versions prior to
/// 3.9 and 3.10, respectively.
///
/// By adding the `__future__` import, the interpreter will no longer interpret
/// annotations at evaluation time, making the code compatible with both past
/// and future Python versions.
///
/// ## Example
/// ```python
/// def func(obj: dict[str, int | None]) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// from __future__ import annotations
///
///
/// def func(obj: dict[str, int | None]) -> None:
/// ...
/// ```
#[violation]
pub struct MissingFutureAnnotationsImportNewStyle {
reason: Reason,
}

#[derive(Debug, PartialEq, Eq)]
pub(crate) enum Reason {
/// The type annotation is written in PEP 585 style (e.g., `list[int]`).
PEP585,
/// The type annotation is written in PEP 604 style (e.g., `int | None`).
PEP604,
}

impl fmt::Display for Reason {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
Reason::PEP585 => fmt.write_str("PEP 585 collection"),
Reason::PEP604 => fmt.write_str("PEP 604 union"),
}
}
}

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

/// FA102
pub(crate) fn missing_future_annotations_new_style(
checker: &mut Checker,
expr: &Expr,
reason: Reason,
) {
checker.diagnostics.push(Diagnostic::new(
MissingFutureAnnotationsImportNewStyle { reason },
expr.range(),
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ use crate::checkers::ast::Checker;
/// from typing import List, Dict, Optional
///
///
/// def function(a_dict: Dict[str, Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// def func(obj: Dict[str, Optional[int]]) -> None:
/// ...
/// ```
///
/// Use instead:
Expand All @@ -38,43 +37,41 @@ use crate::checkers::ast::Checker;
/// from typing import List, Dict, Optional
///
///
/// def function(a_dict: Dict[str, Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// def func(obj: Dict[str, Optional[int]]) -> None:
/// ...
/// ```
///
/// After running the additional pyupgrade rules:
/// ```python
/// from __future__ import annotations
///
///
/// def function(a_dict: dict[str, int | None]) -> None:
/// a_list: list[str] = []
/// a_list.append("hello")
/// def func(obj: dict[str, int | None]) -> None:
/// ...
/// ```
#[violation]
pub struct MissingFutureAnnotationsImport {
pub struct MissingFutureAnnotationsImportOldStyle {
name: String,
}

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

/// FA100
pub(crate) fn missing_future_annotations(checker: &mut Checker, expr: &Expr) {
pub(crate) fn missing_future_annotations_old_style(checker: &mut Checker, expr: &Expr) {
let name = checker
.semantic_model()
.resolve_call_path(expr)
.map(|binding| format_call_path(&binding));

if let Some(name) = name {
checker.diagnostics.push(Diagnostic::new(
MissingFutureAnnotationsImport { name },
MissingFutureAnnotationsImportOldStyle { name },
expr.range(),
));
}
Expand Down
10 changes: 7 additions & 3 deletions crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
pub(crate) use missing_future_annotations::{
missing_future_annotations, MissingFutureAnnotationsImport,
pub(crate) use missing_future_annotations_new_style::{
missing_future_annotations_new_style, MissingFutureAnnotationsImportNewStyle, Reason,
};
pub(crate) use missing_future_annotations_old_style::{
missing_future_annotations_old_style, MissingFutureAnnotationsImportOldStyle,
};

mod missing_future_annotations;
mod missing_future_annotations_new_style;
mod missing_future_annotations_old_style;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff/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
|
2 | def main() -> None:
3 | a_list: list[str] = []
| ^^^^^^^^^ FA102
4 | a_list.append("hello")
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
|
2 | def main() -> None:
3 | a_list: list[str] | None = []
| ^^^^^^^^^^^^^^^^ FA102
4 | a_list.append("hello")
|

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


Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: crates/ruff/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
|
2 | def main() -> None:
3 | a_list: list[str | None] = []
| ^^^^^^^^^^^^^^^^ FA102
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
|
2 | def main() -> None:
3 | a_list: list[str | None] = []
| ^^^^^^^^^^ FA102
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
|
7 | def hello(y: dict[str | None, int]) -> None:
8 | z: tuple[str, str | None, str] = tuple(y)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FA102
9 | del z
|

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


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---