Skip to content

Commit

Permalink
Remove some diagnostics.extend calls (#5483)
Browse files Browse the repository at this point in the history
## Summary

It's more efficient (and more idiomatic for us) to pass in the `Checker`
directly.
  • Loading branch information
charliermarsh authored Jul 3, 2023
1 parent 00fbbe4 commit ca497fa
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 90 deletions.
40 changes: 12 additions & 28 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,7 @@ where
..
}) => {
if self.enabled(Rule::DjangoNonLeadingReceiverDecorator) {
self.diagnostics
.extend(flake8_django::rules::non_leading_receiver_decorator(
decorator_list,
|expr| self.semantic.resolve_call_path(expr),
));
flake8_django::rules::non_leading_receiver_decorator(self, decorator_list);
}
if self.enabled(Rule::AmbiguousFunctionName) {
if let Some(diagnostic) =
Expand Down Expand Up @@ -505,8 +501,7 @@ where
}
}
if self.enabled(Rule::HardcodedPasswordDefault) {
self.diagnostics
.extend(flake8_bandit::rules::hardcoded_password_default(args));
flake8_bandit::rules::hardcoded_password_default(self, args);
}
if self.enabled(Rule::PropertyWithParameters) {
pylint::rules::property_with_parameters(self, stmt, decorator_list, args);
Expand Down Expand Up @@ -1573,9 +1568,7 @@ where
pyupgrade::rules::os_error_alias_handlers(self, handlers);
}
if self.enabled(Rule::PytestAssertInExcept) {
self.diagnostics.extend(
flake8_pytest_style::rules::assert_in_exception_handler(handlers),
);
flake8_pytest_style::rules::assert_in_exception_handler(self, handlers);
}
if self.enabled(Rule::SuppressibleException) {
flake8_simplify::rules::suppressible_exception(
Expand Down Expand Up @@ -1616,11 +1609,7 @@ where
flake8_bugbear::rules::assignment_to_os_environ(self, targets);
}
if self.enabled(Rule::HardcodedPasswordString) {
if let Some(diagnostic) =
flake8_bandit::rules::assign_hardcoded_password_string(value, targets)
{
self.diagnostics.push(diagnostic);
}
flake8_bandit::rules::assign_hardcoded_password_string(self, value, targets);
}
if self.enabled(Rule::GlobalStatement) {
for target in targets.iter() {
Expand Down Expand Up @@ -2615,8 +2604,7 @@ where
flake8_bandit::rules::jinja2_autoescape_false(self, func, args, keywords);
}
if self.enabled(Rule::HardcodedPasswordFuncArg) {
self.diagnostics
.extend(flake8_bandit::rules::hardcoded_password_func_arg(keywords));
flake8_bandit::rules::hardcoded_password_func_arg(self, keywords);
}
if self.enabled(Rule::HardcodedSQLExpression) {
flake8_bandit::rules::hardcoded_sql_expression(self, expr);
Expand Down Expand Up @@ -2871,16 +2859,13 @@ where
&self.settings.flake8_gettext.functions_names,
) {
if self.enabled(Rule::FStringInGetTextFuncCall) {
self.diagnostics
.extend(flake8_gettext::rules::f_string_in_gettext_func_call(args));
flake8_gettext::rules::f_string_in_gettext_func_call(self, args);
}
if self.enabled(Rule::FormatInGetTextFuncCall) {
self.diagnostics
.extend(flake8_gettext::rules::format_in_gettext_func_call(args));
flake8_gettext::rules::format_in_gettext_func_call(self, args);
}
if self.enabled(Rule::PrintfInGetTextFuncCall) {
self.diagnostics
.extend(flake8_gettext::rules::printf_in_gettext_func_call(args));
flake8_gettext::rules::printf_in_gettext_func_call(self, args);
}
}
if self.enabled(Rule::UncapitalizedEnvironmentVariables) {
Expand Down Expand Up @@ -3221,11 +3206,10 @@ where
flake8_2020::rules::compare(self, left, ops, comparators);
}
if self.enabled(Rule::HardcodedPasswordString) {
self.diagnostics.extend(
flake8_bandit::rules::compare_to_hardcoded_password_string(
left,
comparators,
),
flake8_bandit::rules::compare_to_hardcoded_password_string(
self,
left,
comparators,
);
}
if self.enabled(Rule::ComparisonWithItself) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustpython_parser::ast::{Arg, ArgWithDefault, Arguments, Expr, Ranged};

use crate::checkers::ast::Checker;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

Expand Down Expand Up @@ -36,9 +37,7 @@ fn check_password_kwarg(arg: &Arg, default: &Expr) -> Option<Diagnostic> {
}

/// S107
pub(crate) fn hardcoded_password_default(arguments: &Arguments) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = Vec::new();

pub(crate) fn hardcoded_password_default(checker: &mut Checker, arguments: &Arguments) {
for ArgWithDefault {
def,
default,
Expand All @@ -53,9 +52,7 @@ pub(crate) fn hardcoded_password_default(arguments: &Arguments) -> Vec<Diagnosti
continue;
};
if let Some(diagnostic) = check_password_kwarg(def, default) {
diagnostics.push(diagnostic);
checker.diagnostics.push(diagnostic);
}
}

diagnostics
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustpython_parser::ast::{Keyword, Ranged};

use crate::checkers::ast::Checker;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

Expand All @@ -22,10 +23,10 @@ impl Violation for HardcodedPasswordFuncArg {
}

/// S106
pub(crate) fn hardcoded_password_func_arg(keywords: &[Keyword]) -> Vec<Diagnostic> {
keywords
.iter()
.filter_map(|keyword| {
pub(crate) fn hardcoded_password_func_arg(checker: &mut Checker, keywords: &[Keyword]) {
checker
.diagnostics
.extend(keywords.iter().filter_map(|keyword| {
string_literal(&keyword.value).filter(|string| !string.is_empty())?;
let arg = keyword.arg.as_ref()?;
if !matches_password_name(arg) {
Expand All @@ -37,6 +38,5 @@ pub(crate) fn hardcoded_password_func_arg(keywords: &[Keyword]) -> Vec<Diagnosti
},
keyword.range(),
))
})
.collect()
}));
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use rustpython_parser::ast::{self, Constant, Expr, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

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

use super::super::helpers::{matches_password_name, string_literal};

#[violation]
Expand Down Expand Up @@ -47,12 +49,13 @@ fn password_target(target: &Expr) -> Option<&str> {

/// S105
pub(crate) fn compare_to_hardcoded_password_string(
checker: &mut Checker,
left: &Expr,
comparators: &[Expr],
) -> Vec<Diagnostic> {
comparators
.iter()
.filter_map(|comp| {
) {
checker
.diagnostics
.extend(comparators.iter().filter_map(|comp| {
string_literal(comp).filter(|string| !string.is_empty())?;
let Some(name) = password_target(left) else {
return None;
Expand All @@ -63,29 +66,29 @@ pub(crate) fn compare_to_hardcoded_password_string(
},
comp.range(),
))
})
.collect()
}));
}

/// S105
pub(crate) fn assign_hardcoded_password_string(
checker: &mut Checker,
value: &Expr,
targets: &[Expr],
) -> Option<Diagnostic> {
) {
if string_literal(value)
.filter(|string| !string.is_empty())
.is_some()
{
for target in targets {
if let Some(name) = password_target(target) {
return Some(Diagnostic::new(
checker.diagnostics.push(Diagnostic::new(
HardcodedPasswordString {
name: name.to_string(),
},
value.range(),
));
return;
}
}
}
None
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use rustpython_parser::ast::{self, Decorator, Expr, Ranged};
use rustpython_parser::ast::{Decorator, Ranged};

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

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

/// ## What it does
/// Checks that Django's `@receiver` decorator is listed first, prior to
Expand Down Expand Up @@ -48,25 +49,19 @@ impl Violation for DjangoNonLeadingReceiverDecorator {
}

/// DJ013
pub(crate) fn non_leading_receiver_decorator<'a, F>(
decorator_list: &'a [Decorator],
resolve_call_path: F,
) -> Vec<Diagnostic>
where
F: Fn(&'a Expr) -> Option<CallPath<'a>>,
{
let mut diagnostics = vec![];
pub(crate) fn non_leading_receiver_decorator(checker: &mut Checker, decorator_list: &[Decorator]) {
let mut seen_receiver = false;
for (i, decorator) in decorator_list.iter().enumerate() {
let is_receiver = match &decorator.expression {
Expr::Call(ast::ExprCall { func, .. }) => resolve_call_path(func)
let is_receiver = decorator.expression.as_call_expr().map_or(false, |call| {
checker
.semantic()
.resolve_call_path(&call.func)
.map_or(false, |call_path| {
matches!(call_path.as_slice(), ["django", "dispatch", "receiver"])
}),
_ => false,
};
})
});
if i > 0 && is_receiver && !seen_receiver {
diagnostics.push(Diagnostic::new(
checker.diagnostics.push(Diagnostic::new(
DjangoNonLeadingReceiverDecorator,
decorator.range(),
));
Expand All @@ -77,5 +72,4 @@ where
seen_receiver = true;
}
}
diagnostics
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use rustpython_parser::ast::{Expr, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

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

#[violation]
pub struct FStringInGetTextFuncCall;

Expand All @@ -14,11 +16,12 @@ impl Violation for FStringInGetTextFuncCall {
}

/// INT001
pub(crate) fn f_string_in_gettext_func_call(args: &[Expr]) -> Option<Diagnostic> {
pub(crate) fn f_string_in_gettext_func_call(checker: &mut Checker, args: &[Expr]) {
if let Some(first) = args.first() {
if first.is_joined_str_expr() {
return Some(Diagnostic::new(FStringInGetTextFuncCall {}, first.range()));
checker
.diagnostics
.push(Diagnostic::new(FStringInGetTextFuncCall {}, first.range()));
}
}
None
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use rustpython_parser::ast::{self, Expr, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

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

#[violation]
pub struct FormatInGetTextFuncCall;

Expand All @@ -14,15 +16,16 @@ impl Violation for FormatInGetTextFuncCall {
}

/// INT002
pub(crate) fn format_in_gettext_func_call(args: &[Expr]) -> Option<Diagnostic> {
pub(crate) fn format_in_gettext_func_call(checker: &mut Checker, args: &[Expr]) {
if let Some(first) = args.first() {
if let Expr::Call(ast::ExprCall { func, .. }) = &first {
if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() {
if attr == "format" {
return Some(Diagnostic::new(FormatInGetTextFuncCall {}, first.range()));
checker
.diagnostics
.push(Diagnostic::new(FormatInGetTextFuncCall {}, first.range()));
}
}
}
}
None
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged};

use crate::checkers::ast::Checker;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

Expand All @@ -14,7 +15,7 @@ impl Violation for PrintfInGetTextFuncCall {
}

/// INT003
pub(crate) fn printf_in_gettext_func_call(args: &[Expr]) -> Option<Diagnostic> {
pub(crate) fn printf_in_gettext_func_call(checker: &mut Checker, args: &[Expr]) {
if let Some(first) = args.first() {
if let Expr::BinOp(ast::ExprBinOp {
op: Operator::Mod { .. },
Expand All @@ -27,9 +28,10 @@ pub(crate) fn printf_in_gettext_func_call(args: &[Expr]) -> Option<Diagnostic> {
..
}) = left.as_ref()
{
return Some(Diagnostic::new(PrintfInGetTextFuncCall {}, first.range()));
checker
.diagnostics
.push(Diagnostic::new(PrintfInGetTextFuncCall {}, first.range()));
}
}
}
None
}
11 changes: 5 additions & 6 deletions crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ pub(crate) fn assert_falsy(checker: &mut Checker, stmt: &Stmt, test: &Expr) {
}

/// PT017
pub(crate) fn assert_in_exception_handler(handlers: &[ExceptHandler]) -> Vec<Diagnostic> {
handlers
.iter()
.flat_map(|handler| match handler {
pub(crate) fn assert_in_exception_handler(checker: &mut Checker, handlers: &[ExceptHandler]) {
checker
.diagnostics
.extend(handlers.iter().flat_map(|handler| match handler {
ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
name, body, ..
}) => {
Expand All @@ -239,8 +239,7 @@ pub(crate) fn assert_in_exception_handler(handlers: &[ExceptHandler]) -> Vec<Dia
Vec::new()
}
}
})
.collect()
}));
}

#[derive(Copy, Clone)]
Expand Down
Loading

0 comments on commit ca497fa

Please sign in to comment.