Remove ASTNode subclassing

[pcardune]

You said it couldn't be done, but here it is!

Here's my before/after summary:

before:

class FunctionApp extends ASTNode<{ func: ASTNode; args: ASTNode[] }> {
  static spec = nodeSpec(...);
  constructor(from:Pos, to:Pos, func:ASTNode, args:ASTNode[], options:NodeOptions) {
    super(from, to, "functionApp", {func, args}, options);
  }
  render() { ... }
  pretty() { ... }
  longDescription() { ... }
}

after:

function FunctionApp(from:Pos, to:Pos, func:ASTNode, args:ASTNode[], options:NodeOptions) {
  return new ASTNode({
    from,
    to,
    type: "functionApp",
    spec: nodeSpec(...),
    fields: {func, args},
    render: (node) => {...}
    pretty: (node) => {...}
    longDescription: (node) => {...}
  };
}

While this PR is good enough to merge, I don't consider my refactoring to be "done", but it's on the path to "done". The next step are:

• remove the need for new ASTNode() and have the function just return the object.
• separate out the render/pretty/longDescription functions and spec into a separate "registry" so that parsers just have to produce objects with {from, to, type, fields}

I've also been playing around with codemirror 6 on the size and seeing how they link the parse trees from their lezer system to syntax highlighting. We should do something similar. You basically just have a mapping from ast node type to a highlight type, except the mapping is context aware like css, so identifiers can have different colors depending on the context in which they appear in. We could use the same pattern to have context aware block rendering.

[coveralls]

Coverage decreased (-0.05%) to 69.786% when pulling 60401c0 on pcardune-ast-this into adf2503 on master.

[pcardune]

@schanzer do you have access to code climate settings? I think we should either remove it altogether or at least make it report a "warning" instead of a "failure" in the github check system if at all possible.

[schanzer]

[CodeClimate removed]

This is really exciting! Minimizing the surface area of our API like this is going to be a nice improvement. Color me impressed!

I'm intrigued by your proposal, to decouple the node methods from the node structure. And I LOVE the idea of using CM6's context-aware rules for it, too. So to try out your CSS analogy, it sounds like we'd get a giant node array and a style object from a language module. The style object would look something like:

{
  "defFun": {
    render : ...,
    pretty : ...,
    longDescription : ...,
  },
  "sequence": {
    render : ...,
    pretty : ...,
    longDescription : ...,
  },
  "defFun sequence": {
    render : ...,
    pretty : ...,
    longDescription : ...,
  },
}

...where the last style uses some kind of selector rule to denote that a sequence node that is a child of the defFun node should have its own methods.

Is that right? I think that's an incredible idea. Two questions (and feel free to move this discussion to #518 :

1. In the example above, suppose the third rule doesn't declare a pretty method. Would we set this up so that it "inherits" from the next-most-selective-rule?
2. In that case, could we literally use CSS rules? That seems like a developer win, and there's a nice library for it.
3. Is there a universe in which we could rely on CM6's lezer trees, moving the options field out to the style rule as well? This would let us generate our dom from a tree API that has a lot more eyes on it, and already has tons of language modes. Assuming we have good defaults for render, pretty, and longDescription (change to description?), we could theoretically take any CM6 language mode and generate a block language with zero additional work. Adding styling and a11y still needs elbow grease, of course, but that's where we want developers to spend their effort.

[pcardune]

In the example above, suppose the third rule doesn't declare a pretty method. Would we set this up so that it "inherits" from the next-most-selective-rule?

Potentially yes.

In that case, could we literally use CSS rules? That seems like a developer win, and there's a nice library for it.

CSS can get so complicated/convoluted that I'm not sure I'd want to sign up for supporting that kind of API, but I think we could support some stricter subset that fits our specific needs more closely.

The codemirror 6 API doesn't use literal CSS syntax and doesn't support the full feature range of css selectors. The selector syntax they do support is described here.

Is there a universe in which we could rely on CM6's lezer trees, moving the options field out to the style rule as well? This would let us generate our dom from a tree API that has a lot more eyes on it, and already has tons of language modes. Assuming we have good defaults for render, pretty, and longDescription (change to description?), we could theoretically take any CM6 language mode and generate a block language with zero additional work. Adding styling and a11y still needs elbow grease, of course, but that's where we want developers to spend their effort.

Yeah, something like that. The one uncertainty I have about the lezer tree API is how specific child nodes are accessed. Our node spec system provides access via both an ordered list (children()) and via named fields. The named fields make implementing the render/pretty/description functions much easier. The closest thing with lezer trees are node groups, where you can select specific children based only on their node type. Their docs say:

Because Lezer does not, like most parsers, organize a node's children (beyond the order in which they appear in the source), it can be difficult to find the children you are interested in.

It's unclear whether this was a carefully made design decision that's not going to change or whether it's a feature that just hasn't been implemented yet.

[schanzer]

Oh yeah, I wasn't suggesting we implement the entire day of the selector syntax. Maybe just the most important bits. And I certainly understand the potential benefit of using the same selectors that code Mary users, although that does make us even more implementation dependent. But given the name of the library and our intended uses for it, I don't think that's such a bad thing!

As for using Lezer trees natively - given the limitation you describe, it sounds like we'd need to force an invariant like "your style rules must declare fields by name, in the order in which they appear in the source". Then we could just grab the children of each node, name them by matching them to the field array, and pass the resulting object to the renderer.

[pcardune]

Is it alright if I merge this PR or are you still reviewing the code?

[schanzer]

Oh! When you said your work wasn't done I thought you wanted to still push commits. Definitely ready to merge! I'm assuming any necessary changes to the language modules will be tracked in a separate issue?

[pcardune]

I implemented the changes for wescheme-blocks as part of this PR.