Pass target model in AugmentRequest

[jakemac53]

Closes #100

• Adds model field to AugmentRequest, with implementations to pass it (although, these use a host side query).
  • Expose the hostService from the MacroHost (used to do the query)
• Adds a Model parameter to all macro methods (typed as Object for right now for most of them, as we don't have better types yet).
• Make all fields on AugmentRequest required (this simplified implementations, but lmk if you want me to revert this).
• Rename the Model.qualifiedNameOfMember extension to qualifiedNameOf generalizing and fixing it:
  • Now it actually does what it says 🤣 (it used to return just the qualified name of the type the member is declared in I think?), there was in any case no way to represent members with qualified names, see add scope and isStatic to QualifiedName, as well as parsing and a URI format #101 which adds that.
  • It now throws for anything other than types, since they can't be referenced
  • It just accepts a Map<String, Object?> now which is the only thing we can generalize on for the moment, but see Consider mimicking type hierarchies in our extension types. #103 for one possible workaround.
• Fixes up tests and removes expected request/response from the client for the model

[jakemac53]

I am not able to figure out how to fix the remaining test failures related to the scopes in which these models are created :/.

I tried sprinkling a bunch of Scope.query.run stuff around various parts but to no avail.

Figured this out I think, although we do have an issue where the query fails in the types phase. For now I might just work around this with a try/catch and add a TODO.

This is a good forcing function to enable filtered queries (so we can avoid asking for the members).

[davidmorgan]

Thanks! I think there are three things worth discussing here, maybe we can discuss today?

Minimizing data + round trips.

I think we need an API design here that is perfect for performance, or very close.

That means no unnecessary data returned and no unnecessary round trips.

I think the way to do that is for the macro to specify its initial query in the MacroDescription. We still need to flesh out queries, but I guess it can be something like: the MacroDescription has the initial query with missing target, and the host fills in the target.

This could be implemented as a next step after this PR, but there is one aspect that is maybe worth thinking about now, which is that we then need API for specifying the initial query, for example a getter that goes with each macro method, for the bootstrap code to call.

Evolving classes vs parameters.

Classes are easier to evolve than lists of parameters, because you can add fields to a class and old code that receives the class does not have to change. So I wonder if we should put as much as we can in one "macro scope" object ... actually, we have scopes, see next item ;)

That would leave the target as the only parameter for these methods, which makes sense because it's the thing that can be different per method. So for example buildDefinitionsForClass(Interface target). We would never change Interface to a different type, we would add a new method that accepts the different type.

Use of Scope including implicit use.

I realize now that we use the word "scope" twice :( ... let's change that ... here I mean the scope that is a dependency injection scope, not the Dart language concept.

We have a concept of macro scope with a Model in it, so we can just put the model there and not pass it explicitly.

Host seems to fit in with that, i.e. a macro always has a Host available to it, we can add it to the scope.

For implicit use, I mean for example that qualifiedName = model.qualifiedNameOf(target.node) can be replaced with getters like Interface.qualifiedName that get the model from the scope in order to look up the qualified name.

[jakemac53]

I think the way to do that is for the macro to specify its initial query in the MacroDescription. We still need to flesh out queries, but I guess it can be something like: the MacroDescription has the initial query with missing target, and the host fills in the target.

We might want them to be able to only request certain aspects of the target as well, as opposed to getting an implicit query with all the header info (this way you could say, add a mixin and not re-run the macro if it doesn't care about mixins).

I am a bit skeptical that they will be able to express the query at all without already having access to the target. So, we might need to invent a special syntax for expressing queries as templates with variables, in this case the variable would be the target. But, macros can also target different kinds of targets so I am still a bit skeptical this will work out well. We will have to work through examples of that.

I do think it makes sense for the API to provide the target object directly still and not just a generic Model for the initial query response. Especially if you have an initial query you want to run, and it is merged with the implicit Model, you wouldn't even necessarily be able to find what the actual target was in that object because it would be littered with a bunch of other stuff. We could provide the target as a qualified name, and then add a way to look it up in the model easily (we probably want this functionality anyways).

This could be implemented as a next step after this PR, but there is one aspect that is maybe worth thinking about now, which is that we then need API for specifying the initial query, for example a getter that goes with each macro method, for the bootstrap code to call.

I do think it should be a next step, this PR can be adapted to fit whatever we come up with.

For implicit use, I mean for example that qualifiedName = model.qualifiedNameOf(target.node) can be replaced with getters like Interface.qualifiedName that get the model from the scope in order to look up the qualified name.

How would we deal with models from query responses? Would the implicit model from the scope always get merged with each query response (maybe using #105?)

[davidmorgan]

LG to land with various followups as discussed :) thanks.

[jakemac53]

Specific follow up summary:

• land this pr as is
• let you define your initial query
• Make the API only take a builder object, with access to the target model via a getter (@davidmorgan we agreed this should be the model (ie: Interface) and not the qualified name right?)
  • This will also auto merge the results of any queries into the model that is in scope.
  • It also opens up the door to potentially trying to answer queries with the existing model in scope when all data is present, and/or converting queries to minimal ones when additional data is needed.
• Add some convenience methods on qualified name to look it up in the model because this will be generally useful.

[davidmorgan]

Yes, I think the builder can know which type is useful and return that specific type instead of QualifiedName.