fukamachi / mito

An ORM for Common Lisp with migrations, relationships and PostgreSQL support
293 stars 31 forks source link

Effective Slot Definitions Don't Work As Expected #158

Open daninus14 opened 6 days ago

daninus14 commented 6 days ago

The effective slots definitions don't work as expected. In particular the col-type, ghost, and other needed slot definitions are not present in the effective slot definitions and only show up in the direct slot definitions.

Mito itself is using a workaround to compute table-column-slots here https://github.com/fukamachi/mito/blob/master/src/core/class/table.lisp#L205

Here's the code for reference

(defun table-column-slots (class)
  (map-all-superclasses #'table-direct-column-slots
                        class
                        :key #'c2mop:slot-definition-name))

Here are the two helper functions

(defun table-direct-column-slots (class)
  (remove-if-not (lambda (slot)
                   (typep slot 'table-column-class))
                 (c2mop:class-direct-slots class)))

(defun map-all-superclasses (fn class &key (key #'identity))
  (labels ((main (class &optional main-objects)
             (let ((ret (funcall fn class)))
               (loop for superclass in (c2mop:class-direct-superclasses class)
                     if (eq (class-of superclass) (find-class 'standard-class))
                       append (if (eq superclass (find-class 'standard-object))
                                  (append ret main-objects)
                                  ret)
                     else
                       append (main superclass
                                    (append ret main-objects))))))
    (delete-duplicates
     (main class)
     :test #'eq
     :key key
     :from-end t)))

Basically this is a calculation of the class-slots using the standard slot definition of the slots in the class and its parent classes.

This is not the correct implementation and can easily cause issues with inheritance. Furthermore, it makes the system not extensible.

I am using this table-column-slots function myself as a client of mito in different projects, but this is not the ideal way to do it.

I just worked on a project and learned how to properly define the effective slots.

See here for an example https://github.com/daninus14/mito-validate/blob/main/src/classes.lisp#L89

I can actually do a PR to fix the issues in mito. However I don't want to go through all that work if it's not going to be accepted.

Are you open to accepting a PR to fix the current MOP implementation of effective slot definitions?

This would not remove any existing functionality, however it would provide correct effective slot definitions making table-column-slots unnecessary and replaceable with (closer-mop:class-slots). In addition it would make it much easier to extend mito with more plugins and functionality related to slot definitions and metaclasses.

daninus14 commented 6 days ago

Please note that this would require adding the PR: https://github.com/fukamachi/mito/pull/155 because that code breaks the computation of effective slots.