Improve handling of builtin symbols in linter rules (#10919)

Add a new method to the semantic model to simplify and improve the correctness of a common pattern
This commit is contained in:
Alex Waygood 2024-04-16 11:37:31 +01:00 committed by GitHub
parent effd5188c9
commit f779babc5f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
93 changed files with 886 additions and 588 deletions

View file

@ -37,7 +37,7 @@ pub fn classify(
semantic
.resolve_qualified_name(map_callable(expr))
.is_some_and( |qualified_name| {
matches!(qualified_name.segments(), ["", "type"] | ["abc", "ABCMeta"])
matches!(qualified_name.segments(), ["" | "builtins", "type"] | ["abc", "ABCMeta"])
})
})
|| decorator_list.iter().any(|decorator| is_class_method(decorator, semantic, classmethod_decorators))
@ -63,7 +63,7 @@ fn is_static_method(
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["", "staticmethod"] | ["abc", "abstractstaticmethod"]
["" | "builtins", "staticmethod"] | ["abc", "abstractstaticmethod"]
) || staticmethod_decorators
.iter()
.any(|decorator| qualified_name == QualifiedName::from_dotted_name(decorator))
@ -103,7 +103,7 @@ fn is_class_method(
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["", "classmethod"] | ["abc", "abstractclassmethod"]
["" | "builtins", "classmethod"] | ["abc", "abstractclassmethod"]
) || classmethod_decorators
.iter()
.any(|decorator| qualified_name == QualifiedName::from_dotted_name(decorator))

View file

@ -686,7 +686,7 @@ impl TypeChecker for IoBaseChecker {
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["io", "open" | "open_code"] | ["os" | "", "open"]
["io", "open" | "open_code"] | ["os" | "" | "builtins", "open"]
)
})
}

View file

@ -15,20 +15,16 @@ pub enum Visibility {
/// Returns `true` if a function is a "static method".
pub fn is_staticmethod(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
decorator_list.iter().any(|decorator| {
semantic
.resolve_qualified_name(map_callable(&decorator.expression))
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "staticmethod"]))
})
decorator_list
.iter()
.any(|decorator| semantic.match_builtin_expr(&decorator.expression, "staticmethod"))
}
/// Returns `true` if a function is a "class method".
pub fn is_classmethod(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
decorator_list.iter().any(|decorator| {
semantic
.resolve_qualified_name(map_callable(&decorator.expression))
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "classmethod"]))
})
decorator_list
.iter()
.any(|decorator| semantic.match_builtin_expr(&decorator.expression, "classmethod"))
}
/// Returns `true` if a function definition is an `@overload`.
@ -79,7 +75,7 @@ pub fn is_property(
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["", "property"] | ["functools", "cached_property"]
["" | "builtins", "property"] | ["functools", "cached_property"]
) || extra_properties
.iter()
.any(|extra_property| extra_property.segments() == qualified_name.segments())

View file

@ -251,12 +251,53 @@ impl<'a> SemanticModel<'a> {
}
/// Return `true` if `member` is bound as a builtin.
///
/// Note that a "builtin binding" does *not* include explicit lookups via the `builtins`
/// module, e.g. `import builtins; builtins.open`. It *only* includes the bindings
/// that are pre-populated in Python's global scope before any imports have taken place.
pub fn is_builtin(&self, member: &str) -> bool {
self.lookup_symbol(member)
.map(|binding_id| &self.bindings[binding_id])
.is_some_and(|binding| binding.kind.is_builtin())
}
/// If `expr` is a reference to a builtins symbol,
/// return the name of that symbol. Else, return `None`.
///
/// This method returns `true` both for "builtin bindings"
/// (present even without any imports, e.g. `open()`), and for explicit lookups
/// via the `builtins` module (e.g. `import builtins; builtins.open()`).
pub fn resolve_builtin_symbol<'expr>(&'a self, expr: &'expr Expr) -> Option<&'a str>
where
'expr: 'a,
{
// Fast path: we only need to worry about name expressions
if !self.seen_module(Modules::BUILTINS) {
let name = &expr.as_name_expr()?.id;
return if self.is_builtin(name) {
Some(name)
} else {
None
};
}
// Slow path: we have to consider names and attributes
let qualified_name = self.resolve_qualified_name(expr)?;
match qualified_name.segments() {
["" | "builtins", name] => Some(*name),
_ => None,
}
}
/// Return `true` if `expr` is a reference to `builtins.$target`,
/// i.e. either `object` (where `object` is not overridden in the global scope),
/// or `builtins.object` (where `builtins` is imported as a module at the top level)
pub fn match_builtin_expr(&self, expr: &Expr, symbol: &str) -> bool {
debug_assert!(!symbol.contains('.'));
self.resolve_builtin_symbol(expr)
.is_some_and(|name| name == symbol)
}
/// Return `true` if `member` is an "available" symbol, i.e., a symbol that has not been bound
/// in the current scope, or in any containing scope.
pub fn is_available(&self, member: &str) -> bool {
@ -1138,6 +1179,7 @@ impl<'a> SemanticModel<'a> {
pub fn add_module(&mut self, module: &str) {
match module {
"_typeshed" => self.seen.insert(Modules::TYPESHED),
"builtins" => self.seen.insert(Modules::BUILTINS),
"collections" => self.seen.insert(Modules::COLLECTIONS),
"dataclasses" => self.seen.insert(Modules::DATACLASSES),
"datetime" => self.seen.insert(Modules::DATETIME),
@ -1708,6 +1750,7 @@ bitflags! {
const TYPING_EXTENSIONS = 1 << 15;
const TYPESHED = 1 << 16;
const DATACLASSES = 1 << 17;
const BUILTINS = 1 << 18;
}
}