mirrors/ruff
0
0 Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2026-10-28 10:45:37 +08:00
Code Activity

[ty] Add rules that detect suspicious uses of Callable, Iterable, Iterator or Generator types in a boolean context (#28554)

Browse Source
This commit is contained in:
Alex Waygood authored and GitHub committed 2026-09-21 17:42:00 +01:00
1 parent 5bd42a7f22
commit 68073852bf
12 files changed
+2451 -327
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/ty/docs/rules.md
Generated
+279 -125
View File
File diff suppressed because it is too large. Load diff
crates/ty_python_semantic/mdtest.py.lock
+2 -1
View File
@@ -9,10 +9,11 @@ exclude-newer-span = "P7D"
[options.exclude-newer-package]
astral-dev-toolchain-hyperfine = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-insta = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-nextest = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-shear = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-codspeed = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-nextest = "2026-08-27T00:00:00Z"
astral-dev-toolchain-cargo-fuzz = "2026-09-01T00:00:00Z"
astral-dev-toolchain-cargo-deny = "2026-09-15T19:10:00Z"
[manifest]
requirements = [
crates/ty_python_semantic/resources/lint_docs/redundant-condition-strict.md
+7
View File
@@ -378,3 +378,10 @@ DEBUGGING = 0
if DEBUGGING: # error: [redundant-condition-strict]
print("Doing debugging stuff...")
```
## See also
- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables
are tested for their truthiness
- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables
are tested for their truthiness
crates/ty_python_semantic/resources/lint_docs/redundant-condition.md
+7
View File
@@ -167,3 +167,10 @@ def find_duplicate_coordinates(coordinates: list[tuple[int, int]]):
print(f"Duplicates are {duplicates}")
```
## See also
- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables
are tested for their truthiness
- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables
are tested for their truthiness
crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-callable.md
+36
View File
@@ -0,0 +1,36 @@
## What it does
Detects suspicious truthiness tests of `Callable`-typed values and unions of callable types.
## Why is this bad?
`Callable`-typed variables are nearly always functions in practice, and functions are always truthy.
If `predicate` is a variable inferred as having a `Callable` type, therefore, a boolean test such as
`if predicate:` is usually not what you want; `if predicate()` (or similar) is usually what was
intended.
## Examples
```py
from collections.abc import Callable
def announce_if_ready(is_ready: Callable[[], bool]):
if is_ready: # error: [truthiness-test-of-callable]
print("Ready")
```
You probably meant to call the value instead:
```py
def announce_if_ready_fixed(is_ready: Callable[[], bool]):
if is_ready(): # no diagnostic
print("Ready")
```
## See also
- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as
always being truthy or falsy
- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables
are tested for their truthiness
crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-iterable.md
+72
View File
@@ -0,0 +1,72 @@
## What it does
Detects boolean conditions where variables typed as `Iterable`, `Iterator`, `Generator` or similar
are tested for their truthiness.
## Why is this bad?
Testing an `Iterable` object for truthiness strongly suggests that the code expects the object to
evaluate as falsy in a boolean context if it is empty. However, even empty `Iterable` objects can be
truthy if they do not define `__len__` or `__bool__`. The classic example of this is a generator:
unevaluated generators in Python are always truthy, even if they do not yield any elements at
runtime.
## Examples
```py
from collections.abc import Iterable
def process(items: Iterable[int]):
if items: # error: [truthiness-test-of-iterable]
print("Received items")
else:
print("Didn't receive any items")
# prints "Received items", even though the passed-in generator is empty!
process(x for x in range(42) if x > 43)
```
If `process` in the above example does not need to accept generators, one solution is to rewrite the
annotation to use `Collection` instead of `Iterable`. `Collection` mandates that the object passed
in must define `__len__`, making a truthiness test much more likely to be meaningful:
```py
from collections.abc import Collection
def process(items: Collection[int]):
if items: # no diagnostic
print("Received items")
else:
print("Didn't receive any items")
# passing in a generator is now rejected:
# error: [invalid-argument-type] "Expected `Collection[int]`, found `GeneratorType[int, None, None]`"
process(x for x in range(42) if x > 43)
```
If the function must also accept generators, another solution can be to collect the iterable into a
tuple or list before testing its length:
```py
def process(items: Iterable[int]):
collected = tuple(items)
if collected: # no diagnostic
print("Received items")
else:
print("Didn't receive any items")
# correctly prints "Didn't receive any items"
process(x for x in range(42) if x > 43)
```
## See also
- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as
always being truthy or falsy
- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables
are tested for their truthiness
crates/ty_python_semantic/resources/mdtest/suspicious_conditions.md
+1331
View File
File diff suppressed because it is too large. Load diff
crates/ty_python_semantic/src/types/diagnostic.rs
+20
View File
@@ -195,6 +195,8 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
registry.register_lint(&INVALID_LEGACY_POSITIONAL_PARAMETER);
registry.register_lint(&REDUNDANT_CONDITION);
registry.register_lint(&REDUNDANT_CONDITION_STRICT);
registry.register_lint(&TRUTHINESS_TEST_OF_CALLABLE);
registry.register_lint(&TRUTHINESS_TEST_OF_ITERABLE);
// String annotations
registry.register_lint(&ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION);
@@ -1405,6 +1407,24 @@ declare_lint! {
}
}
declare_lint! {
#[doc = include_str!("../../resources/lint_docs/truthiness-test-of-callable.md")]
pub(crate) static TRUTHINESS_TEST_OF_CALLABLE = {
summary: "detects truthiness tests of `Callable`-typed objects",
status: LintStatus::stable("0.0.83"),
default_level: Level::Warn,
}
}
declare_lint! {
#[doc = include_str!("../../resources/lint_docs/truthiness-test-of-iterable.md")]
pub(crate) static TRUTHINESS_TEST_OF_ITERABLE = {
summary: "detects truthiness tests of `Iterable`-typed objects",
status: LintStatus::stable("0.0.83"),
default_level: Level::Warn,
}
}
/// A collection of type check diagnostics.
#[derive(Default, Eq, PartialEq, get_size2::GetSize)]
pub struct TypeCheckDiagnostics {
crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs
+436 -92
View File
@@ -4,8 +4,9 @@ use std::borrow::Cow;
use ruff_db::{
diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity},
parsed::parsed_module,
source::source_text,
files::{FilePath, FileRange},
parsed::{ParsedModuleRef, parsed_module, parsed_string_annotation},
source::{line_index, source_text},
};
use ruff_diagnostics::{Applicability, Edit, Fix};
use ruff_python_ast::{
@@ -20,9 +21,11 @@ use ty_module_resolver::{SearchPath, file_to_module};
use ty_python_core::{
Truthiness,
ast_ids::HasScopedUseId,
definition::DefinitionKind,
definition::{Definition, DefinitionKind},
place::PlaceExpr,
predicate::{Predicate, PredicateNode},
scope::FileScopeId,
semantic_index,
};
use crate::{
@@ -35,6 +38,7 @@ use crate::{
KnownClass, LintDiagnosticGuard, LintDiagnosticGuardBuilder, MemberLookupPolicy, Type,
TypeContext,
call::bind::CallableDescription,
context::InferContext,
diagnostic::typing_module_for_fix,
enum_metadata,
function::KnownFunction,
@@ -96,7 +100,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
let RedundantCondition {
expression: test,
value_type: test_type,
is_truthy,
truthiness,
kind,
} = condition;
@@ -117,7 +121,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
test_type.display(db, env)
));
} else {
let is_truthy = if *is_truthy { "truthy" } else { "falsy" };
let is_truthy = if truthiness.is_always_true() {
"truthy"
} else {
"falsy"
};
diagnostic.set_primary_annotation_message(format_args!(
"Inferred type `{}` is always {is_truthy}",
test_type.display(db, env)
@@ -127,6 +135,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
let describe_condition = |diagnostic: &mut LintDiagnosticGuard| {
let source = source_text(db, self.file());
let is_truthy = if truthiness.is_always_true() {
"true"
} else {
"false"
};
if source.contains_line_break(test.range()) {
diagnostic.set_concise_message(format_args!("Condition is always {is_truthy}"));
} else {
@@ -186,12 +199,233 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
// Short-circuit evaluation can determine a condition's truthiness even when its
// value type does not. In that case, describe the condition rather than the type.
let describe_as_condition =
test_type.is_subtype_of(db, env, KnownClass::Bool.to_instance(db, env))
|| test_type.bool(db, env) != Truthiness::from(*is_truthy);
let describe_as_condition = !truthiness.is_ambiguous()
&& (test_type.is_subtype_of(db, env, KnownClass::Bool.to_instance(db, env))
|| test_type.bool(db, env) != *truthiness);
let builder = self.context.report_lint(rule, test)?;
let diagnostic = if *is_truthy {
let diagnostic = if let ConditionKind::Callable(callables) = kind {
let mut diagnostic = builder.into_diagnostic("Suspicious boolean test of a `Callable`");
let source = source_text(db, self.file());
if source.contains_line_break(test.range()) {
diagnostic.set_concise_message(format_args!(
"Object of type `{}` might always be truthy (did you mean to call it?)",
test_type.display(db, env)
));
} else if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) {
diagnostic.set_concise_message(format_args!(
"Callable `{}` might always be truthy \
(has type `{}` -- did you mean to call it?)",
&source[test.range()],
test_type.display(db, env)
));
} else {
diagnostic.set_concise_message(format_args!(
"Expression `{}` of type `{}` might always be truthy \
(did you mean to call it?)",
&source[test.range()],
test_type.display(db, env)
));
}
diagnostic.set_primary_annotation_message(format_args!(
"`{}` object tested for truthiness",
test_type.display(db, env)
));
diagnostic
.info("Callable objects are usually functions, and functions are always truthy");
let should_await = self.should_await_call(
test,
callables.iter().flat_map(|callable| {
callable
.signatures(db)
.iter()
.map(|signature| signature.return_ty)
}),
);
if should_await {
diagnostic.help("Did you mean to call and await this callable?");
} else {
diagnostic.help("Did you mean to call this callable?");
}
let uncalled_function = UncalledFunction {
has_parameters: callables
.iter()
.any(|callable| callable.signatures(db).has_parameters()),
should_await,
};
uncalled_function.suggest_call(&self.context, &mut diagnostic, test);
diagnostic
} else if kind == &ConditionKind::Iterable {
let mut diagnostic =
builder.into_diagnostic("Suspicious boolean test of an `Iterable`");
let source = source_text(db, self.file());
if source.contains_line_break(test.range()) {
diagnostic.set_concise_message(format_args!(
"Object might be truthy even if its length is 0 (has type `{}`)",
test_type.display(db, env)
));
} else {
let kind = if test.is_name_expr() {
"Variable"
} else {
"Expression"
};
diagnostic.set_concise_message(format_args!(
"{kind} `{}` might be truthy even if its length is 0 (has type `{}`)",
&source[test.range()],
test_type.display(db, env)
));
}
diagnostic.set_primary_annotation_message(format_args!(
"`{}` object tested for truthiness",
test_type.display(db, env)
));
let definition_info =
condition_definition_info(db, self.program_file(), test, |expr| {
self.expression_type(expr)
});
let mut first_party_annotation = None;
if let Some(single_definition) = definition_info.single_definition {
let file = single_definition.python_file(db);
let module = parsed_module(db, file).load(db);
if let Some(annotation) =
self.matching_type_annotation(single_definition, &module, *test_type)
{
let file = single_definition.file(db);
diagnostic.annotate(
Annotation::secondary(Span::from(file).with_range(annotation.range()))
.message(format_args!(
"Inferred as `{}` due to this annotation",
test_type.display(db, env)
)),
);
if test_type
.known_specialization(db, env, KnownClass::Iterable)
.is_some()
&& self.is_first_party_definition(single_definition)
{
first_party_annotation = Some(IterableAnnotation {
range: FileRange::new(file, annotation.range()),
scope: semantic_index(db, single_definition.program_file(db))
.expression_scope_id(annotation),
iterable_ranges: self
.iterable_annotation_ranges(single_definition, annotation),
});
}
}
}
diagnostic.info(format_args!(
"`{}` objects can be generators, \
and generators are truthy even when empty",
test_type.display(db, env)
));
if let Some(IterableAnnotation {
range,
scope,
iterable_ranges,
}) = first_party_annotation
{
let line = line_index(db, range.file()).line_index(range.start());
let collection_module = if env.python_version(db) >= PythonVersion::PY39 {
"collections.abc"
} else {
"typing"
};
let annotation_source = source_text(db, range.file());
let suggestion = iterable_ranges.map(|ranges| &annotation_source[ranges.element]);
if range.file() == self.file() {
if let Some(suggestion) = suggestion {
diagnostic.help(format_args!(
"Use `{collection_module}.Collection[{suggestion}]` as the annotation \
on line {line}"
));
if let Some(ranges) = iterable_ranges
&& let Some(action) = self.context.importer().import_for_diagnostic(
ImportRequest::import_from(collection_module, "Collection"),
scope,
range.start(),
)
{
diagnostic.set_fix(Fix::unsafe_edits(
Edit::range_replacement(
action.symbol_text().to_string(),
ranges.origin,
),
action.import().cloned(),
));
}
} else {
diagnostic.help(format_args!(
"Consider reworking the annotation on line {line} \
to use `{collection_module}.Collection`",
));
}
} else {
let path = range.file().path(db);
let rendered_path = match path {
FilePath::System(path) => path
.strip_prefix(db.system().current_directory())
.unwrap_or(path)
.as_str(),
FilePath::Vendored(_) | FilePath::SystemVirtual(_) => path.as_str(),
};
if let Some(suggestion) = suggestion {
diagnostic.help(format_args!(
"Consider using `{collection_module}.Collection[{suggestion}]` \
in the annotation on line {line} of {rendered_path}"
));
} else {
diagnostic.help(format_args!(
"Consider reworking the annotation on line {line} of {rendered_path} \
to use `{collection_module}.Collection`",
));
}
}
diagnostic.info(
"A `Collection` must define `__len__`, \
so `Collection` excludes generators",
);
diagnostic.help(
"Alternatively, test the length of the iterable \
instead of its truthiness",
);
} else {
diagnostic.help("Test the length of the iterable instead of its truthiness");
}
if diagnostic.fix().is_none() {
let semantic_model = SemanticModel::new(db, self.program_file());
let test_ref = ast::AnyNodeRef::from(*test);
if semantic_model.definitely_has_builtin_binding("len", test_ref)
&& semantic_model.definitely_has_builtin_binding("tuple", test_ref)
{
diagnostic.set_fix(Fix::display_only_edits(
Edit::insertion("len(tuple(".to_string(), test.start()),
[Edit::insertion("))".to_string(), test.end())],
));
}
}
diagnostic
} else if truthiness.is_always_true() {
let add_always_truthy_concise_message = |diagnostic: &mut LintDiagnosticGuard| {
if should_quote_test_expression()
&& let source = source_text(db, self.file())
@@ -242,31 +476,17 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
let mut diagnostic =
builder.into_diagnostic(format_args!("{function} is always truthy"));
// Add a suggestion and fix that they might have meant to call (and possibly
// also await) this function.
//
// It's true that calling the function might not actually fix this diagnostic
// if the function returns something that is always truthy. They still probably
// meant to call the function, though, so it's still a useful suggestion/fix!
// A coroutine return type establishes that calling and awaiting the function
// is appropriate. `Any`, `Unknown`, and `Never` do not establish this, even
// though they are assignable to `CoroutineType`.
// Use the top materialization so the unspecified generic arguments do not
// prevent concrete coroutine types from being subtypes.
let coroutine = KnownClass::CoroutineType
.to_instance(db, env)
.top_materialization(db, env);
let is_awaitable_coro_function = self.can_await_here(test)
&& function.signature().iter().all(|signature| {
!signature.return_ty.is_equivalent_to(db, env, Type::Never)
&& signature.return_ty.is_subtype_of(db, env, coroutine)
});
let should_await = self.should_await_call(
test,
function
.signature()
.iter()
.map(|signature| signature.return_ty),
);
let kind = function.kind();
if is_awaitable_coro_function {
if should_await {
diagnostic.set_primary_annotation_message(format_args!(
"Did you mean to `await` and call this {kind}?",
));
@@ -276,35 +496,12 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
));
}
if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) {
let (call, applicability) = if function.signature().has_parameters() {
("(...)", Applicability::DisplayOnly)
} else {
("()", Applicability::Unsafe)
};
let call_edit = Edit::insertion(call.to_string(), test.end());
let uncalled_function = UncalledFunction {
has_parameters: function.signature().has_parameters(),
should_await,
};
let fix = if is_awaitable_coro_function {
Fix::applicable_edits(
Edit::insertion("await ".to_string(), test.start()),
[call_edit],
applicability,
)
} else {
Fix::applicable_edit(call_edit, applicability)
};
let source = source_text(db, self.file());
let expression_text = &source[test.range()];
let prefix = if is_awaitable_coro_function {
"await "
} else {
""
};
diagnostic.help(format_args!(
"Replace with `{prefix}{expression_text}{call}`"
));
diagnostic.set_fix(fix);
}
uncalled_function.suggest_call(&self.context, &mut diagnostic, test);
diagnostic
} else if let Some(tuple_spec) = test_type.tuple_instance_spec(db, env)
@@ -687,6 +884,82 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
diagnostic
}
/// Find a definition's annotation if it matches the type inferred at the use site.
fn matching_type_annotation<'ast>(
&self,
definition: Definition<'db>,
module: &'ast ParsedModuleRef,
inferred_type: Type<'db>,
) -> Option<&'ast ast::Expr> {
let db = self.db();
let annotation = match definition.kind(db) {
DefinitionKind::AnnotatedAssignment(assignment) => assignment.annotation(module),
DefinitionKind::Parameter(parameter) => parameter.annotation(module)?,
_ => return None,
};
(self.annotation_expression_type(definition, annotation)? == inferred_type)
.then_some(annotation)
}
fn annotation_expression_type(
&self,
definition: Definition<'db>,
expression: &ast::Expr,
) -> Option<Type<'db>> {
let db = self.db();
match definition.kind(db) {
DefinitionKind::AnnotatedAssignment(_) => {
infer_definition_types(db, definition).try_expression_type(expression)
}
DefinitionKind::Parameter(_) => {
let scope = definition.scope(db).scope(db).parent()?;
let scope_id = scope.to_scope_id(db, definition.program_file(db));
infer_scope_types(db, scope_id, TypeContext::default())
.try_expression_type(expression)
}
_ => None,
}
}
/// Locate the iterable name and its element annotation so a fix can change only the name.
/// Keeping the rest of the source preserves aliases, forward references, and version-specific
/// syntax. Parsed string annotations retain source offsets, including nested quotes.
fn iterable_annotation_ranges(
&self,
definition: Definition<'db>,
annotation: &ast::Expr,
) -> Option<IterableAnnotationRanges> {
let db = self.db();
match annotation {
ast::Expr::StringLiteral(string) => {
let source = source_text(db, definition.file(db));
let parsed =
parsed_string_annotation(&source, string.as_single_part_string()?).ok()?;
self.iterable_annotation_ranges(definition, parsed.expr())
}
ast::Expr::Subscript(subscript) => {
let known_class = self
.annotation_expression_type(definition, &subscript.value)?
.as_class_literal()?
.known(db)?;
(known_class == KnownClass::Iterable).then_some(IterableAnnotationRanges {
origin: subscript.value.range(),
element: subscript.slice.range(),
})
}
_ => None,
}
}
fn is_first_party_definition(&self, definition: Definition<'db>) -> bool {
let db = self.db();
definition.file(db) == self.file()
|| file_to_module(db, definition.program_file(db).resolver_file(db))
.and_then(|module| module.search_path(db))
.is_some_and(SearchPath::is_first_party)
}
fn diagnose_single_length_tuple(
&self,
length: TupleLength,
@@ -727,31 +1000,9 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
if let Some(single_definition) = definition_info.single_definition {
let file = single_definition.python_file(db);
let program_file = single_definition.program_file(db);
let module = parsed_module(db, file).load(db);
let annotation_info = match single_definition.kind(db) {
DefinitionKind::AnnotatedAssignment(assignment) => {
let annotation = assignment.annotation(&module);
infer_definition_types(db, single_definition)
.try_expression_type(annotation)
.map(|annotation_type| (annotation, annotation_type))
}
DefinitionKind::Parameter(parameter) => {
parameter.annotation(&module).and_then(|annotation| {
let scope = single_definition.scope(db).scope(db).parent()?;
let annotation_type = infer_scope_types(
db,
scope.to_scope_id(db, program_file),
TypeContext::default(),
)
.try_expression_type(annotation)?;
Some((annotation, annotation_type))
})
}
_ => None,
};
if let Some((annotation, annotation_type)) = annotation_info
&& annotation_type == node_type
if let Some(annotation) =
self.matching_type_annotation(single_definition, &module, node_type)
{
let file = single_definition.file(db);
let diagnostic_annotation =
@@ -767,19 +1018,13 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
.to_string_parts();
if suggested_type.is_valid_syntax {
let resolver_file = single_definition.program_file(db).resolver_file(db);
let annotated_in_first_party_code = file == self.file()
|| file_to_module(db, resolver_file)
.and_then(|module| module.search_path(db))
.is_some_and(SearchPath::is_first_party);
let maybe_star = if annotation.is_starred_expr() {
"*"
} else {
""
};
let annotation = if annotated_in_first_party_code {
let annotation = if self.is_first_party_definition(single_definition) {
diagnostic_annotation().message(format_args!(
"Did you mean `{maybe_star}{}`?",
suggested_type.label
@@ -798,6 +1043,32 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
}
}
/// Whether every callable return type is known to be awaitable in this context.
fn should_await_call(
&self,
expression: &ast::Expr,
return_types: impl IntoIterator<Item = Type<'db>>,
) -> bool {
if !self.can_await_here(expression) {
return false;
}
let db = self.db();
let env = self.program_environment();
// Use the top materialization so concrete return types can be subtypes regardless of
// the awaitable's generic arguments. `Any`, `Unknown`, and `Never` do not establish
// that a call returns an awaitable.
let awaitable = KnownClass::Awaitable
.to_instance(db, env)
.top_materialization(db, env);
return_types.into_iter().all(|return_ty| {
!return_ty.is_equivalent_to(db, env, Type::Never)
&& return_ty.is_subtype_of(db, env, awaitable)
})
}
/// Return a [`TextRange`] spanning from `branch_start` up to and including
/// the offset of the first newline character after the start of `first_statement`.
///
@@ -1007,7 +1278,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
let RedundantCondition {
expression: test,
value_type: _,
is_truthy: _,
truthiness: _,
kind,
} = condition;
@@ -1070,7 +1341,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
}
if !implicit_else_is_unreachable
&& *kind == ConditionKind::Boolean
&& kind.is_boolean()
&& let Some(clause) = if_stmt.elif_else_clauses.last()
&& clause.test.as_ref() == Some(test)
&& !diagnostic.has_applicable_fix(Applicability::DisplayOnly)
@@ -1337,3 +1608,76 @@ fn logical_line_end(source: &str, tokens: &Tokens, offset: TextSize) -> TextSize
fn first_nontrivial_statement(suite: &[ast::Stmt]) -> Option<&ast::Stmt> {
suite.iter().find(|stmt| !is_trivial_statement(stmt))
}
#[derive(Debug, Clone, Copy)]
struct UncalledFunction {
has_parameters: bool,
should_await: bool,
}
impl UncalledFunction {
/// Add a suggestion and fix that they might have meant to call (and possibly
/// also await) this function.
///
/// It's true that calling the function might not actually fix this diagnostic
/// if the function returns something that is always truthy. They still probably
/// meant to call the function, though, so it's still a useful suggestion/fix!
fn suggest_call(self, context: &InferContext, diagnostic: &mut Diagnostic, test: &ast::Expr) {
if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) {
let (call, applicability) = if self.has_parameters {
("(...)", Applicability::DisplayOnly)
} else {
("()", Applicability::Unsafe)
};
let call_edit = Edit::insertion(call.to_string(), test.end());
let prefix = if self.should_await { "await " } else { "" };
let fix = if self.should_await {
Fix::applicable_edits(
Edit::insertion(prefix.to_string(), test.start()),
[call_edit],
applicability,
)
} else {
Fix::applicable_edit(call_edit, applicability)
};
let source = source_text(context.db(), context.file());
diagnostic.help(format_args!(
"Replace with `{prefix}{}{call}`",
&source[test.range()]
));
diagnostic.set_fix(fix);
}
}
}
/// A first-party `Iterable` annotation that can be replaced with `Collection`.
#[derive(Debug)]
struct IterableAnnotation {
range: FileRange,
/// The scope where the annotation's names are resolved. Import validation uses this scope
/// because names can be shadowed differently at the truthiness test.
scope: FileScopeId,
iterable_ranges: Option<IterableAnnotationRanges>,
}
/// Source ranges for replacing `Iterable` while preserving the element annotation.
///
/// In this function's stringized parameter annotation, `origin` covers `Iterable` inside the
/// string, and `element` covers `P` inside the string:
///
/// ```python
/// from collections.abc import Iterable
/// from pathlib import Path as P
///
/// def check(items: "Iterable[P]"):
/// if items:
/// print("Received paths")
/// ```
///
/// The fix adds a `Collection` import and replaces `origin`, producing `"Collection[P]"` while
/// preserving the original quotes and the alias `P`.
#[derive(Debug, Clone, Copy)]
struct IterableAnnotationRanges {
origin: TextRange,
element: TextRange,
}
crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/exemptions.rs
+1 -1
View File
@@ -190,7 +190,7 @@ impl RedundantConditionContext {
);
is_boolean_or_short_circuit
&& if condition.is_truthy {
&& if condition.truthiness.is_always_true() {
falsy_branch
} else {
truthy_branch
crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/mod.rs
+238 -106
View File
@@ -1,5 +1,5 @@
//! Logic for reporting boolean tests that are unintentionally always truthy or always falsy.
//! These may be reported under either `redundant-condition` or `redundant-condition-strict`.
//! Logic for reporting boolean tests with fixed truthiness, or suspicious tests of values typed as
//! `Callable` or `Iterable`.
//!
//! This module classifies tests and selects which expressions to report. [`exemptions`] handles
//! assertions, defensive branches, and environment checks; [`diagnostic`] builds messages and fixes.
@@ -25,11 +25,11 @@
//!
//! The whole condition is also always truthy, but reporting the fixed truthiness of the outer
//! condition as well the fixed truthiness of its subexpressions would flag the same mistake
//! twice. To choose which expression to report, the redundant-condition checker therefore needs
//! to see the context of the complete condition. We therefore wait until the
//! [`TypeInferenceBuilder`] has inferred the whole condition before checking any subexpression
//! boolean tests within it. See [`TypeInferenceBuilder::check_condition_redundancy`] for how a
//! check of a smaller expression is postponed until that point.
//! twice. To choose which expression to report, we therefore need to see the context of the
//! complete condition when checking subexpressions. To do this, we wait until the
//! [`TypeInferenceBuilder`] has inferred types for the whole condition before checking any
//! subexpression boolean tests within it. See [`TypeInferenceBuilder::check_condition_redundancy`]
//! for how a check of a smaller expression is postponed until that point.
//!
//! A boolean test can also occur inside an expression that is being passed to a function or
//! stored in a variable. For example, `not ready` below tests the truthiness of `ready` while
@@ -45,10 +45,16 @@
//!
//! consume(not ready) # `redundant-condition` on `ready`.
//! ```
//!
//! Note that while `redundant-condition` on a subexpression can suppress a
//! `redundant-condition-strict` warning on an outer expression, `truthiness-test-of-iterable`
//! can only suppress `truthiness-test-of-iterable` warnings on an outer expression. The same is
//! true for `truthiness-test-of-callable`.
mod diagnostic;
mod exemptions;
use bitflags::bitflags;
use ruff_python_ast::{
self as ast,
helpers::any_over_expr,
@@ -58,22 +64,28 @@ use ruff_text_size::Ranged;
use ty_python_core::{Truthiness, expression::ExpressionContext, predicate::StatementCall};
use crate::{
Db,
lint::LintMetadata,
reachability::{analyze_condition_expression, is_non_terminal_call},
types::{
KnownClass, KnownInstanceType, Type,
diagnostic::{REDUNDANT_CONDITION, REDUNDANT_CONDITION_STRICT},
CallableTypes, KnownClass, KnownInstanceType, Type,
constraints::ConstraintSetBuilder,
diagnostic::{
REDUNDANT_CONDITION, REDUNDANT_CONDITION_STRICT, TRUTHINESS_TEST_OF_CALLABLE,
TRUTHINESS_TEST_OF_ITERABLE,
},
infer::TypeInferenceBuilder,
typevar::TypeVarSet,
},
};
use self::exemptions::RedundantConditionContext;
/// Classification of a redundant condition.
/// Classification of a boolean test.
///
/// This is used to determine which diagnostic rule would apply to the condition.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
enum ConditionKind {
#[derive(Debug, PartialEq, Eq)]
enum ConditionKind<'db> {
/// A condition whose value type is assignable to `int`, including `bool`.
///
/// These tests commonly enforce runtime invariants, so they are only flagged by the
@@ -127,18 +139,34 @@ enum ConditionKind {
/// print("ready")
/// ```
Value,
/// A value that does not have a known truthiness,
/// but is nonetheless suspicious in a boolean context because it is a subtype
/// of `Iterable[object]` and a supertype of `GeneratorType[Never]`.
Iterable,
/// A value with ambiguous truthiness whose type is `Callable`, or a union in which
/// every member is either a `Callable` type or another callable type known to be
/// always truthy, such as a function or bound method.
Callable(CallableTypes<'db>),
}
impl ConditionKind {
impl ConditionKind<'_> {
/// Return the rule responsible for reporting this category of redundant condition.
const fn rule(self) -> &'static LintMetadata {
const fn rule(&self) -> &'static LintMetadata {
match self {
Self::Value => &REDUNDANT_CONDITION,
Self::Iterable => &TRUTHINESS_TEST_OF_ITERABLE,
Self::Callable(_) => &TRUTHINESS_TEST_OF_CALLABLE,
Self::Boolean | Self::ShortCircuit | Self::ContainsWalrus => {
&REDUNDANT_CONDITION_STRICT
}
}
}
const fn is_boolean(&self) -> bool {
matches!(self, Self::Boolean)
}
}
/// An already-inferred boolean test.
@@ -254,7 +282,7 @@ impl BooleanTest<'_, '_> {
condition: &RedundantCondition<'_, '_>,
) -> Truthiness {
if expression.range() == condition.expression.range() {
return Truthiness::from(condition.is_truthy);
return condition.truthiness;
}
match expression {
@@ -291,21 +319,21 @@ impl BooleanTest<'_, '_> {
}
}
/// A condition with known truthiness, and the rule category needed to report it.
/// A classified boolean test and the rule category needed to report it.
///
/// We may or may not eventually report a diagnostic for this condition! A condition is classified
/// before we examine the context the condition occurs in, so an instance of this struct represents
/// a "diagnostic candidate" rather than a guarantee that a diagnostic will be emitted.
/// A test is classified before its context is considered, so this struct represents a diagnostic
/// candidate rather than a guarantee that a diagnostic will be emitted.
///
/// The value type is retained for diagnostic messages. The `is_truthy` field describes the condition's
/// outcome, which can depend on short-circuit evaluation as well as the value type (see the doc-comment
/// for [`ExpressionContext`] for more details).
/// The value type is retained for diagnostic messages. The `truthiness` field records what can be
/// inferred about the test's outcome, which can depend on short-circuit evaluation as well as the
/// value type (see the doc-comment for [`ExpressionContext`] for more details). It remains
/// ambiguous for suspicious `Callable` and `Iterable` tests.
#[derive(Debug)]
struct RedundantCondition<'ast, 'db> {
expression: &'ast ast::Expr,
value_type: Type<'db>,
is_truthy: bool,
kind: ConditionKind,
truthiness: Truthiness,
kind: ConditionKind<'db>,
}
/// Determination of whether `redundant-condition-strict` diagnostics should be reported
@@ -323,40 +351,26 @@ enum BooleanDiagnosticPreference {
EnclosingCondition,
}
/// The result of checking a boolean test, indicating whether an expression containing that test
/// should also be checked for redundancy.
///
/// This is returned by [`TypeInferenceBuilder::check_boolean_test`] to prevent duplicate diagnostics
/// on an expression and its subexpressions. A condition can suppress an enclosing diagnostic even
/// when its own diagnostic is disabled, exempt, or ignored.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
enum ConditionCheckResult {
/// The containing boolean or conditional expression can still be reported.
///
/// For example, the boolean result of `isinstance()` is not reported as a subexpression when
/// the complete `not` expression has fixed truthiness:
///
/// ```python
/// def check(value: str):
/// if not isinstance(value, str): # `redundant-condition-strict` on the whole condition.
/// print("unreachable")
/// ```
CheckEnclosingCondition,
bitflags! {
/// Biset representing the rules on an outer expression that can be suppressed by a diagnostic
/// on an inner subexpression.
#[derive(Clone, Copy, Debug)]
struct ConditionCheckResult: u8 {
/// A fixed-truthiness operand takes precedence over the entire enclosing test.
const SUPPRESS_ALL = 1 << 0;
/// The containing boolean or conditional expression should not be reported.
///
/// For example, reporting that `ready` is always truthy suppresses a second diagnostic saying
/// that `not ready` is always false. This also applies if the diagnostic on `ready` is ignored
/// or its rule is disabled:
///
/// ```python
/// def ready() -> bool:
/// return True
///
/// if not ready: # Only `ready` is reported, under `redundant-condition`.
/// print("unreachable")
/// ```
SuppressEnclosingCondition,
/// A suspicious `Callable` operand suppresses only an enclosing callable diagnostic.
const SUPPRESS_CALLABLE = 1 << 1;
/// A suspicious `Iterable` operand suppresses only an enclosing iterable diagnostic.
const SUPPRESS_ITERABLE = 1 << 2;
}
}
impl ConditionCheckResult {
const fn suppresses(self, enclosing: Self) -> bool {
self.contains(Self::SUPPRESS_ALL) || self.intersects(enclosing)
}
}
impl<'db> TypeInferenceBuilder<'db, '_> {
@@ -368,14 +382,21 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
/// - It's a stub file
/// - We're inside a string annotation. Its expressions are absent from the semantic index,
/// so we cannot group nested tests to avoid duplicate diagnostics.
/// - Neither `redundant-condition` nor `redundant-condition-strict` is enabled
/// in the user's configuration.
/// - None of the relevant rules are enabled in the user's configuration
fn should_check_redundant_conditions(&self) -> bool {
static RELEVANT_RULES: &[&LintMetadata] = &[
&REDUNDANT_CONDITION,
&REDUNDANT_CONDITION_STRICT,
&TRUTHINESS_TEST_OF_CALLABLE,
&TRUTHINESS_TEST_OF_ITERABLE,
];
!self.in_string_annotation()
&& self.db().should_check_file(self.file())
&& !self.file().is_stub(self.db())
&& (self.context.is_lint_enabled(&REDUNDANT_CONDITION)
|| self.context.is_lint_enabled(&REDUNDANT_CONDITION_STRICT))
&& RELEVANT_RULES
.iter()
.any(|rule| self.context.is_lint_enabled(rule))
}
/// Check a condition, for which types have already been inferred, to see if it is redundant.
@@ -641,38 +662,78 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
/// Classify a condition using its inferred type and truthiness.
///
/// We return `None` if we cannot determine whether the test is always truthy or always falsy.
/// We also return `None` for literal boolean and integer expressions: a condition written as
/// `True`, `False`, `1`, or `0` is almost certainly deliberate. A variable with an inferred
/// literal type is still classified, however, because its truthiness may be less obvious to
/// the author:
/// We return `Some` if the condition is always truthy, always falsy, or otherwise suspicious
/// in a boolean context due to it being a `Callable` or `Iterable` type. Exemptions are made
/// for literal boolean and integer expressions: although these can always be determined to be
/// either always truthy or always falsy, a condition written as `True`, `False`, `1`, or `0`
/// is almost certainly deliberate. A variable with an inferred literal type is still classified,
/// however, because its truthiness may be less obvious to the author:
///
/// ```python
/// from typing import Literal
/// from typing import Literal, Callable, Iterable
///
/// def check(flag: bool, known: Literal[True]):
/// if flag: # No classification: the outcome is unknown.
/// def check(
/// flag: bool,
/// known: Literal[True],
/// callback: Callable[[], bool],
/// items: Iterable[int]
/// ):
/// if flag: # No classification: ambiguous truthiness without a suspicious type.
/// pass
/// if True: # No classification: a literal boolean expression.
///
/// if True: # No classification: a literal boolean expression.
/// pass
/// if 1: # No classification: a literal integer expression.
///
/// if 1: # No classification: a literal integer expression.
/// pass
/// if known: # `ConditionKind::Boolean`: the value type is boolean.
///
/// if known: # `ConditionKind::Boolean`: the value type is boolean.
/// pass
///
/// if callback: # `ConditionKind::Callable`: ambiguous truthiness.
/// pass
///
/// if items: # `ConditionKind::Iterable`: ambiguous truthiness.
/// pass
/// ```
fn classify_redundant_condition<'expr>(
&self,
test: BooleanTest<'expr, 'db>,
) -> Option<RedundantCondition<'expr, 'db>> {
/// The bottom specialization for `GeneratorType`
static BOTTOM_SPEC: &[Type] = &[Type::Never, Type::object(), Type::Never];
/// The top specialization for `Iterable`
static TOP_SPEC: &[Type] = &[Type::object()];
/// Fast path for conditions with ambiguous truthiness,
/// so that we do not have to materialize `GeneratorType` specializations
/// for common cases such as `bool`, `TypeIs` and `TypeGuard` conditions, etc.
fn cannot_satisfy_iterable_check<'db>(db: &'db dyn Db, ty: Type<'db>) -> bool {
match ty {
Type::NominalInstance(_)
| Type::TypeGuard(_)
| Type::TypeIs(_)
| Type::Dynamic(_)
| Type::LiteralValue(_) => true,
Type::Intersection(intersection) => intersection
.positive(db)
.iter()
.any(|&item| cannot_satisfy_iterable_check(db, item)),
Type::Union(union) => union
.elements(db)
.iter()
.all(|&item| cannot_satisfy_iterable_check(db, item)),
_ => false,
}
}
let BooleanTest {
expression,
value_type,
truthiness,
..
} = test;
if truthiness.is_ambiguous() {
return None;
}
if matches!(
expression,
@@ -691,7 +752,68 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
let db = self.db();
let env = self.program_environment();
let kind = if value_type.is_assignable_to(db, env, KnownClass::Int.to_instance(db, env)) {
let kind = if truthiness.is_ambiguous() {
let expanded = match value_type.resolve_type_alias(db) {
Type::Union(union) if union.has_aliases(db) => union.expand_aliases(db, env),
ty => ty,
};
let callables = match expanded {
Type::Callable(callable) => Some(CallableTypes::one(callable)),
Type::Union(union) => {
let mut emit_diagnostic = true;
let union_elements = union.elements(db);
let mut callable_elements = Vec::with_capacity(union_elements.len());
for element in union_elements {
if let Type::Callable(callable) = element {
callable_elements.push(*callable);
} else if element.bool(db, env).is_always_true()
&& let Some(callables) = element.try_upcast_to_callable(db, env)
{
callable_elements.extend(&callables);
} else {
emit_diagnostic = false;
break;
}
}
if emit_diagnostic {
Some(CallableTypes::from_elements(callable_elements))
} else {
None
}
}
_ => None,
};
if let Some(callables) = callables {
ConditionKind::Callable(callables)
} else {
if cannot_satisfy_iterable_check(db, value_type) {
return None;
}
let builder = ConstraintSetBuilder::new();
let generator =
KnownClass::GeneratorType.to_specialized_instance(db, env, BOTTOM_SPEC);
let check = |source: Type<'db>, target: Type<'db>| {
source.when_subtype_of(db, env, target, &builder, TypeVarSet::None)
};
if check(generator, value_type)
.and(db, &builder, || {
let iterable =
KnownClass::Iterable.to_specialized_instance(db, env, TOP_SPEC);
check(value_type, iterable)
})
.is_always_satisfied(db, env)
{
ConditionKind::Iterable
} else {
return None;
}
}
} else if value_type.is_assignable_to(db, env, KnownClass::Int.to_instance(db, env)) {
ConditionKind::Boolean
} else if value_type.bool(db, env).is_ambiguous() {
ConditionKind::ShortCircuit
@@ -705,7 +827,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
Some(RedundantCondition {
expression,
value_type,
is_truthy: truthiness.is_always_true(),
truthiness,
kind,
})
}
@@ -746,9 +868,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
///
/// Independent tests within subexpressions are included in the same result.
///
/// If the truthiness of the complete condition is fixed, we only report a single diagnostic
/// for the entire condition. If its truthiness is ambiguous, however, subexpressions with
/// fixed truthiness can be reported individually:
/// When an operand and the complete condition both have fixed truthiness, we select one
/// diagnostic for the mistake. Suspicious `Callable` and `Iterable` operands can instead be
/// reported alongside a complete condition with fixed truthiness. If the complete condition
/// has ambiguous truthiness, subexpressions with fixed truthiness can be reported individually:
///
/// ```python
/// def check(value: int, flag: bool):
@@ -775,22 +898,22 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
/// Check `test` and the boolean tests contributing to its outcome.
///
/// Diagnostic selection follows two rules:
/// Diagnostic selection follows these rules:
///
/// - Within a condition with fixed truthiness, boolean and short-circuit operands are not
/// reported. The enclosing condition represents their redundancy, even if its diagnostic
/// is exempt or ignored.
/// - Other operands, such as uncalled functions, take precedence over their enclosing
/// conditions because they identify the likely mistake more directly.
/// - Other operands with fixed truthiness, such as uncalled functions, take precedence over
/// their enclosing conditions because they identify the likely mistake more directly.
/// - Suspicious `Callable` and `Iterable` operands suppress only their own rule on enclosing
/// expressions, leaving the other suspicious rule and fixed-truthiness rules eligible.
///
/// The first rule is decided before visiting operands, since the whole condition's truthiness
/// is already known. The second is decided after visiting them, using [`ConditionCheckResult`].
/// is already known. The others are decided after visiting them, using [`ConditionCheckResult`].
///
/// Return [`ConditionCheckResult::SuppressEnclosingCondition`] if a caller checking a boolean
/// or conditional expression containing `test` should suppress the diagnostic on that larger
/// expression. For example, the code below receives a diagnostic on `ready` because the
/// function object is always truthy. It does not also receive a diagnostic saying that
/// `not ready` is always false:
/// A fixed-truthiness operand can suppress a diagnostic on the enclosing expression. For
/// example, the code below receives a diagnostic on `ready` because the function object is
/// always truthy. It does not also receive a diagnostic saying that `not ready` is always false:
///
/// ```python
/// def ready() -> bool:
@@ -800,11 +923,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
/// print("unreachable")
/// ```
///
/// Checking `ready` returns [`ConditionCheckResult::SuppressEnclosingCondition`], so the call
/// checking `not ready` knows not to report a diagnostic regarding the larger expression.
/// Checking `ready` returns [`ConditionCheckResult::SUPPRESS_ALL`], so the call checking
/// `not ready` does not report a diagnostic on the larger expression. A suspicious `Callable`
/// or `Iterable` test instead suppresses only the same rule on enclosing expressions.
///
/// Return the same result if the selected diagnostic is disabled or ignored. Disabling the
/// diagnostic on `ready` should not cause a different diagnostic on `not ready` to appear.
/// Conditions selected for reporting are appended to `conditions` in traversal order.
fn check_boolean_test<'ast>(
&self,
@@ -819,18 +941,16 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
BooleanDiagnosticPreference::EnclosingCondition
};
let mut operand_result = ConditionCheckResult::CheckEnclosingCondition;
let mut operand_result = ConditionCheckResult::empty();
let mut check_operand = |expression: &'ast ast::Expr, context, conditions: &mut Vec<_>| {
if self.check_boolean_test(
let result = self.check_boolean_test(
self.boolean_test(expression, test.evaluation),
context,
operand_preference,
conditions,
) == ConditionCheckResult::SuppressEnclosingCondition
{
operand_result = ConditionCheckResult::SuppressEnclosingCondition;
}
);
operand_result |= result;
};
match test.expression {
@@ -873,7 +993,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
),
}
if operand_result == ConditionCheckResult::SuppressEnclosingCondition {
if operand_result.contains(ConditionCheckResult::SUPPRESS_ALL) {
return operand_result;
}
@@ -884,11 +1004,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
// Boolean tests against `ty_extensions._internal.ConstraintSet` are (hopefully)
// only going to occur in the context of our test suite, and it's very annoying
// if we report them as redundant.
return ConditionCheckResult::SuppressEnclosingCondition;
return ConditionCheckResult::SUPPRESS_ALL;
}
let Some(condition) = self.classify_redundant_condition(test) else {
return ConditionCheckResult::CheckEnclosingCondition;
return operand_result;
};
if preference == BooleanDiagnosticPreference::EnclosingCondition
@@ -897,7 +1017,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
ConditionKind::Boolean | ConditionKind::ShortCircuit
)
{
return ConditionCheckResult::CheckEnclosingCondition;
return operand_result;
}
// An unreachable operand can retain a literal type, as in `False and "yes"`.
@@ -909,16 +1029,28 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
.context
.is_range_reachable(condition.expression.range())
{
return ConditionCheckResult::CheckEnclosingCondition;
return operand_result;
}
let result = match &condition.kind {
ConditionKind::Callable(_) => ConditionCheckResult::SUPPRESS_CALLABLE,
ConditionKind::Iterable => ConditionCheckResult::SUPPRESS_ITERABLE,
ConditionKind::Boolean
| ConditionKind::ContainsWalrus
| ConditionKind::ShortCircuit
| ConditionKind::Value => ConditionCheckResult::SUPPRESS_ALL,
};
if operand_result.suppresses(result) {
return operand_result;
}
let rule = condition.kind.rule();
if self.context.is_lint_enabled(rule) && !condition_context.exempts(self, &condition) {
conditions.push(condition);
}
ConditionCheckResult::SuppressEnclosingCondition
operand_result | result
}
/// Find and check boolean tests within subexpressions of `test`. This includes tests inside
ty.schema.json
Generated
+22 -2
View File
@@ -1386,7 +1386,7 @@
},
"redundant-condition": {
"title": "detects conditions that are always truthy or always falsey",
"description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false due to the inferred type of the condition.\n\nThis rule is enabled by default, and is deliberately not comprehensive. In order to avoid false\npositives, it excludes conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- The boolean test can be inferred as always evaluating to `True` and `False`, but this inference is\n due to boolean-test short-circuiting in `if` conditions, `while` conditions or `assert` tests\n rather than the inferred type of the boolean test.\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error that triggers this rule is to forget to call a function, for example:\n\n```py\nimport random\n\n\ndef should_do_action() -> bool:\n return random.choice([True, False])\n\n\n# oops! You forgot the parentheses here... this should have been `if should_do_action()`.\n# Because it's not, this will always be `True`:\nif should_do_action: # error: [redundant-condition]\n print(\"Doing stuff...\")\n```\n\nAnother common mistake is to forget to `await` a coroutine:\n\n```py\nimport random\n\n\nasync def should_do_async_action():\n return random.choice([True, False])\n\n\nasync def main():\n # oops! Forgot the await here... this should have been `if await should_do_async_action()`.\n # Because it's not, this will always be `True`:\n if should_do_async_action(): # error: [redundant-condition]\n print(\"Doing stuff async...\")\n```\n\nOr to forget that `tuple[X]` means \"A tuple with exactly one element\" rather than \"a tuple with an\narbitrary number of elements\" (for which you'd use `tuple[X, ...]`):\n\n```py\n# you almost certainly meant to write `tuple[str, ...]` here rather than `tuple[str]`...\ndef consume_tuples(x: tuple[str]):\n # ...and that means that this later condition is inferred as always being True by ty:\n if x: # error: [redundant-condition]\n print(\"Got a non-empty tuple\")\n```\n\nSome Pythonistas fall into the trap of thinking that a generator expression will be falsy if it has\nzero elements inside it -- but generator expressions are lazy, and so they're always truthy unless\nyou collect them into a tuple:\n\n```py\ndef test_my_data(data: list[int]):\n # this will always be `True`, because the asserted object is a `types.GeneratorType` instance,\n # not a `tuple`! `assert any(item for item in data if item > 42)`\n # is probably what you meant instead.\n assert (item for item in data if item > 42) # error: [redundant-condition]\n```\n\n## Boolean operators used to compute values\n\nThe rule checks `and` and `or` operands when the expression is used as a condition: in an `if`,\n`elif`, `while`, or `assert` test, a conditional expression, a comprehension filter, a match guard,\nor as the operand of `not`. It does not flag `and` or `or` expressions used to compute values --\neven if an operand in an `and` or `or` expression is always truthy, it doesn't necessarily make the\nexpression redundant:\n\n```py\ndef f(): ...\ndef g(): ...\n\n\ndef test(coinflip: bool):\n # could also be written as `func = f if coinflip else g`,\n # but use of an `and` expression for this is common in older codebases.\n func = coinflip and f or g\n\n # `func` will be the `f` function if `coinflip` is `True`,\n # and the `g` function otherwise\n func()\n```\n\nThis also allows calls that are deliberately always falsy but are used for their side effects:\n\n```py\nfrom unittest.mock import patch\n\n\ndef ask_to_continue() -> bool:\n return input(\"Continue? \") == \"yes\"\n\n\ndef test_ask_to_continue():\n prompts = []\n with patch(\n \"builtins.input\",\n side_effect=lambda prompt: prompts.append(prompt) or \"yes\",\n ):\n assert ask_to_continue()\n\n assert prompts == [\"Continue? \"]\n```\n\nBy contrast, `not` always produces a boolean, so we will still emit a diagnostic on the following\nexample -- negating the truthiness of a function object is pointless, since a function object is\nalways truthy:\n\n```py\ndef f(): ...\n\n\nvalue = not f # error: [redundant-condition]\n```\n\n## Known issues and workarounds\n\nThis rule can sometimes trigger on code that is not incorrect, but could be written in a clearer\nway. For example, the rule will flag this codLine truncated
"description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false due to the inferred type of the condition.\n\nThis rule is enabled by default, and is deliberately not comprehensive. In order to avoid false\npositives, it excludes conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- The boolean test can be inferred as always evaluating to `True` and `False`, but this inference is\n due to boolean-test short-circuiting in `if` conditions, `while` conditions or `assert` tests\n rather than the inferred type of the boolean test.\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error that triggers this rule is to forget to call a function, for example:\n\n```py\nimport random\n\n\ndef should_do_action() -> bool:\n return random.choice([True, False])\n\n\n# oops! You forgot the parentheses here... this should have been `if should_do_action()`.\n# Because it's not, this will always be `True`:\nif should_do_action: # error: [redundant-condition]\n print(\"Doing stuff...\")\n```\n\nAnother common mistake is to forget to `await` a coroutine:\n\n```py\nimport random\n\n\nasync def should_do_async_action():\n return random.choice([True, False])\n\n\nasync def main():\n # oops! Forgot the await here... this should have been `if await should_do_async_action()`.\n # Because it's not, this will always be `True`:\n if should_do_async_action(): # error: [redundant-condition]\n print(\"Doing stuff async...\")\n```\n\nOr to forget that `tuple[X]` means \"A tuple with exactly one element\" rather than \"a tuple with an\narbitrary number of elements\" (for which you'd use `tuple[X, ...]`):\n\n```py\n# you almost certainly meant to write `tuple[str, ...]` here rather than `tuple[str]`...\ndef consume_tuples(x: tuple[str]):\n # ...and that means that this later condition is inferred as always being True by ty:\n if x: # error: [redundant-condition]\n print(\"Got a non-empty tuple\")\n```\n\nSome Pythonistas fall into the trap of thinking that a generator expression will be falsy if it has\nzero elements inside it -- but generator expressions are lazy, and so they're always truthy unless\nyou collect them into a tuple:\n\n```py\ndef test_my_data(data: list[int]):\n # this will always be `True`, because the asserted object is a `types.GeneratorType` instance,\n # not a `tuple`! `assert any(item for item in data if item > 42)`\n # is probably what you meant instead.\n assert (item for item in data if item > 42) # error: [redundant-condition]\n```\n\n## Boolean operators used to compute values\n\nThe rule checks `and` and `or` operands when the expression is used as a condition: in an `if`,\n`elif`, `while`, or `assert` test, a conditional expression, a comprehension filter, a match guard,\nor as the operand of `not`. It does not flag `and` or `or` expressions used to compute values --\neven if an operand in an `and` or `or` expression is always truthy, it doesn't necessarily make the\nexpression redundant:\n\n```py\ndef f(): ...\ndef g(): ...\n\n\ndef test(coinflip: bool):\n # could also be written as `func = f if coinflip else g`,\n # but use of an `and` expression for this is common in older codebases.\n func = coinflip and f or g\n\n # `func` will be the `f` function if `coinflip` is `True`,\n # and the `g` function otherwise\n func()\n```\n\nThis also allows calls that are deliberately always falsy but are used for their side effects:\n\n```py\nfrom unittest.mock import patch\n\n\ndef ask_to_continue() -> bool:\n return input(\"Continue? \") == \"yes\"\n\n\ndef test_ask_to_continue():\n prompts = []\n with patch(\n \"builtins.input\",\n side_effect=lambda prompt: prompts.append(prompt) or \"yes\",\n ):\n assert ask_to_continue()\n\n assert prompts == [\"Continue? \"]\n```\n\nBy contrast, `not` always produces a boolean, so we will still emit a diagnostic on the following\nexample -- negating the truthiness of a function object is pointless, since a function object is\nalways truthy:\n\n```py\ndef f(): ...\n\n\nvalue = not f # error: [redundant-condition]\n```\n\n## Known issues and workarounds\n\nThis rule can sometimes trigger on code that is not incorrect, but could be written in a clearer\nway. For example, the rule will flag this codLine truncated
"default": "warn",
"oneOf": [
{
@@ -1396,7 +1396,7 @@
},
"redundant-condition-strict": {
"title": "detects conditions that are always truthy or always falsey (strict)",
"description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false.\n\nThis rule is disabled by default. It exclusively covers cases that its sibling (enabled-by-default)\nrule `redundant-condition` does not cover. These cases often flag real bugs in user code, but also\nhave a significantly higher rate of unavoidable false positives than other cases.\n\nThis rule reports redundant conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- Short-circuit evaluation means the condition can be guaranteed to be always truthy or always falsy\n despite fixed truthiness not being guaranteed by the inferred type of the expression's value\n (see \"Short-circuiting boolean conditions\" below for an example).\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error in Python code is to make the mistake of thinking that indexing into a `bytes` object\nwill get you an object of type `bytes`. But `bytes` work differently to `str`s in Python -- although\na string is a sequence of strings, a bytestring is a sequence of `int`s, so indexing into a `bytes`\nobject gives you an `int`. This rule can catch that error by alerting you to the fact that checking\nwhether a `bytes` object is unequal to an `int` will always evaluate to `True`:\n\n```py\ndef validate_record(data: bytes) -> None:\n if data[0] != b\"\\x1e\": # error: [redundant-condition-strict]\n raise ValueError(\"Invalid record separator\")\n```\n\nAnother common mistake is to assume that annotating `**kwargs` with `dict[str, str]` describes the\ndictionary containing the keyword arguments. In fact, a `**kwargs` annotation describes each\nindividual keyword argument, so this annotation says that every value is itself a dictionary.\nComparing one of those values with a string will therefore always evaluate to `False`:\n\n```py\ndef trace(**kwargs: dict[str, str]) -> None:\n if kwargs.get(\"operation\") == \"task\": # error: [redundant-condition-strict]\n print(\"Tracing task\")\n```\n\n## Short-circuiting boolean conditions\n\nIn some situations, ty can know that a condition will always be true, or it can know that a\ncondition will always be false, even when this is not guaranteed by the inferred type of that\ncondition. This is because of the way that Python short-circuits evaluation of conditions in the\ncontext of `if` tests, `while` tests and `assert` statements.\n\nConsider a class whose comparison method has an `object` return type:\n\n```py\nfrom typing_extensions import reveal_type\n\n\nclass Comparable:\n def __lt__(self, other: int) -> object: ...\n\n\ndef check(value: Comparable):\n reveal_type(value < 1 < 0) # revealed: ~AlwaysTruthy\n\n if value < 1 < 0: # error: [redundant-condition-strict] \"always false\"\n pass\n```\n\nOutside the context of an `if` test, the revealed type of the condition here is `~AlwaysTruthy`: in\nother words, ty knows that this expression is not *always true*, but cannot guarantee that it is\ndefinitely *always false*. It could be an object that is sometimes true and sometimes false -- for\nexample, a `list` (which is falsy when it is empty, and truthy otherwise).\n\nNonetheless, when `value < 1 < 0` is used directly as a condition, ty knows that the condition will\nalways be falsy and the `if` branch will never be taken. Python tests the truthiness of the object\nreturned by `Comparable.__lt__` once: if it is falsy, the condition fails immediately. If it is\ntruthy, Python evaluates `1 < 0`, which is false. There is no second truthiness test of the object\nreturned by `__lt__`.\n\nIf the chained comparison is saved as a variable first, its value can be the object returned by\n`__lt__`, if that object was falsy when first tested. The `if result` statement then tests that\nobject's truthiness again. A user-defined `__bool__` method can return a different result on that\nsecond call, so ty cannot guarantee that the saved value is still falsy, and no diagnostic is\nemitted:\n\n```py\ndef check_saved(value: Comparable):\n result = value < 1 < 0\n if result: # no diagnostic\n pass\n```\n\n## Exemptions\n\nLike `redundant-condition`, this rule checks subexpressions of an `and` or `or` expression only when\nthe outer expression is used as a condition. This is to avoid emitting false-positive diagnostics on\ncLine truncated
"description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false.\n\nThis rule is disabled by default. It exclusively covers cases that its sibling (enabled-by-default)\nrule `redundant-condition` does not cover. These cases often flag real bugs in user code, but also\nhave a significantly higher rate of unavoidable false positives than other cases.\n\nThis rule reports redundant conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- Short-circuit evaluation means the condition can be guaranteed to be always truthy or always falsy\n despite fixed truthiness not being guaranteed by the inferred type of the expression's value\n (see \"Short-circuiting boolean conditions\" below for an example).\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error in Python code is to make the mistake of thinking that indexing into a `bytes` object\nwill get you an object of type `bytes`. But `bytes` work differently to `str`s in Python -- although\na string is a sequence of strings, a bytestring is a sequence of `int`s, so indexing into a `bytes`\nobject gives you an `int`. This rule can catch that error by alerting you to the fact that checking\nwhether a `bytes` object is unequal to an `int` will always evaluate to `True`:\n\n```py\ndef validate_record(data: bytes) -> None:\n if data[0] != b\"\\x1e\": # error: [redundant-condition-strict]\n raise ValueError(\"Invalid record separator\")\n```\n\nAnother common mistake is to assume that annotating `**kwargs` with `dict[str, str]` describes the\ndictionary containing the keyword arguments. In fact, a `**kwargs` annotation describes each\nindividual keyword argument, so this annotation says that every value is itself a dictionary.\nComparing one of those values with a string will therefore always evaluate to `False`:\n\n```py\ndef trace(**kwargs: dict[str, str]) -> None:\n if kwargs.get(\"operation\") == \"task\": # error: [redundant-condition-strict]\n print(\"Tracing task\")\n```\n\n## Short-circuiting boolean conditions\n\nIn some situations, ty can know that a condition will always be true, or it can know that a\ncondition will always be false, even when this is not guaranteed by the inferred type of that\ncondition. This is because of the way that Python short-circuits evaluation of conditions in the\ncontext of `if` tests, `while` tests and `assert` statements.\n\nConsider a class whose comparison method has an `object` return type:\n\n```py\nfrom typing_extensions import reveal_type\n\n\nclass Comparable:\n def __lt__(self, other: int) -> object: ...\n\n\ndef check(value: Comparable):\n reveal_type(value < 1 < 0) # revealed: ~AlwaysTruthy\n\n if value < 1 < 0: # error: [redundant-condition-strict] \"always false\"\n pass\n```\n\nOutside the context of an `if` test, the revealed type of the condition here is `~AlwaysTruthy`: in\nother words, ty knows that this expression is not *always true*, but cannot guarantee that it is\ndefinitely *always false*. It could be an object that is sometimes true and sometimes false -- for\nexample, a `list` (which is falsy when it is empty, and truthy otherwise).\n\nNonetheless, when `value < 1 < 0` is used directly as a condition, ty knows that the condition will\nalways be falsy and the `if` branch will never be taken. Python tests the truthiness of the object\nreturned by `Comparable.__lt__` once: if it is falsy, the condition fails immediately. If it is\ntruthy, Python evaluates `1 < 0`, which is false. There is no second truthiness test of the object\nreturned by `__lt__`.\n\nIf the chained comparison is saved as a variable first, its value can be the object returned by\n`__lt__`, if that object was falsy when first tested. The `if result` statement then tests that\nobject's truthiness again. A user-defined `__bool__` method can return a different result on that\nsecond call, so ty cannot guarantee that the saved value is still falsy, and no diagnostic is\nemitted:\n\n```py\ndef check_saved(value: Comparable):\n result = value < 1 < 0\n if result: # no diagnostic\n pass\n```\n\n## Exemptions\n\nLike `redundant-condition`, this rule checks subexpressions of an `and` or `or` expression only when\nthe outer expression is used as a condition. This is to avoid emitting false-positive diagnostics on\ncLine truncated
"default": "ignore",
"oneOf": [
{
@@ -1474,6 +1474,26 @@
}
]
},
"truthiness-test-of-callable": {
"title": "detects truthiness tests of `Callable`-typed objects",
"description": "## What it does\n\nDetects suspicious truthiness tests of `Callable`-typed values and unions of callable types.\n\n## Why is this bad?\n\n`Callable`-typed variables are nearly always functions in practice, and functions are always truthy.\nIf `predicate` is a variable inferred as having a `Callable` type, therefore, a boolean test such as\n`if predicate:` is usually not what you want; `if predicate()` (or similar) is usually what was\nintended.\n\n## Examples\n\n```py\nfrom collections.abc import Callable\n\n\ndef announce_if_ready(is_ready: Callable[[], bool]):\n if is_ready: # error: [truthiness-test-of-callable]\n print(\"Ready\")\n```\n\nYou probably meant to call the value instead:\n\n```py\ndef announce_if_ready_fixed(is_ready: Callable[[], bool]):\n if is_ready(): # no diagnostic\n print(\"Ready\")\n```\n\n## See also\n\n- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as\n always being truthy or falsy\n- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables\n are tested for their truthiness",
"default": "warn",
"oneOf": [
{
"$ref": "#/definitions/Level"
}
]
},
"truthiness-test-of-iterable": {
"title": "detects truthiness tests of `Iterable`-typed objects",
"description": "## What it does\n\nDetects boolean conditions where variables typed as `Iterable`, `Iterator`, `Generator` or similar\nare tested for their truthiness.\n\n## Why is this bad?\n\nTesting an `Iterable` object for truthiness strongly suggests that the code expects the object to\nevaluate as falsy in a boolean context if it is empty. However, even empty `Iterable` objects can be\ntruthy if they do not define `__len__` or `__bool__`. The classic example of this is a generator:\nunevaluated generators in Python are always truthy, even if they do not yield any elements at\nruntime.\n\n## Examples\n\n```py\nfrom collections.abc import Iterable\n\n\ndef process(items: Iterable[int]):\n if items: # error: [truthiness-test-of-iterable]\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# prints \"Received items\", even though the passed-in generator is empty!\nprocess(x for x in range(42) if x > 43)\n```\n\nIf `process` in the above example does not need to accept generators, one solution is to rewrite the\nannotation to use `Collection` instead of `Iterable`. `Collection` mandates that the object passed\nin must define `__len__`, making a truthiness test much more likely to be meaningful:\n\n```py\nfrom collections.abc import Collection\n\n\ndef process(items: Collection[int]):\n if items: # no diagnostic\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# passing in a generator is now rejected:\n# error: [invalid-argument-type] \"Expected `Collection[int]`, found `GeneratorType[int, None, None]`\"\nprocess(x for x in range(42) if x > 43)\n```\n\nIf the function must also accept generators, another solution can be to collect the iterable into a\ntuple or list before testing its length:\n\n```py\ndef process(items: Iterable[int]):\n collected = tuple(items)\n if collected: # no diagnostic\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# correctly prints \"Didn't receive any items\"\nprocess(x for x in range(42) if x > 43)\n```\n\n## See also\n\n- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as\n always being truthy or falsy\n- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables\n are tested for their truthiness",
"default": "warn",
"oneOf": [
{
"$ref": "#/definitions/Level"
}
]
},
"type-assertion-failure": {
"title": "detects failed type assertions",
"description": "## What it does\n\nChecks for `assert_type()` and `assert_never()` calls where the actual type is not the same as the\nasserted type.\n\n## Why is this bad?\n\n`assert_type()` allows confirming the inferred type of a certain value.\n\n## Example\n\n```toml\n[environment]\npython-version = \"3.11\"\n```\n\n```python\nfrom typing import assert_type\n\n\ndef _(x: int):\n assert_type(x, int) # fine\n # Actual type does not match asserted type\n assert_type(x, str) # error\n```",