Record Source Annotations

[maennchen]

Implements #65

Features

• Provides Introspection Function to retrieve erl_anno for sections, section options, entities, and entity properties
• Adds location to DslError and provides it where feasible
• Adds Spark.Warning (internal) for emitting warnings. Supports intentional locations as well as stacktraces.

Missing / Incomplete

• End Of Expression Locations require token_metadata parser option to be enabled.
• Start Location Column is not tracked since that would require Elixir to pass column meta into Macro.Env.
• Option End Of Expression is not tracked since the value is expanded and the macro receives the value and not an AST.

Contributor checklist

Leave anything that you believe does not apply unchecked.

• I accept the AI Policy, or AI was not used in the creation of this PR.
• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

[maennchen]

@jimsynz In the issue you asked for column info. Unfortunately, Macro.Env does not contain column info. See https://elixirforum.com/t/why-is-column-information-stripped-from-elixirs-ast/6848/14

[zachdaniel]

So, I'm ecstatic about this change, but I'm not a huge fan of the fact that entities have to add the source annotations within themselves. Its data that will be passed around during introspection for example.

It probably doesn't matter? It is nice because it solves for entities that don't have identifiers, because we'd have to use the identifier feature otherwise...yeah, okay. This is the best way in general then.

As for updating entities, most of the time when updating entities you don't create a new one, you just update some fields and then replace it. I don't know of any place that we do otherwise.

As for errors, most of the time people are raising Spark.Error.DslError so we should add a location field to that, and that location field should mirror what the location of a compile error looks like in Elixir, because that will make language servers figure things out better IIRC.

[maennchen]

but I'm not a huge fan of the fact that entities have to add the source annotations within themselves. Its data that will be passed around during introspection for example.

My goal of this PR is clarity related: I would like to have the location info in ash structs like Ash.Resource.Actions.Action. The goal is exactly that the info is passed around during introspection.

I'd prefer not to have to dig around spark directly in a higher level introspection tool.

As for updating entities, most of the time when updating entities you don't create a new one, you just update some fields and then replace it. I don't know of any place that we do otherwise.

Makes sense. I initially wrote the docs where I had a statically named field. But with the anno_field, the responsibility is with the user anyways. I'll delete point 4 out of the list.

As for errors, most of the time people are raising Spark.Error.DslError

Ah yes, I can use that one in the exception example.

so we should add a location field to that, and that location field should mirror what the location of a compile error looks like in Elixir, because that will make language servers figure things out better IIRC.

Elixir right now does only line info in errors as far as I can see with the exception of stacktraces. I would probably add the full anno to it so that we can support columns without any changes for the user once (hopefully) elixir adds column info into the Macro.Env struct.

[zachdaniel]

Elixir right now does only line info in errors as far as I can see with the exception of stacktraces. I would probably add the full anno to it so that we can support columns without any changes for the user once (hopefully) elixir adds column info into the Macro.Env struct.

The main question is just will it be compatible with red squiggles from the language servers 😆

Now, you may still need to dig around in spark for this stuff eventually, if you want to show options or section location.

[maennchen]

@zachdaniel There's specific clauses in Expert to be able to handle CompileErrors: https://github.com/elixir-lang/expert/blob/d9099800cb4168be1b4239e661b20f59c7db0bc6/apps/engine/lib/engine/build/error.ex#L94-L109

It will therefore for sure not work right now.

[zachdaniel]

We could turn all Dsl Errors that are raised or returned from transformations with locations into compile errors automatically. We'd also want to extract that location info for warnings that we create.

[maennchen]

We could turn all Dsl Errors that are raised or returned from transformations with locations into compile errors automatically.

That sounds like a breaking change. Are you ok to do a new major release for this?

We'd also want to extract that location info for warnings that we create.

That sounds like a good idea. But warnings are just strings & IO. I don't think we can pass the structured location info anywhere...

[zachdaniel]

That sounds like a breaking change. Are you ok to do a new major release for this?

Right you are, no I'm not :laughing

That sounds like a good idea. But warnings are just strings & IO. I don't think we can pass the structured location info anywhere...

We can at least include the location in the warning. I could swear Elixir warnings can get you yellow squiggles, so someone is doing it somehow. 😆

[maennchen]

I could swear Elixir warnings can get you yellow squiggles

TIL, IO.warn/2 does allow passing that info :)

I added a second commit that takes care of the warnings. Basically the module will either take a user supplied location or a stacktrace. If nothing is provided, it will use the current stacktrace.

[maennchen]

@zachdaniel Ok, I believe this should be ready :)

[zachdaniel]

its not required though is it? Its optional, but will add extra information if present.

[maennchen]

If we rely on it for errors and warnings, I would make it required.

[zachdaniel]

But this would break every current DSL that doesn't have that field? That wouldn't be an option unless we want to do a major release.

[zachdaniel]

🚀

[zachdaniel]

🚀 Thank you for your contribution! 🚀