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.

Sign up for GitHub

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

Audit remove_argument usages to use end-of-function #5480

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Audit remove_argument usages to use end-of-function #5480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions crates/ruff/resources/test/fixtures/pandas_vet/PD002.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

x.drop(["a"], axis=1, inplace=True)

x.drop(["a"], axis=1, inplace=True)
x.y.drop(["a"], axis=1, inplace=True)

x["y"].drop(["a"], axis=1, inplace=True)

x.drop(
inplace=True,
Expand All @@ -23,6 +25,7 @@
x.drop(["a"], axis=1, inplace=True, **kwargs)
f(x.drop(["a"], axis=1, inplace=True))

x.apply(lambda x: x.sort_values('a', inplace=True))
x.apply(lambda x: x.sort_values("a", inplace=True))
import torch
torch.m.ReLU(inplace=True) # safe because this isn't a pandas call

torch.m.ReLU(inplace=True) # safe because this isn't a pandas call
19 changes: 5 additions & 14 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,21 +668,17 @@ where
}
if !self.is_stub {
if self.enabled(Rule::DjangoModelWithoutDunderStr) {
if let Some(diagnostic) =
flake8_django::rules::model_without_dunder_str(self, bases, body, stmt)
{
self.diagnostics.push(diagnostic);
}
flake8_django::rules::model_without_dunder_str(self, class_def);
}
}
if self.enabled(Rule::GlobalStatement) {
pylint::rules::global_statement(self, name);
}
if self.enabled(Rule::UselessObjectInheritance) {
pyupgrade::rules::useless_object_inheritance(self, class_def, stmt);
pyupgrade::rules::useless_object_inheritance(self, class_def);
}
if self.enabled(Rule::UnnecessaryClassParentheses) {
pyupgrade::rules::unnecessary_class_parentheses(self, class_def, stmt);
pyupgrade::rules::unnecessary_class_parentheses(self, class_def);
}
if self.enabled(Rule::AmbiguousClassName) {
if let Some(diagnostic) =
Expand Down Expand Up @@ -2756,17 +2752,12 @@ where
flake8_debugger::rules::debugger_call(self, expr, func);
}
if self.enabled(Rule::PandasUseOfInplaceArgument) {
self.diagnostics.extend(
pandas_vet::rules::inplace_argument(self, expr, func, args, keywords)
.into_iter(),
);
pandas_vet::rules::inplace_argument(self, expr, func, args, keywords);
}
pandas_vet::rules::call(self, func);

if self.enabled(Rule::PandasUseOfPdMerge) {
if let Some(diagnostic) = pandas_vet::rules::use_of_pd_merge(func) {
self.diagnostics.push(diagnostic);
};
pandas_vet::rules::use_of_pd_merge(self, func);
}
if self.enabled(Rule::CallDatetimeWithoutTzinfo) {
flake8_datetimez::rules::call_datetime_without_tzinfo(
Expand Down
23 changes: 11 additions & 12 deletions crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,20 @@ impl Violation for DjangoModelWithoutDunderStr {

/// DJ008
pub(crate) fn model_without_dunder_str(
checker: &Checker,
bases: &[Expr],
body: &[Stmt],
class_location: &Stmt,
) -> Option<Diagnostic> {
checker: &mut Checker,
ast::StmtClassDef {
name, bases, body, ..
}: &ast::StmtClassDef,
) {
if !is_non_abstract_model(bases, body, checker.semantic()) {
return None;
return;
}
if !has_dunder_method(body) {
return Some(Diagnostic::new(
DjangoModelWithoutDunderStr,
class_location.range(),
));
if has_dunder_method(body) {
return;
}
None
checker
.diagnostics
.push(Diagnostic::new(DjangoModelWithoutDunderStr, name.range()));
}

fn has_dunder_method(body: &[Stmt]) -> bool {
Expand Down
62 changes: 15 additions & 47 deletions .../src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,26 @@
---
source: crates/ruff/src/rules/flake8_django/mod.rs
---
DJ008.py:6:1: DJ008 Model does not define `__str__` method
|
5 | # Models without __str__
6 | / class TestModel1(models.Model):
7 | | new_field = models.CharField(max_length=10)
8 | |
9 | | class Meta:
10 | | verbose_name = "test model"
11 | | verbose_name_plural = "test models"
12 | |
13 | | @property
14 | | def my_brand_new_property(self):
15 | | return 1
16 | |
17 | | def my_beautiful_method(self):
18 | | return 2
| |________________^ DJ008
|
DJ008.py:6:7: DJ008 Model does not define `__str__` method
|
5 | # Models without __str__
6 | class TestModel1(models.Model):
| ^^^^^^^^^^ DJ008
7 | new_field = models.CharField(max_length=10)
|

DJ008.py:21:1: DJ008 Model does not define `__str__` method
DJ008.py:21:7: DJ008 Model does not define `__str__` method
|
21 | / class TestModel2(Model):
22 | | new_field = models.CharField(max_length=10)
23 | |
24 | | class Meta:
25 | | verbose_name = "test model"
26 | | verbose_name_plural = "test models"
27 | |
28 | | @property
29 | | def my_brand_new_property(self):
30 | | return 1
31 | |
32 | | def my_beautiful_method(self):
33 | | return 2
| |________________^ DJ008
21 | class TestModel2(Model):
| ^^^^^^^^^^ DJ008
22 | new_field = models.CharField(max_length=10)
|

DJ008.py:36:1: DJ008 Model does not define `__str__` method
DJ008.py:36:7: DJ008 Model does not define `__str__` method
|
36 | / class TestModel3(Model):
37 | | new_field = models.CharField(max_length=10)
38 | |
39 | | class Meta:
40 | | abstract = False
41 | |
42 | | @property
43 | | def my_brand_new_property(self):
44 | | return 1
45 | |
46 | | def my_beautiful_method(self):
47 | | return 2
| |________________^ DJ008
36 | class TestModel3(Model):
| ^^^^^^^^^^ DJ008
37 | new_field = models.CharField(max_length=10)
|


135 changes: 63 additions & 72 deletions crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use std::fmt;

use anyhow::Result;
use ruff_text_size::{TextLen, TextRange, TextSize};
use ruff_text_size::{TextLen, TextRange};
use rustpython_parser::ast::Decorator;
use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Expr, Keyword, Ranged, Stmt};
use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Expr, Ranged, Stmt};

use ruff_diagnostics::{AlwaysAutofixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::collect_call_path;
use ruff_python_ast::helpers::collect_arg_names;
use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_semantic::analyze::visibility::is_abstract;
Expand All @@ -25,21 +23,6 @@ use super::helpers::{
get_mark_decorators, is_pytest_fixture, is_pytest_yield_fixture, keyword_is_literal,
};

#[derive(Debug, PartialEq, Eq)]
pub(crate) enum Parentheses {
None,
Empty,
}

impl fmt::Display for Parentheses {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
Parentheses::None => fmt.write_str(""),
Parentheses::Empty => fmt.write_str("()"),
}
}
}

#[violation]
pub struct PytestFixtureIncorrectParenthesesStyle {
expected: Parentheses,
Expand Down Expand Up @@ -196,8 +179,23 @@ impl AlwaysAutofixableViolation for PytestUnnecessaryAsyncioMarkOnFixture {
}
}

#[derive(Default)]
#[derive(Debug, PartialEq, Eq)]
enum Parentheses {
None,
Empty,
}

impl fmt::Display for Parentheses {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
Parentheses::None => fmt.write_str(""),
Parentheses::Empty => fmt.write_str("()"),
}
}
}

/// Visitor that skips functions
#[derive(Debug, Default)]
struct SkipFunctionsVisitor<'a> {
has_return_with_value: bool,
has_yield_from: bool,
Expand Down Expand Up @@ -245,7 +243,7 @@ where
}
}

fn get_fixture_decorator<'a>(
fn fixture_decorator<'a>(
decorators: &'a [Decorator],
semantic: &SemanticModel,
) -> Option<&'a Decorator> {
Expand All @@ -271,16 +269,6 @@ fn pytest_fixture_parentheses(
checker.diagnostics.push(diagnostic);
}

pub(crate) fn fix_extraneous_scope_function(
locator: &Locator,
stmt_at: TextSize,
expr_range: TextRange,
args: &[Expr],
keywords: &[Keyword],
) -> Result<Edit> {
remove_argument(locator, stmt_at, expr_range, args, keywords, false)
}

/// PT001, PT002, PT003
fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &Decorator) {
match &decorator.expression {
Expand All @@ -290,28 +278,31 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D
keywords,
range: _,
}) => {
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle)
&& !checker.settings.flake8_pytest_style.fixture_parentheses
&& args.is_empty()
&& keywords.is_empty()
{
let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end()));
pytest_fixture_parentheses(
checker,
decorator,
fix,
Parentheses::None,
Parentheses::Empty,
);
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) {
if !checker.settings.flake8_pytest_style.fixture_parentheses
&& args.is_empty()
&& keywords.is_empty()
{
let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end()));
pytest_fixture_parentheses(
checker,
decorator,
fix,
Parentheses::None,
Parentheses::Empty,
);
}
}

if checker.enabled(Rule::PytestFixturePositionalArgs) && !args.is_empty() {
checker.diagnostics.push(Diagnostic::new(
PytestFixturePositionalArgs {
function: func_name.to_string(),
},
decorator.range(),
));
if checker.enabled(Rule::PytestFixturePositionalArgs) {
if !args.is_empty() {
checker.diagnostics.push(Diagnostic::new(
PytestFixturePositionalArgs {
function: func_name.to_string(),
},
decorator.range(),
));
}
}

if checker.enabled(Rule::PytestExtraneousScopeFunction) {
Expand All @@ -324,16 +315,16 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D
let mut diagnostic =
Diagnostic::new(PytestExtraneousScopeFunction, scope_keyword.range());
if checker.patch(diagnostic.kind.rule()) {
let expr_range = diagnostic.range();
#[allow(deprecated)]
diagnostic.try_set_fix_from_edit(|| {
fix_extraneous_scope_function(
diagnostic.try_set_fix(|| {
remove_argument(
checker.locator,
decorator.start(),
expr_range,
func.end(),
scope_keyword.range,
args,
keywords,
false,
)
.map(Fix::suggested)
});
}
checker.diagnostics.push(diagnostic);
Expand All @@ -342,20 +333,20 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D
}
}
_ => {
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle)
&& checker.settings.flake8_pytest_style.fixture_parentheses
{
let fix = Fix::automatic(Edit::insertion(
Parentheses::Empty.to_string(),
decorator.end(),
));
pytest_fixture_parentheses(
checker,
decorator,
fix,
Parentheses::Empty,
Parentheses::None,
);
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) {
if checker.settings.flake8_pytest_style.fixture_parentheses {
let fix = Fix::automatic(Edit::insertion(
Parentheses::Empty.to_string(),
decorator.end(),
));
pytest_fixture_parentheses(
checker,
decorator,
fix,
Parentheses::Empty,
Parentheses::None,
);
}
}
}
}
Expand Down Expand Up @@ -511,7 +502,7 @@ pub(crate) fn fixture(
decorators: &[Decorator],
body: &[Stmt],
) {
let decorator = get_fixture_decorator(decorators, checker.semantic());
let decorator = fixture_decorator(decorators, checker.semantic());
if let Some(decorator) = decorator {
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle)
|| checker.enabled(Rule::PytestFixturePositionalArgs)
Expand Down
Loading