Skip to content

Commit

Permalink
Update function-call-in-argument-default (B008) to ignore argumen…
Browse files Browse the repository at this point in the history
…ts with immutable annotations (#6784)

Extends #6781 
Part of #3762
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->
Allows calls in argument defaults if the argument is annotated as an
immutable type to avoid false positives.

## Test Plan

<!-- How was it tested? -->

Snapshots
  • Loading branch information
zanieb authored Aug 24, 2023
1 parent 948cd29 commit 3bd199c
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 192 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ def timedelta_okay(value=dt.timedelta(hours=1)):
def path_okay(value=Path(".")):
pass

# B008 allow arbitrary call with immutable annotation
def immutable_annotation_call(value: Sequence[int] = foo()):
pass

# B006 and B008
# We should handle arbitrary nesting of these B008.
def nested_combo(a=[float(3), dt.datetime.now()]):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import List

import fastapi
import custom
from fastapi import Query


Expand All @@ -16,5 +17,9 @@ def okay(data: List[str] = Query(None)):
...


def okay(data: custom.ImmutableTypeA = foo()):
...


def error_due_to_missing_import(data: List[str] = Depends(None)):
...
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_bugbear/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ mod tests {
extend_immutable_calls: vec![
"fastapi.Depends".to_string(),
"fastapi.Query".to_string(),
"custom.ImmutableTypeA".to_string(),
],
},
..Settings::for_rule(Rule::FunctionCallInDefaultArgument)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::{compose_call_path, from_qualified_name, CallPath};
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_semantic::analyze::typing::{is_immutable_func, is_mutable_func};
use ruff_python_semantic::analyze::typing::{
is_immutable_annotation, is_immutable_func, is_mutable_func,
};
use ruff_python_semantic::SemanticModel;

use crate::checkers::ast::Checker;
Expand All @@ -20,6 +22,13 @@ use crate::checkers::ast::Checker;
/// once, at definition time. The returned value will then be reused by all
/// calls to the function, which can lead to unexpected behaviour.
///
/// Calls can be marked as an exception to this rule with the
/// [`flake8-bugbear.extend-immutable-calls`] configuration option.
///
/// Arguments with immutable type annotations will be ignored by this rule.
/// Types outside of the standard library can be marked as immutable with the
/// [`flake8-bugbear.extend-immutable-calls`] configuration option as well.
///
/// ## Example
/// ```python
/// def create_list() -> list[int]:
Expand Down Expand Up @@ -60,14 +69,14 @@ impl Violation for FunctionCallInDefaultArgument {
}
}

struct ArgumentDefaultVisitor<'a> {
semantic: &'a SemanticModel<'a>,
extend_immutable_calls: Vec<CallPath<'a>>,
struct ArgumentDefaultVisitor<'a, 'b> {
semantic: &'a SemanticModel<'b>,
extend_immutable_calls: &'a [CallPath<'b>],
diagnostics: Vec<(DiagnosticKind, TextRange)>,
}

impl<'a> ArgumentDefaultVisitor<'a> {
fn new(semantic: &'a SemanticModel<'a>, extend_immutable_calls: Vec<CallPath<'a>>) -> Self {
impl<'a, 'b> ArgumentDefaultVisitor<'a, 'b> {
fn new(semantic: &'a SemanticModel<'b>, extend_immutable_calls: &'a [CallPath<'b>]) -> Self {
Self {
semantic,
extend_immutable_calls,
Expand All @@ -76,15 +85,12 @@ impl<'a> ArgumentDefaultVisitor<'a> {
}
}

impl<'a, 'b> Visitor<'b> for ArgumentDefaultVisitor<'b>
where
'b: 'a,
{
fn visit_expr(&mut self, expr: &'b Expr) {
impl Visitor<'_> for ArgumentDefaultVisitor<'_, '_> {
fn visit_expr(&mut self, expr: &Expr) {
match expr {
Expr::Call(ast::ExprCall { func, .. }) => {
if !is_mutable_func(func, self.semantic)
&& !is_immutable_func(func, self.semantic, &self.extend_immutable_calls)
&& !is_immutable_func(func, self.semantic, self.extend_immutable_calls)
{
self.diagnostics.push((
FunctionCallInDefaultArgument {
Expand Down Expand Up @@ -114,25 +120,28 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter
.iter()
.map(|target| from_qualified_name(target))
.collect();
let diagnostics = {
let mut visitor = ArgumentDefaultVisitor::new(checker.semantic(), extend_immutable_calls);
for ParameterWithDefault {
default,
parameter: _,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
if let Some(expr) = &default {

let mut visitor = ArgumentDefaultVisitor::new(checker.semantic(), &extend_immutable_calls);
for ParameterWithDefault {
default,
parameter,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
if let Some(expr) = &default {
if !parameter.annotation.as_ref().is_some_and(|expr| {
is_immutable_annotation(expr, checker.semantic(), &extend_immutable_calls)
}) {
visitor.visit_expr(expr);
}
}
visitor.diagnostics
};
for (check, range) in diagnostics {
}

for (check, range) in visitor.diagnostics {
checker.diagnostics.push(Diagnostic::new(check, range));
}
}
Loading

0 comments on commit 3bd199c

Please sign in to comment.