Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add source file location into RDF output #17

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mchaloupka
Copy link

@mchaloupka mchaloupka commented Dec 30, 2024

I have added the location in SDML file in the RDF output which is produced after the generate CLI command is called. I have done it using an additional macro which accesses the span() method on the given definition and writes out the location (if available).

To be able to do that, I also had to add the get/set methods for span on the definitions where it was not yet present. The field was always present already.

From usability perspective, I also added the logic to include also line and column in the source file as that tends to be more user friendly than bytes. The tree-sitter exposes that, but I had to change the Span definition to include this detailed information.

The lats part of this change was to update all the tests that use snapshots. It would be tedious to update them by hand, so I added the ability to update them automatically. The following command does not fail if the examples output files differ, it updates them instead:

UPDATE_EXAMPLES_OUTPUT=1 cargo test

Disclaimer: the diff contains also #16 as that allows me to have working tests.

@mchaloupka mchaloupka changed the title Feature/add file location Add source file location into RDF output Dec 30, 2024
@@ -169,6 +169,8 @@ impl TypeClassDef {
extend_variables
=> variables, TypeVariable
);

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually performed by the impl_has_source_span_for macro, which implements the crate::model::HasSourceSpan trait resulting in the methods with_source_span, has_source_span, source_span, set_source_span and unset_source_span.

@@ -156,4 +156,6 @@ impl DatatypeDef {
get_and_set_bool!(pub opaque, is_opaque, set_opaque);

get_and_set!(pub base_type, set_base_type => IdentifierReference);

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -88,6 +88,8 @@ impl EntityDef {
body: None,
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -103,6 +103,8 @@ impl EnumDef {
body: None,
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -168,4 +170,6 @@ impl ValueVariant {
body: None,
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -78,4 +78,5 @@ impl EventDef {
// --------------------------------------------------------------------------------------------

get_and_set!(pub event_source, set_event_source => IdentifierReference);
get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -83,4 +83,5 @@ impl PropertyDef {
// --------------------------------------------------------------------------------------------

get_and_set!(pub member, member_def, set_member_def => MemberDef);
get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -126,4 +126,6 @@ impl RdfDef {
pub fn individual(name: Identifier) -> Self {
Self::new(name)
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -112,6 +112,8 @@ impl UnionDef {
..self
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -225,4 +227,6 @@ impl TypeVariant {
}
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -239,6 +239,8 @@ impl Member {
MemberKind::Definition(defn) => Some(defn.target_cardinality().clone()),
}
}

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

@@ -217,6 +217,8 @@ impl Module {

get_and_set!(pub source_file, set_source_file, unset_source_file => optional has_source_file, PathBuf);

get_and_set!(pub span, set_span, unset_span => optional has_span, Span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above re:impl_has_source_span_for.

Copy link
Member

@johnstonskj johnstonskj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove all the get_and_set macro calls you should be able to use the existing source_span instead of your span methods. If you make those changes I'll merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants