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 12 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")
37 changes: 30 additions & 7 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,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 @@ -1175,7 +1175,20 @@ where
pyupgrade::rules::unnecessary_builtin_import(self, stmt, module, names);
}
}

if self
.settings
.rules
.enabled(Rule::MissingFutureAnnotationsWithImports)
{
if let Some(module) = module.as_deref() {
flake8_future_annotations::rules::missing_future_annotations_from_typing_import(
self,
stmt,
module,
names,
);
}
}
if self.settings.rules.enabled(Rule::BannedApi) {
if let Some(module) = helpers::resolve_imported_module_path(
*level,
Expand Down Expand Up @@ -2451,6 +2464,16 @@ where
if self.settings.rules.enabled(Rule::BannedApi) {
flake8_tidy_imports::banned_api::banned_attribute_access(self, expr);
}
if self
.settings
.rules
.enabled(Rule::MissingFutureAnnotationsWithImports)
&& 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.

These should only be activated for minimum-version < Python 3.9, right? Since PEP 585 was part of Python 3.9 (so future annotations aren't required to use the standard-library variants in 3.9 and later).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice, this works on Python 3.7+.

Copy link
Member

Choose a reason for hiding this comment

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

But if your minimum-supported Python version is Python 3.9, you don't need __future__ annotations to use the standard-library generics. So these errors would already be detected and fixed by the existing pyupgrade rules, right?

Copy link
Contributor Author

@TylerYep TylerYep May 12, 2023

Choose a reason for hiding this comment

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

Ah, you're asking about the upper version.

Unions like str | None didn't start working until Python 3.10, which are also covered by this plugin.

After 3.10, the future annotations import is not as useful anymore.

{
flake8_future_annotations::rules::missing_future_annotations_from_typing_usage(
self, expr,
);
}
if self.settings.rules.enabled(Rule::PrivateMemberAccess) {
flake8_self::rules::private_member_access(self, expr);
}
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 @@ -330,6 +330,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::MissingFutureAnnotationsWithImports,

// 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 @@ -289,6 +289,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::MissingFutureAnnotationsWithImports,
// flake8-2020
rules::flake8_2020::rules::SysVersionSlice3,
rules::flake8_2020::rules::SysVersion2,
Expand Down Expand Up @@ -758,6 +760,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
36 changes: 36 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,36 @@
//! 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::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::for_rules(vec![Rule::MissingFutureAnnotationsWithImports]),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
100 changes: 100 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,100 @@
use itertools::Itertools;
use rustpython_parser::ast::{Alias, Expr, Stmt};

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

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

/// ## Example
/// ```python
/// import typing as t
/// from typing import List
///
///
/// def function(a_dict: t.Dict[str, t.Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// ```
///
/// To fix the lint error:
/// ```python
/// from __future__ import annotations
///
/// import typing as t
/// from typing import List
///
///
/// def function(a_dict: t.Dict[str, t.Optional[int]]) -> None:
/// a_list: List[str] = []
/// a_list.append("hello")
/// ```
///
/// After running additional pyupgrade autofixes:
/// ```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 MissingFutureAnnotationsWithImports {
pub names: Vec<String>,
}

impl Violation for MissingFutureAnnotationsWithImports {
#[derive_message_formats]
fn message(&self) -> String {
let MissingFutureAnnotationsWithImports { names } = self;
let names = names.iter().map(|name| format!("`{name}`")).join(", ");
format!("Missing `from __future__ import annotations`, but imports: {names}")
}
}

/// FA100
pub fn missing_future_annotations_from_typing_import(
checker: &mut Checker,
stmt: &Stmt,
module: &str,
names: &[Alias],
) {
if checker.ctx.annotations_future_enabled {
return;
}

let names: Vec<String> = names
.iter()
.map(|name| name.node.name.as_str())
.filter(|alias| PEP_585_SUBSCRIPT_ELIGIBLE.contains(&[module, alias].as_slice()))
.map(std::string::ToString::to_string)
.sorted()
.collect();

if !names.is_empty() {
checker.diagnostics.push(Diagnostic::new(
MissingFutureAnnotationsWithImports { names },
stmt.range(),
));
}
}

/// FA100
pub fn missing_future_annotations_from_typing_usage(checker: &mut Checker, expr: &Expr) {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, last question: what's the benefit in flagging both the usages and the imports? Would flagging the import alone be insufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user uses import typing, we can only flag typing.List at the usage. If they use from typing import List, then flagging the import alone is sufficient.

if checker.ctx.annotations_future_enabled {
return;
}

if let Some(binding) = checker.ctx.resolve_call_path(expr) {
if PEP_585_SUBSCRIPT_ELIGIBLE.contains(&binding.as_slice()) {
checker.diagnostics.push(Diagnostic::new(
MissingFutureAnnotationsWithImports {
names: vec![binding.iter().join(".")],
},
expr.range(),
));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
edge_case.py:1:1: FA100 Missing `from __future__ import annotations`, but imports: `List`
|
1 | from typing import List
| ^^^^^^^^^^^^^^^^^^^^^^^ FA100
2 | import typing as t
|

edge_case.py:6:13: FA100 Missing `from __future__ import annotations`, but imports: `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,10 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
from_typing_import.py:1:1: FA100 Missing `from __future__ import annotations`, but imports: `List`
|
1 | from typing import List
| ^^^^^^^^^^^^^^^^^^^^^^^ FA100
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/ruff/src/rules/flake8_future_annotations/mod.rs
---
from_typing_import_many.py:1:1: FA100 Missing `from __future__ import annotations`, but imports: `Dict`, `List`, `Optional`, `Set`, `Union`
|
1 | from typing import Dict, List, Optional, Set, Union, cast
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FA100
|


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


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

Loading