Improve error message for invalid input in Get()s

src/python/pants/engine/internals/scheduler_test.py

@@ -261,15 +261,13 @@ def rules(cls):
         )
 
     def test_get_type_match_failure(self):
-        """Test that Get(...)s are now type-checked during rule execution, to allow for union
-        types."""
-
         with self.assertRaises(ExecutionError) as cm:
             # `a_typecheck_fail_test` above expects `wrapper.inner` to be a `B`.
             self.request(A, [TypeCheckFailWrapper(A())])
-
-        expected_regex = "WithDeps.*did not declare a dependency on JustGet"
-        self.assertRegex(str(cm.exception), expected_regex)
+        assert (
+            "Invalid input to `Get(A, B)`. Expected an object with type `B`, but got `A()` with "
+            "type `A` instead."
+        ) in str(cm.exception)
 
     def test_unhashable_root_params_failure(self):
         """Test that unhashable root params result in a structured error."""

src/rust/engine/src/core.rs

@@ -148,33 +148,47 @@ impl fmt::Display for TypeId {
   }
 }
 
-// An identifier for a python function.
+/// An identifier for a Python function.
 #[repr(C)]
 #[derive(Clone, Copy, Eq, Hash, PartialEq)]
 pub struct Function(pub Key);
 
 impl Function {
+  /// A Python function's module, e.g. `project.app`.
+  pub fn module(&self) -> String {
+    let val = externs::val_for(&self.0);
+    externs::getattr_as_string(&val, "__module__")
+  }
+
+  /// A Python function's name, without its module.
   pub fn name(&self) -> String {
-    let Function(key) = self;
-    let val = externs::val_for(&key);
-    let module = externs::getattr_as_string(&val, "__module__");
-    let name = externs::getattr_as_string(&val, "__name__");
+    let val = externs::val_for(&self.0);
+    externs::getattr_as_string(&val, "__name__")
+  }
+
+  /// The line number of a Python function's first line.
+  pub fn line_number(&self) -> u64 {
+    let val = externs::val_for(&self.0);
     // NB: this is a custom dunder method that Python code should populate before sending the
     // function (e.g. an `@rule`) through FFI.
-    let line_number: u64 = externs::getattr(&val, "__line_number__").unwrap();
-    format!("{}:{}:{}", module, line_number, name)
+    externs::getattr(&val, "__line_number__").unwrap()
+  }
+
+  /// The function represented as `path.to.module:lineno:func_name`.
+  pub fn full_name(&self) -> String {
+    format!("{}:{}:{}", self.module(), self.line_number(), self.name())
   }
 }
 
 impl fmt::Display for Function {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    write!(f, "{}()", self.name())
+    write!(f, "{}()", self.full_name())
   }
 }
 
 impl fmt::Debug for Function {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    write!(f, "{}()", self.name())
+    write!(f, "{}()", self.full_name())
   }
 }
 

src/rust/engine/src/externs/mod.rs

@@ -452,7 +452,7 @@ py_class!(pub class PyGeneratorResponseGetMulti |py| {
 pub struct Get {
   pub output: TypeId,
   pub input: Key,
-  pub input_type: Option<TypeId>,
+  pub input_type: TypeId,
 }
 
 impl Get {
@@ -462,7 +462,7 @@ impl Get {
       input: interns
         .key_insert(py, get.subject(py).clone_ref(py).into())
         .map_err(|e| Failure::from_py_err_with_gil(py, e))?,
-      input_type: Some(interns.type_insert(py, get.declared_subject(py).clone_ref(py))),
+      input_type: interns.type_insert(py, get.declared_subject(py).clone_ref(py)),
     })
   }
 }

src/rust/engine/src/nodes.rs

@@ -902,34 +902,49 @@ impl Task {
           .core
           .rule_graph
           .edges_for_inner(&entry)
-          .ok_or_else(|| throw(&format!("no edges for task {:?} exist!", entry)))
+          .ok_or_else(|| throw(&format!("No edges for task {:?} exist!", entry)))
           .and_then(|edges| {
-            edges
-              .entry_for(&dependency_key)
-              .cloned()
-              .ok_or_else(|| match get.input_type {
-                Some(ty) if externs::is_union(ty) => {
-                  let value = externs::type_for_type_id(ty).into_object();
-                  match externs::call_method(
-                    &value,
-                    "non_member_error_message",
-                    &[externs::val_for(&get.input)],
-                  ) {
-                    Ok(err_msg) => throw(&externs::val_to_str(&err_msg)),
-                    // If the non_member_error_message() call failed for any reason,
-                    // fall back to a generic message.
-                    Err(_e) => throw(&format!(
-                      "Type {} is not a member of the {} @union",
-                      get.input.type_id(),
-                      ty
-                    )),
-                  }
+            edges.entry_for(&dependency_key).cloned().ok_or_else(|| {
+              if externs::is_union(get.input_type) {
+                let value = externs::type_for_type_id(get.input_type).into_object();
+                match externs::call_method(
+                  &value,
+                  "non_member_error_message",
+                  &[externs::val_for(&get.input)],
+                ) {
+                  Ok(err_msg) => throw(&externs::val_to_str(&err_msg)),
+                  // If the non_member_error_message() call failed for any reason,
+                  // fall back to a generic message.
+                  Err(_e) => throw(&format!(
+                    "Type {} is not a member of the {} @union",
+                    get.input.type_id(),
+                    get.input_type
+                  )),
                 }
-                _ => throw(&format!(
-                  "{:?} did not declare a dependency on {:?}",
-                  entry, dependency_key
-                )),
-              })
+              } else {
+                let parent_rule_func = match &*entry {
+                  rule_graph::Entry::WithDeps(entry_with_deps) => match entry_with_deps.rule() {
+                    Some(ref rule) => match rule {
+                      tasks::Rule::Task(ref task) => Some(task.func),
+                      _ => None,
+                    },
+                    None => None,
+                  },
+                  _ => None
+                };
+                let rule_context = match parent_rule_func {
+                  Some(func) => format!(
+                    " See the rule {}() in {} (line {}).",
+                    func.name(), func.module(), func.line_number()
+                  ),
+                  _ => "".to_string()
+                };
+                throw(&format!(
+                  "Invalid input to `Get({}, {})`. Expected an object with type `{}`, but got `{:?}` with type `{}` instead.{}",
+                  get.output, get.input_type, get.input_type, get.input, get.input.type_id(), rule_context
+                ))
+              }
+            })
           });
         // The subject of the get is a new parameter that replaces an existing param of the same
         // type.

src/rust/engine/src/tasks.rs

@@ -23,7 +23,7 @@ impl DisplayForGraph for Rule {
   fn fmt_for_graph(&self, display_args: DisplayForGraphArgs) -> String {
     match self {
       Rule::Task(ref task) => {
-        let task_name = task.func.name();
+        let task_name = task.func.full_name();
         let product = format!("{}", task.product);
 
         let clause_portion = Self::formatted_select_clause(&task.clause, display_args);