Support References in docs

[srawlins]

Work for #217. (I'm not sure what else is desired for that bug...)

[matanlurey]

Some thoughts. Would love @natebosch to weigh in too!

[matanlurey]

Let's add comments here and below why Object is being used and what a valid type is (and isn't).

[srawlins]

Done

[matanlurey]

Part of me wonders if docs should be a more complex class than a List, i.e.:

class DocsBuilder {
  void addText();
  void addReference();
}

... WDUT? It would be more breaking, though.

[srawlins]

I started out with a much bigger change where I added a new class, DocComments, and everything that HasDocComments today also got a @nullable DocComments field. Then that class just had a List<Object>, so that you could add Strings, and References.

Then I looked back and saw that I had really just created this unnecessary nesting, and confusion with the existing docs field. And the emitter would probably have to have some weird logic in it saying "use one or the other." Essentially the new DocComment type did not help with the List<Object> issue.

If there was a way to hook into BuiltList, with validation logic, that would be tops. Then I could validate the type at runtime. Or add specific methods, addText and addReference. There isn't, is there? Would I subtype BuiltList/ListBuilder? ☹️

[matanlurey]

I honestly am not sure. Let's wait and see what @natebosch says - we could also consult with @davidmorgan.

If you want this change as-is, I'm willing to approve. I do think we should consider making the interface better.

[srawlins]

Nope! I am definitely in favor of getting this right. And if creating a new field and adding "don't use both" logic to emitter is ultimately the right way, I'm game.

[davidmorgan]

I think the recommended way to do that would be to make an interface Doc and have built value classes Text and Reference that implement it. I don't think that can be a compatible change, unfortunately.

BuiltList can't do validation, you could validate in the class that has the list, a built_value can do validation in the private _ constructor, the fields are all initialized when it runs.

[srawlins]

Ack

[natebosch]

I do think it would be best to add a new field for this and deprecate the old one.

I think we should move away from List and call the new field doc, or docComment or maybe documentation instead of the plural docs - and it should be a built value type. We can add a convenience text constructor. Usage could migrate from docs.add('Something.') to doc = DocComment.text('Something.')

[srawlins]

Sounds good.