From 7a9fa8aab76474024ea5f47b730b93b054b22aa8 Mon Sep 17 00:00:00 2001
From: Carl Gay <carlgay@gmail.com>
Date: Wed, 12 Jun 2024 10:20:00 -0400
Subject: [PATCH] <type-error>: fix test and remove condition-to-string method

* Remove `condition-to-string(<type-error>)` because
  `condition-to-string(<simple-condition>)` already does the same job.

* Add `make-condition(<type-error>)` to supply the required init args for the
  test.

* Make both value: and type: be required init keywords. Potentially
  controversial since they're not specified as required in the DRM, but the
  error is meaningless without these slots set and this would have uncovered the
  test suite bug right away.
---
 .../common-dylan/common-extensions.rst             | 14 ++++++++++----
 sources/common-dylan/format.dylan                  |  7 -------
 .../tests/condition-test-utilities.dylan           |  6 ++++++
 sources/dylan/condition-extras.dylan               | 10 +++++++---
 sources/dylan/tests/specification.dylan            |  8 +++++++-
 5 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/documentation/source/library-reference/common-dylan/common-extensions.rst b/documentation/source/library-reference/common-dylan/common-extensions.rst
index 7bb3fffe67..e5d853c3da 100644
--- a/documentation/source/library-reference/common-dylan/common-extensions.rst
+++ b/documentation/source/library-reference/common-dylan/common-extensions.rst
@@ -132,7 +132,7 @@ The extensions are:
 .. generic-function:: condition-to-string
    :open:
 
-   Returns a string representation of a condition object.
+   Returns a :drm:`<string>` representation of a condition object.
 
    :signature: condition-to-string *condition* => *string*
 
@@ -142,9 +142,15 @@ The extensions are:
    :description:
 
      Returns a string representation of a general instance of
-     :drm:`<condition>`. There is a method on
-     :class:`<format-string-condition>` and method on
-     :drm:`<type-error>`.
+     :drm:`<condition>`.
+
+     Note that it is often not necessary to write a method on this generic
+     function for your condition classes because you can use the method
+     provided by :class:`<simple-condition>`, usually via one of its
+     subclasses, :drm:`<simple-error>`, :drm:`<simple-warning>`, or
+     :drm:`<simple-restart>`. Simply make your condition a subclass of one of
+     these classes and provide the ``format-string:`` and ``format-arguments:``
+     init keywords.
 
 .. macro:: debug-assert
    :statement:
diff --git a/sources/common-dylan/format.dylan b/sources/common-dylan/format.dylan
index 6860ddcfac..cdf63103f8 100644
--- a/sources/common-dylan/format.dylan
+++ b/sources/common-dylan/format.dylan
@@ -588,13 +588,6 @@ define method condition-to-string
         condition-format-arguments(condition))
 end method condition-to-string;
 
-define method condition-to-string
-    (error :: <type-error>) => (string :: <string>)
-  format-to-string("%= is not of type %=",
-                   type-error-value(error),
-                   type-error-expected-type(error))
-end method condition-to-string;
-
 define method print-pretty-name
     (buffer :: <string-buffer>, condition :: <condition>)
  => ()
diff --git a/sources/common-dylan/tests/condition-test-utilities.dylan b/sources/common-dylan/tests/condition-test-utilities.dylan
index d9ca4d8ec2..c47811638e 100644
--- a/sources/common-dylan/tests/condition-test-utilities.dylan
+++ b/sources/common-dylan/tests/condition-test-utilities.dylan
@@ -66,6 +66,12 @@ define method test-condition (condition :: <type-error>) => ()
             ))
 end method test-condition;
 
+define method make-condition (class == <type-error>) => (c :: <type-error>)
+  make(<type-error>,
+       value: #"type-error-value",
+       type: <string>)
+end method;
+
 define method test-condition (condition :: <simple-warning>) => ()
   next-method();
   do(method (function) function(condition) end,
diff --git a/sources/dylan/condition-extras.dylan b/sources/dylan/condition-extras.dylan
index 5d9e09fb86..be53f97fff 100644
--- a/sources/dylan/condition-extras.dylan
+++ b/sources/dylan/condition-extras.dylan
@@ -89,9 +89,13 @@ end method abort;
 
 /// TYPE-ERRORS
 
-define open class <type-error> (<error>, <format-string-condition>)  // Should be sealed?
-  constant slot type-error-value, init-keyword: value:;
-  constant slot type-error-expected-type :: <type>, init-keyword: type:;
+// Note that the DRM only mentions <error> as a superclass. Speculation: we
+// additionally subclass <simple-condition> because there is special support
+// for its condition-format-string and condition-format-arguments in the
+// debugger manager.
+define sealed class <type-error> (<error>, <simple-condition>)
+  constant slot type-error-value, required-init-keyword: value:;
+  constant slot type-error-expected-type :: <type>, required-init-keyword: type:;
 end class <type-error>;
 
 define method make
diff --git a/sources/dylan/tests/specification.dylan b/sources/dylan/tests/specification.dylan
index 81daafbb38..46c50f63da 100644
--- a/sources/dylan/tests/specification.dylan
+++ b/sources/dylan/tests/specification.dylan
@@ -238,7 +238,7 @@ define interface-specification-suite dylan-conditions-specification-suite ()
   open abstract class <serious-condition> (<condition>);
   sealed instantiable class <simple-error> (<error>);
   sealed instantiable class <simple-warning> (<warning>);
-  sealed instantiable class <type-error> (<error>);
+  sealed instantiable class <type-error> (<error>); // make-test-instance method below.
   open abstract class <warning> (<condition>);
 
   /// Restarts
@@ -286,6 +286,12 @@ define interface-specification-suite dylan-conditions-specification-suite ()
     expected-to-fail-reason: "https://github.com/dylan-lang/opendylan/issues/1295";
 end dylan-conditions-specification-suite;
 
+define sideways method make-test-instance (class == <type-error>) => (err :: <type-error>)
+  make(<type-error>,
+       value: #"type-error-value",
+       type: <string>)
+end method;
+
 //--- Bindings not defined by the DRM
 //---*** Are there any others?