Skip to content

Commit

Permalink
Add flake-pyi PYI033 "Do not use type comments in stubs"
Browse files Browse the repository at this point in the history
I think this is also a case where people like want this for .py files, too, at least on 3.7+ (https://docs.python.org/3/whatsnew/3.7.html#pep-563-postponed-evaluation-of-annotations)

On that note, does none of the tools upgrade all type comments to type annotations automatically? I looked in pyupgrade and googled but couldn't find anything
  • Loading branch information
konstin committed Mar 2, 2023
1 parent 3ed539d commit df50951
Show file tree
Hide file tree
Showing 14 changed files with 138 additions and 4 deletions.
4 changes: 4 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
[alias]
dev = "run --package ruff_dev --bin ruff_dev"

[target.x86_64-unknown-linux-gnu]
linker = "clang"
rustflags = ["-C", "link-arg=-fuse-ld=/home/konsti/.local/bin/mold"]

[target.'cfg(all())']
rustflags = [
# CLIPPY LINT SETTINGS
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI033.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# OK
x = 1 # type: int


# OK
def reverse(x): # type:(str) -> str
...
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI033.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# ERROR: PYI033 Don't use type comments in stub file
x = 1 # type: int


# ERROR: PYI033 Don't use type comments in stub file
def reverse(x): # type:(str) -> str
...
11 changes: 9 additions & 2 deletions crates/ruff/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::lex::docstring_detection::StateMachine;
use crate::registry::{Diagnostic, Rule};
use crate::rules::ruff::rules::Context;
use crate::rules::{
eradicate, flake8_commas, flake8_implicit_str_concat, flake8_quotes, pycodestyle, pyupgrade,
ruff,
eradicate, flake8_commas, flake8_implicit_str_concat, flake8_pyi, flake8_quotes, pycodestyle,
pyupgrade, ruff,
};
use crate::settings::{flags, Settings};
use crate::source_code::Locator;
Expand All @@ -18,6 +18,7 @@ pub fn check_tokens(
tokens: &[LexResult],
settings: &Settings,
autofix: flags::Autofix,
is_interface_definition: bool,
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];

Expand Down Expand Up @@ -55,6 +56,7 @@ pub fn check_tokens(
.enabled(&Rule::TrailingCommaOnBareTupleProhibited)
|| settings.rules.enabled(&Rule::TrailingCommaProhibited);
let enforce_extraneous_parenthesis = settings.rules.enabled(&Rule::ExtraneousParentheses);
let enforce_type_comment_in_stub = settings.rules.enabled(&Rule::TypeCommentInStub);

// RUF001, RUF002, RUF003
if enforce_ambiguous_unicode_character {
Expand Down Expand Up @@ -161,5 +163,10 @@ pub fn check_tokens(
);
}

// PYI033
if enforce_type_comment_in_stub && is_interface_definition {
diagnostics.extend(flake8_pyi::rules::type_comment_in_stub(tokens));
}

diagnostics
}
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Flake8Pyi, "011") => Rule::TypedArgumentSimpleDefaults,
(Flake8Pyi, "014") => Rule::ArgumentSimpleDefaults,
(Flake8Pyi, "021") => Rule::DocstringInStub,
(Flake8Pyi, "033") => Rule::TypeCommentInStub,

// flake8-pytest-style
(Flake8PytestStyle, "001") => Rule::IncorrectFixtureParenthesesStyle,
Expand Down
10 changes: 9 additions & 1 deletion crates/ruff/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
use crate::message::{Message, Source};
use crate::noqa::{add_noqa, rule_is_ignored};
use crate::registry::{Diagnostic, Rule};
use crate::resolver::is_interface_definition_path;
use crate::rules::pycodestyle;
use crate::settings::{flags, Settings};
use crate::source_code::{Indexer, Locator, Stylist};
Expand Down Expand Up @@ -83,7 +84,14 @@ pub fn check_path(
.iter_enabled()
.any(|rule_code| rule_code.lint_source().is_tokens())
{
diagnostics.extend(check_tokens(locator, &tokens, settings, autofix));
let is_interface_definition = is_interface_definition_path(path);
diagnostics.extend(check_tokens(
locator,
&tokens,
settings,
autofix,
is_interface_definition,
));
}

// Run the filesystem-based rules.
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ ruff_macros::register_rules!(
rules::flake8_pyi::rules::DocstringInStub,
rules::flake8_pyi::rules::TypedArgumentSimpleDefaults,
rules::flake8_pyi::rules::ArgumentSimpleDefaults,
rules::flake8_pyi::rules::TypeCommentInStub,
// flake8-pytest-style
rules::flake8_pytest_style::rules::IncorrectFixtureParenthesesStyle,
rules::flake8_pytest_style::rules::FixturePositionalArgs,
Expand Down Expand Up @@ -827,7 +828,8 @@ impl Rule {
| Rule::MultipleStatementsOnOneLineColon
| Rule::UselessSemicolon
| Rule::MultipleStatementsOnOneLineSemicolon
| Rule::TrailingCommaProhibited => &LintSource::Tokens,
| Rule::TrailingCommaProhibited
| Rule::TypeCommentInStub => &LintSource::Tokens,
Rule::IOError => &LintSource::Io,
Rule::UnsortedImports | Rule::MissingRequiredImport => &LintSource::Imports,
Rule::ImplicitNamespacePackage | Rule::InvalidModuleName => &LintSource::Filesystem,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ mod tests {
#[test_case(Rule::ArgumentSimpleDefaults, Path::new("PYI014.pyi"))]
#[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))]
#[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))]
#[test_case(Rule::TypeCommentInStub, Path::new("PYI033.py"))]
#[test_case(Rule::TypeCommentInStub, Path::new("PYI033.pyi"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub use simple_defaults::{
argument_simple_defaults, typed_argument_simple_defaults, ArgumentSimpleDefaults,
TypedArgumentSimpleDefaults,
};
pub use type_comment_in_stub::{type_comment_in_stub, TypeCommentInStub};
pub use unrecognized_platform::{
unrecognized_platform, UnrecognizedPlatformCheck, UnrecognizedPlatformName,
};
Expand All @@ -17,4 +18,5 @@ mod non_empty_stub_body;
mod pass_statement_stub_body;
mod prefix_type_params;
mod simple_defaults;
mod type_comment_in_stub;
mod unrecognized_platform;
59 changes: 59 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/type_comment_in_stub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
use rustpython_parser::lexer::LexResult;
use rustpython_parser::Tok;

use ruff_macros::{define_violation, derive_message_formats};

use crate::registry::Diagnostic;
use crate::violation::Violation;
use crate::Range;

define_violation!(
/// ## What it does
/// Do not use type comments (e.g. `x = 1 # type: int`) in stubs, even if
/// the stub supports Python 2. Always use annotations instead (e.g.
/// `x: int = 1`).
///
/// ## Why is this bad?
/// You should use type annotation directly in pyi files.
///
/// ## Example
/// ```python
/// x = 1 # type: int
/// ```
///
/// Use instead:
/// ```python
/// x: int = 1
/// ```
pub struct TypeCommentInStub;
);
impl Violation for TypeCommentInStub {
#[derive_message_formats]
fn message(&self) -> String {
format!("Don't use type comments in stub file")
}
}

/// PYI033
pub fn type_comment_in_stub(tokens: &[LexResult]) -> Vec<Diagnostic> {
let mut diagnostics = vec![];

for token in tokens.iter().flatten() {
if let (location, Tok::Comment(comment), end_location) = token {
// I couldn't find any PEP on the exact syntax (the closest being
// https://peps.python.org/pep-0484/#type-comments), but every case I saw used
// `# type:` verbatim so this seems to be the right thing to pick
if comment.starts_with("# type:") {
diagnostics.push(Diagnostic::new(
TypeCommentInStub,
Range {
location: *location,
end_location: *end_location,
},
));
}
}
}

diagnostics
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
expression: diagnostics
---
[]

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
expression: diagnostics
---
- kind:
TypeCommentInStub: ~
location:
row: 2
column: 6
end_location:
row: 2
column: 17
fix: ~
parent: ~
- kind:
TypeCommentInStub: ~
location:
row: 6
column: 17
end_location:
row: 6
column: 36
fix: ~
parent: ~

2 changes: 2 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ Options:
Run in watch mode by re-running whenever files change
--fix-only
Fix any fixable lint violations, but don't report on leftover violations. Implies `--fix`
--ignore-noqa
Ignore any `# noqa` comments
--format <FORMAT>
Output serialization format for violations [env: RUFF_FORMAT=] [possible values: text, json, junit, grouped, github, gitlab, pylint]
--target-version <TARGET_VERSION>
Expand Down
2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit df50951

Please sign in to comment.