[ty] Better error message for attempting to assign to a read-only property

crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -522,6 +522,18 @@ c.name = None
 c.name = 42
 ```
 
+If a property has no setter, we emit a bespoke error message when a user attempts to set that
+attribute, since this is a common error:
+
+```py
+class DontAssignToMe:
+    @property
+    def immutable(self): ...
+
+# error: [invalid-assignment] "Cannot assign to read-only property `immutable` on object of type `DontAssignToMe`"
+DontAssignToMe().immutable = "the properties, they are a-changing"
+```
+
 ### Built-in `classmethod` descriptor
 
 Similarly to `property`, `classmethod` decorator creates an implicit descriptor that binds the first

crates/ty_python_semantic/resources/mdtest/named_tuple.md

@@ -78,10 +78,7 @@ reveal_type(Person.id)  # revealed: property
 reveal_type(Person.name)  # revealed: property
 reveal_type(Person.age)  # revealed: property
 
-# TODO... the error is correct, but this is not the friendliest error message
-# for assigning to a read-only property :-)
-#
-# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `id` on type `Person` with custom `__set__` method"
+# error: [invalid-assignment] "Cannot assign to read-only property `id` on object of type `Person`"
 alice.id = 42
 # error: [invalid-assignment]
 bob.age = None
@@ -221,10 +218,7 @@ james = SuperUser(0, "James", 42, "Jimmy")
 # on the subclass
 james.name = "Robert"
 
-# TODO: the error is correct (can't assign to the read-only property inherited from the superclass)
-# but the error message could be friendlier :-)
-#
-# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `nickname` on type `SuperUser` with custom `__set__` method"
+# error: [invalid-assignment] "Cannot assign to read-only property `nickname` on object of type `SuperUser`"
 james.nickname = "Bob"
 ```
 

crates/ty_python_semantic/src/types/call.rs

@@ -1,6 +1,7 @@
 use super::context::InferContext;
 use super::{Signature, Type};
 use crate::Db;
+use crate::types::call::bind::BindingError;
 
 mod arguments;
 pub(crate) mod bind;
@@ -14,6 +15,17 @@ pub(super) use bind::{Binding, Bindings, CallableBinding, MatchedArgument};
 #[derive(Debug)]
 pub(crate) struct CallError<'db>(pub(crate) CallErrorKind, pub(crate) Box<Bindings<'db>>);
 
+impl CallError<'_> {
+    pub(crate) fn stems_from_attempt_to_set_property_with_no_setter(&self) -> bool {
+        self.0 == CallErrorKind::BindingError
+            && self.1.into_iter().flatten().any(|binding| {
+                binding
+                    .errors()
+                    .contains(&BindingError::PropertyHasNoSetter)
+            })
+    }
+}
+
 /// The reason why calling a type failed.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) enum CallErrorKind {

crates/ty_python_semantic/src/types/call/bind.rs

@@ -426,9 +426,7 @@ impl<'db> Bindings<'db> {
                                         overload.set_return_type(Type::unknown());
                                     }
                                 } else {
-                                    overload.errors.push(BindingError::InternalCallError(
-                                        "property has no getter",
-                                    ));
+                                    overload.errors.push(BindingError::PropertyHasNoSetter);
                                     overload.set_return_type(Type::Never);
                                 }
                             }
@@ -482,9 +480,7 @@ impl<'db> Bindings<'db> {
                                     ));
                                 }
                             } else {
-                                overload.errors.push(BindingError::InternalCallError(
-                                    "property has no setter",
-                                ));
+                                overload.errors.push(BindingError::PropertyHasNoSetter);
                             }
                         }
                     }
@@ -500,9 +496,7 @@ impl<'db> Bindings<'db> {
                                     ));
                                 }
                             } else {
-                                overload.errors.push(BindingError::InternalCallError(
-                                    "property has no setter",
-                                ));
+                                overload.errors.push(BindingError::PropertyHasNoSetter);
                             }
                         }
                     }
@@ -2593,6 +2587,10 @@ impl<'db> Binding<'db> {
     pub(crate) fn argument_matches(&self) -> &[MatchedArgument<'db>] {
         &self.argument_matches
     }
+
+    pub(crate) fn errors(&self) -> &[BindingError<'db>] {
+        &self.errors
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -2811,7 +2809,9 @@ pub(crate) enum BindingError<'db> {
         provided_ty: Type<'db>,
     },
     /// One or more required parameters (that is, with no default) is not supplied by any argument.
-    MissingArguments { parameters: ParameterContexts },
+    MissingArguments {
+        parameters: ParameterContexts,
+    },
     /// A call argument can't be matched to any parameter.
     UnknownArgument {
         argument_name: ast::name::Name,
@@ -2833,6 +2833,7 @@ pub(crate) enum BindingError<'db> {
         error: SpecializationError<'db>,
         argument_index: Option<usize>,
     },
+    PropertyHasNoSetter,
     /// The call itself might be well constructed, but an error occurred while evaluating the call.
     /// We use this variant to report errors in `property.__get__` and `property.__set__`, which
     /// can occur when the call to the underlying getter/setter fails.
@@ -3099,6 +3100,16 @@ impl<'db> BindingError<'db> {
                 }
             }
 
+            Self::PropertyHasNoSetter => BindingError::InternalCallError("property has no setter")
+                .report_diagnostic(
+                    context,
+                    node,
+                    callable_ty,
+                    callable_description,
+                    union_diag,
+                    matching_overload,
+                ),
+
             Self::InternalCallError(reason) => {
                 let node = Self::get_node(node, None);
                 if let Some(builder) = context.report_lint(&CALL_NON_CALLABLE, node) {

crates/ty_python_semantic/src/types/diagnostic.rs

@@ -10,6 +10,7 @@ use crate::semantic_index::SemanticIndex;
 use crate::semantic_index::definition::Definition;
 use crate::semantic_index::place::{PlaceTable, ScopedPlaceId};
 use crate::suppression::FileSuppressionId;
+use crate::types::call::CallError;
 use crate::types::class::{DisjointBase, DisjointBaseKind, Field};
 use crate::types::function::KnownFunction;
 use crate::types::string_annotation::{
@@ -1979,6 +1980,32 @@ pub(super) fn report_invalid_attribute_assignment(
     );
 }
 
+pub(super) fn report_bad_dunder_set_call<'db>(
+    context: &InferContext<'db, '_>,
+    dunder_set_failure: &CallError<'db>,
+    attribute: &str,
+    object_type: Type<'db>,
+    target: &ast::ExprAttribute,
+) {
+    let Some(builder) = context.report_lint(&INVALID_ASSIGNMENT, target) else {
+        return;
+    };
+    if dunder_set_failure.stems_from_attempt_to_set_property_with_no_setter() {
+        builder.into_diagnostic(format_args!(
+            "Cannot assign to read-only property `{attribute}` on object of type `{}`",
+            object_type.display(context.db())
+        ));
+    } else {
+        // TODO: Here, it would be nice to emit an additional diagnostic
+        // that explains why the call failed
+        builder.into_diagnostic(format_args!(
+            "Invalid assignment to data descriptor attribute \
+            `{attribute}` on type `{}` with custom `__set__` method",
+            object_type.display(context.db())
+        ));
+    }
+}
+
 pub(super) fn report_invalid_return_type(
     context: &InferContext,
     object_range: impl Ranged,

crates/ty_python_semantic/src/types/infer.rs

@@ -102,7 +102,7 @@ use crate::types::diagnostic::{
     INVALID_TYPE_VARIABLE_CONSTRAINTS, IncompatibleBases, POSSIBLY_UNBOUND_IMPLICIT_CALL,
     POSSIBLY_UNBOUND_IMPORT, TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE,
     UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR,
-    report_implicit_return_type, report_instance_layout_conflict,
+    report_bad_dunder_set_call, report_implicit_return_type, report_instance_layout_conflict,
     report_invalid_argument_number_to_special_form, report_invalid_arguments_to_annotated,
     report_invalid_arguments_to_callable, report_invalid_assignment,
     report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
@@ -4217,32 +4217,30 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                                     if let Place::Type(meta_dunder_set, _) =
                                         meta_attr_ty.class_member(db, "__set__".into()).place
                                     {
-                                        let successful_call = meta_dunder_set
-                                            .try_call(
-                                                db,
-                                                &CallArguments::positional([
-                                                    meta_attr_ty,
-                                                    object_ty,
-                                                    value_ty,
-                                                ]),
-                                            )
-                                            .is_ok();
-
-                                        if !successful_call && emit_diagnostics {
-                                            if let Some(builder) = self
-                                                .context
-                                                .report_lint(&INVALID_ASSIGNMENT, target)
+                                        let dunder_set_result = meta_dunder_set.try_call(
+                                            db,
+                                            &CallArguments::positional([
+                                                meta_attr_ty,
+                                                object_ty,
+                                                value_ty,
+                                            ]),
+                                        );
+
+                                        if emit_diagnostics {
+                                            if let Err(dunder_set_failure) =
+                                                dunder_set_result.as_ref()
                                             {
-                                                // TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
-                                                builder.into_diagnostic(format_args!(
-                                            "Invalid assignment to data descriptor attribute \
-                                         `{attribute}` on type `{}` with custom `__set__` method",
-                                            object_ty.display(db)
-                                        ));
+                                                report_bad_dunder_set_call(
+                                                    &self.context,
+                                                    dunder_set_failure,
+                                                    attribute,
+                                                    object_ty,
+                                                    target,
+                                                );
                                             }
                                         }
 
-                                        successful_call
+                                        dunder_set_result.is_ok()
                                     } else {
                                         ensure_assignable_to(meta_attr_ty)
                                     };
@@ -4343,27 +4341,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                         let assignable_to_meta_attr = if let Place::Type(meta_dunder_set, _) =
                             meta_attr_ty.class_member(db, "__set__".into()).place
                         {
-                            let successful_call = meta_dunder_set
-                                .try_call(
-                                    db,
-                                    &CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
-                                )
-                                .is_ok();
+                            let dunder_set_result = meta_dunder_set.try_call(
+                                db,
+                                &CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
+                            );
 
-                            if !successful_call && emit_diagnostics {
-                                if let Some(builder) =
-                                    self.context.report_lint(&INVALID_ASSIGNMENT, target)
-                                {
-                                    // TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
-                                    builder.into_diagnostic(format_args!(
-                                        "Invalid assignment to data descriptor attribute \
-                                         `{attribute}` on type `{}` with custom `__set__` method",
-                                        object_ty.display(db)
-                                    ));
+                            if emit_diagnostics {
+                                if let Err(dunder_set_failure) = dunder_set_result.as_ref() {
+                                    report_bad_dunder_set_call(
+                                        &self.context,
+                                        dunder_set_failure,
+                                        attribute,
+                                        object_ty,
+                                        target,
+                                    );
                                 }
                             }
 
-                            successful_call
+                            dunder_set_result.is_ok()
                         } else {
                             ensure_assignable_to(meta_attr_ty)
                         };