[Feature] Attach attributes inside imported algebraic types

[b0o]

It seems a large use case for import is, as stated in the readme, for use with deriving to derive functions for types that you do not own.

It is commonly necessary to place attributes within a definition of an algebraic type to specify the desired behavior of a deriving plugin. However, this does not seem to be currently possible with import.

For example, I have a use case (see #39) where I need to derive sexp conversion functions for a type I do not own:

Library code:

(* I3ipc.Event.workspace_event_info *)
type workspace_event_info = {
  change: workspace_change;
  current: Reply.node option;
  old: Reply.node option;
}

I can accomplish this:

type workspace_event_info = [%import: I3ipc.Event.workspace_event_info] [@@deriving sexp]

However, I would really like to be able to attach [@default None] to the current and old option types:

type workspace_event_info = {
  change: workspace_change;
  current: Reply.node option [@default None];
  old: Reply.node option [@default None];
}

It would be nice if I could use the existing with syntax to accomplish this. Perhaps it would look something like this (I am not very familiar with ppx semantics):

type workspace_event_info = [%import: I3ipc.Event.workspace_event_info 
    [@with workspace_event_info.current := I3ipc.Event.workspace_event_info.current [@default None];
           workspace_event_info.old := I3ipc.Event.workspace_event_info.old [@default None]]] 
[@@deriving sexp]

Currently, my best idea of how to accomplish this is to simply duplicate the type definitions that I want to modify in this way, but I feel that this would be a good feature to make import more useful.

I would be happy to help implement such a feature with some guidance and feedback.

Thank you!

[whitequark]

I am not sure if this can, or even should be covered under ppx_import's model. (a) It's likely that adding new variants will require customization, and (b) the workaround you suggested is actually longer than writing out the entire type yourself.

[b0o]

@whitequark Thanks for your feedback! You bring up good points.

A:

By "adding new variants", I assume you are referring to [%import]-ing variant/record types; sorry if I am wrong, please correct me if so

I agree that there are cases where customizing an external type would be necessary, and accommodating this seems out of scope for ppx_import.

However, there are cases where one wants to import and preserve a type, but add attributes to its record fields/variant cases to aid in further ppx processing.

A quick search of GitHub shows that most uses of ppx_import in the wild are in combination with ppx_deriving plugins. Commonly used in these cases seem to be ppx_sexp_conv and ppx_deriving_yojson, each of which provide attributes as a means to configure their behavior, yet cannot currently be taken advantage of by the user of ppx_import.

B:

I think there are two issues here:

• the syntax I provided as an example is quite clunky, but was meant as a visual aid. I think there exists a better syntax
• in a real-world scenario, I think this proposal could end up being much shorter than manually re-declaring a type.

---

Thinking more about it, I think re-using the [@with] syntax is probably not the best solution. Another attribute field could be used - maybe [@attach].

Here is a contrived example:

We are building a client for a service who provides a simple ocaml library declaring the User.t type:

module User = struct
  type user_type = Admin | Moderator | Standard
  type t = {
    name: string;
    _type: user_type;
    email: string;
    email_verified: bool;
    bio: string option;
    birthdate: Date.t option;
    friends: t list;
    liked_tags: string list;
  }
end

This type does not supply yojson conversion functions, so we use ppx_import and ppx_deriving_yojson:

type user_type = [%import: User.user_type] [@@deriving yojson]
type user = [%import: User.t] [@@deriving yojson]

But we have a few issues:

• we want to be able to omit optional fields and empty lists when converting from json
  • solution: use [@default None] and [@default []], respectively
• the User.t._type field is actually called type in our API, but it has to be prefixed with an underscore in the record definition to avoid clashing with the type keyword.
  • solution: use [@key "type"] to inform yojson of the correct field name

Currently, I don't think there is a way to accomplish this with [%import], so we would need to duplicate the entire user definition, which also means we need to maintain it in the future as the upstream User.t type gets additional fields.

I propose a syntax such as this:

type user_type = [%import: User.user_type] [@@deriving yojson]
type user = [%import: User.t 
                      [@attach _type [@key "type"];
                               bio [@default None];
                               birthdate [@default None];
                               friends [@default []];
                               liked_tags [@default []]]]
[@@deriving yojson]

The benefits to this approach:

• gives ppx_import maximal flexibility with regards to usage with ppx_deriving plugins
• reduce code duplication
• our code stays in sync with upstream as long as the fields we are attaching attributes to remain compatible

Further feedback is welcomed, especially as to whether or not this would be a useful feature.

[whitequark]

By "adding new variants", I assume you are referring to [%import]-ing variant/record types; sorry if I am wrong, please correct me if so

I mean that if the upstream is adding new variants (or fields for that matter), there will be no indication downstream that this happened. In particular, if you're just changing one field (maybe _type) that's reasonable; you essentially never want to update the downstream import in response to upstream changes. But your examples show that you want to attach attributes to many fields, which means that upstream changes may mean that more attributes downstream need to be added.

With your proposal, this is more error-prone than if you duplicated the type, since the compiler won't help you and you'd have to look at diffs or changelogs of the upstream package to determine if any downstream updates are needed.