Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/pyrefly_config/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl Preset {
let errors = HashMap::from([
(ErrorKind::DirectAbstractBaseInstantiation, Severity::Error),
(ErrorKind::ImplicitAny, Severity::Error),
(ErrorKind::MethodAssign, Severity::Error),
(ErrorKind::MissingOverrideDecorator, Severity::Error),
(ErrorKind::OpenUnpacking, Severity::Error),
(ErrorKind::PotentialBadKeywordArgument, Severity::Error),
Expand Down
1 change: 1 addition & 0 deletions crates/pyrefly_config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3430,6 +3430,7 @@ output-format = "omit-errors"
errors.severity(ErrorKind::MissingOverrideDecorator),
Severity::Error
);
assert_eq!(errors.severity(ErrorKind::MethodAssign), Severity::Error);
assert_eq!(errors.severity(ErrorKind::OpenUnpacking), Severity::Error);
// Pyrefly infers concrete return types in most cases, so we don't
// ask users for an explicit annotation in strict mode.
Expand Down
3 changes: 3 additions & 0 deletions crates/pyrefly_config/src/error_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ pub enum ErrorKind {
/// Attempting to use `yield` in a way that is not allowed.
/// e.g. `yield from` with something that's not an iterable.
InvalidYield,
/// Assigning to a method on a class object or instance.
MethodAssign,
/// A file-level `# pyrefly: ignore-errors` (or `ignore-errors[code]`) directive
/// appears after the first line of code, where it is silently inert. File-level
/// suppressions are only honored in the preamble, at the top of the file.
Expand Down Expand Up @@ -585,6 +587,7 @@ impl ErrorKind {
ErrorKind::InvalidAbstractMethod => Severity::Ignore,
ErrorKind::InvalidCast => Severity::Ignore,
ErrorKind::InvalidDecorator => Severity::Warn,
ErrorKind::MethodAssign => Severity::Ignore,
ErrorKind::MisplacedIgnore => Severity::Warn,
ErrorKind::MissingAttributePatchTarget => Severity::Warn,
ErrorKind::MissingOverrideDecorator => Severity::Ignore,
Expand Down
25 changes: 25 additions & 0 deletions crates/pyrefly_config/src/migration/mypy/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,31 @@ allow_redefinition = true
Ok(())
}

#[test]
fn test_strict_enables_method_assign() -> anyhow::Result<()> {
let mut cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?;
cfg.configure();
assert_eq!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we remove method assign from this test so it only tests unannotated_defs?

cfg.errors(Path::new(".")).severity(ErrorKind::MethodAssign),
Severity::Error
);
Ok(())
}

#[test]
fn test_enable_method_assign_error() -> anyhow::Result<()> {
let src = r#"[tool.mypy]
enable_error_code = ["method-assign"]
"#;
let mut cfg = parse_pyproject_config(src)?;
cfg.configure();
assert_eq!(
cfg.errors(Path::new(".")).severity(ErrorKind::MethodAssign),
Severity::Error
);
Ok(())
}

#[test]
fn test_ignore_imports() -> anyhow::Result<()> {
let src = r#"[tool.mypy]
Expand Down
4 changes: 4 additions & 0 deletions crates/pyrefly_config/src/migration/mypy/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ pub fn make_error_config(
if disallow_any_generics || strict {
errors.insert(ErrorKind::ImplicitAny, Severity::Error);
}
if strict {
errors.insert(ErrorKind::MethodAssign, Severity::Error);
}
if disallow_any_explicit {
errors.insert(ErrorKind::ExplicitAny, Severity::Error);
}
Expand Down Expand Up @@ -210,6 +213,7 @@ fn code_to_kind(errors: HashMap<String, Severity>) -> HashMap<ErrorKind, Severit
"union-attr" | "attr-defined" => add(severity, ErrorKind::MissingAttribute),
"arg-type" => add(severity, ErrorKind::BadArgumentType),
"assignment" => add(severity, ErrorKind::BadAssignment),
"method-assign" => add(severity, ErrorKind::MethodAssign),
"call-arg" => add(severity, ErrorKind::BadArgumentCount),
"call-overload" => add(severity, ErrorKind::NoMatchingOverload),
"index" => {
Expand Down
180 changes: 119 additions & 61 deletions pyrefly/lib/alt/attr.rs

Large diffs are not rendered by default.

102 changes: 88 additions & 14 deletions pyrefly/lib/alt/class/class_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,13 @@ impl ClassField {
}
}

pub(crate) fn is_method(&self) -> bool {
matches!(
&self.0,
ClassFieldInner::Method { .. } | ClassFieldInner::ProxyMethod { .. }
)
}

pub fn is_final(&self) -> bool {
match &self.0 {
ClassFieldInner::Property { ty, .. } => ty.has_final_decoration(),
Expand Down Expand Up @@ -4816,8 +4823,22 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
}

pub fn get_instance_attribute(&self, cls: &ClassType, name: &Name) -> Option<ClassAttribute> {
self.get_instance_attribute_with_method(cls, name)
.map(|(attr, _)| attr)
}

pub fn get_instance_attribute_with_method(
&self,
cls: &ClassType,
name: &Name,
) -> Option<(ClassAttribute, bool)> {
self.get_class_member(cls.class_object(), name)
.map(|field| self.as_instance_attribute(name, field.as_ref(), &Instance::of_class(cls)))
.map(|field| {
let is_method = field.is_method();
let attr =
self.as_instance_attribute(name, field.as_ref(), &Instance::of_class(cls));
(attr, is_method)
})
}

pub fn get_self_attribute(&self, cls: &ClassType, name: &Name) -> Option<ClassAttribute> {
Expand Down Expand Up @@ -4886,7 +4907,18 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
metaclass: &ClassType,
name: &Name,
) -> Option<ClassAttribute> {
self.get_metaclass_attribute_with_method(cls, metaclass, name)
.map(|(attr, _)| attr)
}

pub fn get_metaclass_attribute_with_method(
&self,
cls: &ClassBase,
metaclass: &ClassType,
name: &Name,
) -> Option<(ClassAttribute, bool)> {
let attr = self.get_class_member(metaclass.class_object(), name)?;
let is_method = attr.is_method();
let attr = self.as_instance_attribute(
name,
attr.as_ref(),
Expand All @@ -4897,11 +4929,12 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
.class_object()
.has_toplevel_qname(ModuleName::builtins().as_str(), "type")
{
return Some(ClassAttribute::read_write(
self.constructor_to_callable(cls.class_type()),
return Some((
ClassAttribute::read_write(self.constructor_to_callable(cls.class_type())),
is_method,
));
}
Some(attr)
Some((attr, is_method))
}

/// Returns true if this attribute represents a data descriptor
Expand Down Expand Up @@ -4960,6 +4993,15 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
td: &TypedDictInner,
name: &Name,
) -> Option<ClassAttribute> {
self.get_typed_dict_attribute_with_method(td, name)
.map(|(attr, _)| attr)
}

pub fn get_typed_dict_attribute_with_method(
&self,
td: &TypedDictInner,
name: &Name,
) -> Option<(ClassAttribute, bool)> {
if let Some(meta) = self
.get_metadata_for_class(td.class_object())
.typed_dict_metadata()
Expand All @@ -4981,15 +5023,20 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
}
})
.map(|member| {
self.as_instance_attribute(
let is_method = member.value.is_method();
let attr = self.as_instance_attribute(
name,
member.value.as_ref(),
&Instance::of_typed_dict(td),
)
);
(attr, is_method)
});
}
self.get_class_member(td.class_object(), name).map(|field| {
self.as_instance_attribute(name, field.as_ref(), &Instance::of_typed_dict(td))
let is_method = field.is_method();
let attr =
self.as_instance_attribute(name, field.as_ref(), &Instance::of_typed_dict(td));
(attr, is_method)
})
}

Expand Down Expand Up @@ -5034,31 +5081,45 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
super_obj: &SuperObj,
name: &Name,
) -> Option<ClassAttribute> {
self.get_super_attribute_with_method(start_lookup_cls, super_obj, name)
.map(|(attr, _)| attr)
}

pub fn get_super_attribute_with_method(
&self,
start_lookup_cls: &ClassType,
super_obj: &SuperObj,
name: &Name,
) -> Option<(ClassAttribute, bool)> {
match super_obj {
SuperObj::Instance(obj) => self
.get_super_class_member(obj.class_object(), Some(start_lookup_cls), name)
.map(|member| {
let is_method = member.value.is_method();
if let Some(reason) = self.super_method_needs_impl_reason(&member) {
ClassAttribute::no_access(reason)
(ClassAttribute::no_access(reason), is_method)
} else {
self.as_instance_attribute(
let attr = self.as_instance_attribute(
name,
member.value.as_ref(),
&Instance::of_self_type(obj),
)
);
(attr, is_method)
}
}),
SuperObj::Class(obj) => self
.get_super_class_member(obj.class_object(), Some(start_lookup_cls), name)
.map(|member| {
let is_method = member.value.is_method();
if let Some(reason) = self.super_method_needs_impl_reason(&member) {
ClassAttribute::no_access(reason)
(ClassAttribute::no_access(reason), is_method)
} else {
self.as_class_attribute(
let attr = self.as_class_attribute(
name,
member.value.as_ref(),
&ClassBase::SelfType(obj.clone()),
)
);
(attr, is_method)
}
}),
}
Expand Down Expand Up @@ -5098,8 +5159,21 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
/// Access is disallowed for instance-only attributes and for attributes whose
/// type contains a class-scoped type parameter - e.g., `class A[T]: x: T`.
pub fn get_class_attribute(&self, cls: &ClassBase, name: &Name) -> Option<ClassAttribute> {
self.get_class_attribute_with_method(cls, name)
.map(|(attr, _)| attr)
}

pub fn get_class_attribute_with_method(
&self,
cls: &ClassBase,
name: &Name,
) -> Option<(ClassAttribute, bool)> {
self.get_class_member(cls.class_object(), name)
.map(|field| self.as_class_attribute(name, field.as_ref(), cls))
.map(|field| {
let is_method = field.is_method();
let attr = self.as_class_attribute(name, field.as_ref(), cls);
(attr, is_method)
})
}

pub fn get_bounded_quantified_class_attribute(
Expand Down
26 changes: 24 additions & 2 deletions pyrefly/lib/alt/class/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,19 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
metadata: &ClassMetadata,
attr_name: &Name,
) -> Option<ClassAttribute> {
self.get_enum_or_instance_attribute_with_method(class, metadata, attr_name)
.map(|(attr, _)| attr)
}

pub fn get_enum_or_instance_attribute_with_method(
&self,
class: &ClassType,
metadata: &ClassMetadata,
attr_name: &Name,
) -> Option<(ClassAttribute, bool)> {
self.special_case_enum_attr_lookup(class, None, metadata, attr_name)
.or_else(|| self.get_instance_attribute(class, attr_name))
.map(|attr| (attr, false))
.or_else(|| self.get_instance_attribute_with_method(class, attr_name))
}

/// Checks for a special-cased enum attribute on an enum literal, falling back to a regular instance attribute lookup.
Expand All @@ -165,9 +176,20 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
metadata: &ClassMetadata,
attr_name: &Name,
) -> Option<ClassAttribute> {
self.get_enum_literal_or_instance_attribute_with_method(lit, metadata, attr_name)
.map(|(attr, _)| attr)
}

pub fn get_enum_literal_or_instance_attribute_with_method(
&self,
lit: &LitEnum,
metadata: &ClassMetadata,
attr_name: &Name,
) -> Option<(ClassAttribute, bool)> {
let class = &lit.class;
self.special_case_enum_attr_lookup(class, Some(lit), metadata, attr_name)
.or_else(|| self.get_instance_attribute(class, attr_name))
.map(|attr| (attr, false))
.or_else(|| self.get_instance_attribute_with_method(class, attr_name))
}

/// Special-case enum attribute lookups. Dispatches to the appropriate helper
Expand Down
Loading
Loading