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

Implement flake8-future-annotations FA100 #3979

Merged
merged 22 commits into from
May 14, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ quality tools, including:
- [flake8-eradicate](https://pypi.org/project/flake8-eradicate/)
- [flake8-errmsg](https://pypi.org/project/flake8-errmsg/)
- [flake8-executable](https://pypi.org/project/flake8-executable/)
- [flake8-future-annotations](https://pypi.org/project/flake8-future-annotations/)
- [flake8-gettext](https://pypi.org/project/flake8-gettext/)
- [flake8-implicit-str-concat](https://pypi.org/project/flake8-implicit-str-concat/)
- [flake8-import-conventions](https://github.com/joaopalmeiro/flake8-import-conventions)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from typing import List
import typing as t


def main(_: List[int]) -> None:
a_list: t.List[str] = []
a_list.append("hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from typing import List


def main() -> None:
a_list: List[str] = []
a_list.append("hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from typing import Dict, List, Optional, Set, Union, cast


def main() -> None:
a_list: List[Optional[str]] = []
a_list.append("hello")
a_dict = cast(Dict[int | None, Union[int, Set[bool]]], {})
a_dict[1] = {True, False}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import typing


def main() -> None:
a_list: typing.List[str] = []
a_list.append("hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import typing as t


def main() -> None:
a_list: t.List[str] = []
a_list.append("hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def main() -> None:
a_list: list[str] = []
a_list.append("hello")


def hello(y: dict[str, int]) -> None:
del y
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def main() -> None:
a_list: list[str] | None = []
a_list.append("hello")


def hello(y: dict[str, int] | None) -> None:
del y
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
def main() -> None:
a_list: list[str | None] = []
a_list.append("hello")


def hello(y: dict[str | None, int]) -> None:
z: tuple[str, str | None, str] = tuple(y)
del z
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def main() -> str:
a_str = "hello"
return a_str
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from typing import NamedTuple


class Stuff(NamedTuple):
x: int


def main() -> None:
a_list = Stuff(5)
print(a_list)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from __future__ import annotations


def main() -> None:
a_list: list[str] = []
a_list.append("hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import typing

IRRELEVANT = typing.TypeVar


def main() -> None:
List: list[str] = []
List.append("hello")
52 changes: 45 additions & 7 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ use crate::registry::{AsRule, Rule};
use crate::rules::{
flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap,
flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger,
flake8_django, flake8_errmsg, flake8_gettext, flake8_implicit_str_concat,
flake8_import_conventions, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi,
flake8_pytest_style, flake8_raise, flake8_return, flake8_self, flake8_simplify,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, flake8_use_pathlib, flynt,
mccabe, numpy, pandas_vet, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks,
pylint, pyupgrade, ruff, tryceratops,
flake8_django, flake8_errmsg, flake8_future_annotations, flake8_gettext,
flake8_implicit_str_concat, flake8_import_conventions, flake8_logging_format, flake8_pie,
flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_self,
flake8_simplify, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments,
flake8_use_pathlib, flynt, mccabe, numpy, pandas_vet, pep8_naming, pycodestyle, pydocstyle,
pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
Expand Down Expand Up @@ -1077,7 +1077,6 @@ where
pyupgrade::rules::unnecessary_builtin_import(self, stmt, module, names);
}
}

if self.settings.rules.enabled(Rule::BannedApi) {
if let Some(module) =
helpers::resolve_imported_module_path(level, module, self.module_path)
Expand Down Expand Up @@ -2258,12 +2257,25 @@ where
match &expr.node {
ExprKind::Subscript(ast::ExprSubscript { value, slice, .. }) => {
// Ex) Optional[...], Union[...]
if self
.settings
.rules
.enabled(Rule::MissingFutureAnnotationsImport)
&& (self.settings.target_version < PythonVersion::Py310
&& (self.settings.target_version >= PythonVersion::Py37
&& !self.ctx.future_annotations()
&& self.ctx.in_annotation()))
&& analyze::typing::is_pep604_builtin(value, &self.ctx)
{
flake8_future_annotations::rules::missing_future_annotations(self, value);
}
if !self.settings.pyupgrade.keep_runtime_typing
&& self.settings.rules.enabled(Rule::NonPEP604Annotation)
&& (self.settings.target_version >= PythonVersion::Py310
|| (self.settings.target_version >= PythonVersion::Py37
&& self.ctx.future_annotations()
&& self.ctx.in_annotation()))
&& analyze::typing::is_pep604_builtin(value, &self.ctx)
{
pyupgrade::rules::use_pep604_annotation(self, expr, value, slice);
}
Expand Down Expand Up @@ -2321,6 +2333,20 @@ where
}

// Ex) List[...]
if self
.settings
.rules
.enabled(Rule::MissingFutureAnnotationsImport)
&& (self.settings.target_version < PythonVersion::Py39
&& (self.settings.target_version >= PythonVersion::Py37
&& !self.ctx.future_annotations()
&& self.ctx.in_annotation()))
&& analyze::typing::is_pep585_builtin(expr, &self.ctx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is a lot to read, I'd like to clean this up... But the idea is: mirror the logic below for Rule::NonPEP585Annotation, but flip the !self.ctx.future_annotations(). So we're asking: would we trigger here if __future__ annotations were enabled?

{
flake8_future_annotations::rules::missing_future_annotations(
self, expr,
);
}
if !self.settings.pyupgrade.keep_runtime_typing
&& self.settings.rules.enabled(Rule::NonPEP585Annotation)
&& (self.settings.target_version >= PythonVersion::Py39
Expand Down Expand Up @@ -2372,6 +2398,18 @@ where
}
ExprKind::Attribute(ast::ExprAttribute { attr, value, .. }) => {
// Ex) typing.List[...]
if self
.settings
.rules
.enabled(Rule::MissingFutureAnnotationsImport)
&& (self.settings.target_version < PythonVersion::Py39
&& (self.settings.target_version >= PythonVersion::Py37
&& !self.ctx.future_annotations()
&& self.ctx.in_annotation()))
&& analyze::typing::is_pep585_builtin(expr, &self.ctx)
{
flake8_future_annotations::rules::missing_future_annotations(self, expr);
}
if !self.settings.pyupgrade.keep_runtime_typing
&& self.settings.rules.enabled(Rule::NonPEP585Annotation)
&& (self.settings.target_version >= PythonVersion::Py39
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Flake8Annotations, "206") => Rule::MissingReturnTypeClassMethod,
(Flake8Annotations, "401") => Rule::AnyType,

// flake8-future-annotations
(Flake8FutureAnnotations, "100") => Rule::MissingFutureAnnotationsImport,

// flake8-2020
(Flake82020, "101") => Rule::SysVersionSlice3,
(Flake82020, "102") => Rule::SysVersion2,
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ ruff_macros::register_rules!(
rules::flake8_annotations::rules::MissingReturnTypeStaticMethod,
rules::flake8_annotations::rules::MissingReturnTypeClassMethod,
rules::flake8_annotations::rules::AnyType,
// flake8-future-annotations
rules::flake8_future_annotations::rules::MissingFutureAnnotationsImport,
// flake8-2020
rules::flake8_2020::rules::SysVersionSlice3,
rules::flake8_2020::rules::SysVersion2,
Expand Down Expand Up @@ -769,6 +771,9 @@ pub enum Linter {
/// [flake8-executable](https://pypi.org/project/flake8-executable/)
#[prefix = "EXE"]
Flake8Executable,
/// [flake8-future-annotations](https://pypi.org/project/flake8-future-annotations/)
#[prefix = "FA"]
Flake8FutureAnnotations,
/// [flake8-implicit-str-concat](https://pypi.org/project/flake8-implicit-str-concat/)
#[prefix = "ISC"]
Flake8ImplicitStrConcat,
Expand Down
40 changes: 40 additions & 0 deletions crates/ruff/src/rules/flake8_future_annotations/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//! Rules from [flake8-future-annotations](https://pypi.org/project/flake8-future-annotations/).
pub(crate) mod rules;

#[cfg(test)]
mod tests {
use std::path::Path;

use anyhow::Result;
use test_case::test_case;

use crate::registry::Rule;
use crate::settings::types::PythonVersion;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Path::new("edge_case.py"); "edge_case")]
#[test_case(Path::new("from_typing_import.py"); "from_typing_import")]
#[test_case(Path::new("from_typing_import_many.py"); "from_typing_import_many")]
#[test_case(Path::new("import_typing.py"); "import_typing")]
#[test_case(Path::new("import_typing_as.py"); "import_typing_as")]
#[test_case(Path::new("no_future_import_uses_lowercase.py"); "no_future_import_uses_lowercase")]
#[test_case(Path::new("no_future_import_uses_union.py"); "no_future_import_uses_union")]
#[test_case(Path::new("no_future_import_uses_union_inner.py"); "no_future_import_uses_union_inner")]
#[test_case(Path::new("ok_no_types.py"); "ok_no_types")]
#[test_case(Path::new("ok_non_simplifiable_types.py"); "ok_non_simplifiable_types")]
#[test_case(Path::new("ok_uses_future.py"); "ok_uses_future")]
#[test_case(Path::new("ok_variable_name.py"); "ok_variable_name")]
fn rules(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)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
78 changes: 78 additions & 0 deletions crates/ruff/src/rules/flake8_future_annotations/rules.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use rustpython_parser::ast::Expr;

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

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

/// ## What it does
/// Checks for missing `from __future__ import annotations` imports upon
/// detecting type annotations that can be written more succinctly under
/// PEP 563.
///
/// ## Why is this bad?
/// PEP 563 enabled the use of a number of convenient type annotations, such as
/// `list[str]` instead of `List[str]`, or `str | None` instead of
/// `Optional[str]`. However, these annotations are only available on Python
/// 3.9 and higher, _unless_ the `from __future__ import annotations` import is present.
///
/// By adding the `__future__` import, the pyupgrade rules can automatically
/// migrate existing code to use the new syntax, even for older Python versions.
/// This rule thus pairs well with pyupgrade and with Ruff's pyupgrade rules.
///
/// ## Example
/// ```python
/// from typing import List, Dict, Optional
///
///
/// def function(a_dict: Dict[str, Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// ```
///
/// Use instead:
/// ```python
/// from __future__ import annotations
///
/// from typing import List, Dict, Optional
///
///
/// def function(a_dict: Dict[str, Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// ```
///
/// 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")
/// ```
#[violation]
pub struct MissingFutureAnnotationsImport {
name: String,
}

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

/// FA100
pub(crate) fn missing_future_annotations(checker: &mut Checker, expr: &Expr) {
if let Some(binding) = checker.ctx.resolve_call_path(expr) {
checker.diagnostics.push(Diagnostic::new(
MissingFutureAnnotationsImport {
name: format_call_path(&binding),
},
expr.range(),
));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
edge_case.py:6:13: FA100 Missing `from __future__ import annotations`, but uses `typing.List`
|
6 | def main(_: List[int]) -> None:
7 | a_list: t.List[str] = []
| ^^^^^^ FA100
8 | a_list.append("hello")
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
from_typing_import.py:5:13: FA100 Missing `from __future__ import annotations`, but uses `typing.List`
|
5 | def main() -> None:
6 | a_list: List[str] = []
| ^^^^ FA100
7 | a_list.append("hello")
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
from_typing_import_many.py:5:13: FA100 Missing `from __future__ import annotations`, but uses `typing.List`
|
5 | def main() -> None:
6 | a_list: List[Optional[str]] = []
| ^^^^ FA100
7 | a_list.append("hello")
8 | a_dict = cast(Dict[int | None, Union[int, Set[bool]]], {})
|

from_typing_import_many.py:5:18: FA100 Missing `from __future__ import annotations`, but uses `typing.Optional`
|
5 | def main() -> None:
6 | a_list: List[Optional[str]] = []
| ^^^^^^^^ FA100
7 | a_list.append("hello")
8 | a_dict = cast(Dict[int | None, Union[int, Set[bool]]], {})
|


Loading