Skip to content

feat: added parser for model definitions#156

Open
HassanAkbar wants to merge 24 commits into
mainfrom
adding_lml_parser_for_lutaml_model_definitions
Open

feat: added parser for model definitions#156
HassanAkbar wants to merge 24 commits into
mainfrom
adding_lml_parser_for_lutaml_model_definitions

Conversation

@HassanAkbar

Copy link
Copy Markdown
Member

Added parser for model definition

for -> lutaml/lutaml-model#285

@HassanAkbar HassanAkbar marked this pull request as ready for review July 7, 2025 09:25
@HassanAkbar HassanAkbar requested a review from Copilot July 7, 2025 09:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new LML parser and transformer to support model definitions, updates the main parser to load LML, and includes example LML fixtures.

  • Introduce Parslet-based LML grammar, transform rules, and AST node classes
  • Update lib/lutaml/parser.rb to require the new LML module
  • Minor cleanup in Uml::Document for lazy-initialized collections

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/fixtures/lml/iho_s102_check.lml Add fixture demonstrating model and attribute syntax
spec/fixtures/lml/iho_data_models.lml Add fixture for metadata and compliant standards
lutaml.gemspec Add metadata entry
lib/lutaml/parser.rb Require the new LML parser
lib/lutaml/uml/document.rb Switch to `
lib/lutaml/lml/parsers/class_definition*.rb Define LML grammar and transform logic
lib/lutaml/lml/parser.rb Main entry point for LML parsing
lib/lutaml/lml/nodes/*.rb Define node classes for the LML AST
lib/lutaml/lml/nodes.rb Entry-point for loading a subset of LML nodes
Comments suppressed due to low confidence (3)

lib/lutaml/lml/parsers/class_definition_transformer.rb:9

  • [nitpick] The file name class_definition_transformer.rb suggests a transformer class, but the class is named ClassDefinitionTransform. For consistency, consider renaming the class to ClassDefinitionTransformer or renaming the file to class_definition_transform.rb.
      class ClassDefinitionTransform < Parslet::Transform

lib/lutaml/lml/parser.rb:11

  • There's no associated test coverage for the new LML parsing entry point in lib/lutaml/lml/parser.rb. Consider adding unit tests to validate parsing and transformation of sample LML documents.
    def self.parse(input)

lib/lutaml/lml/nodes.rb:1

  • [nitpick] Consider requiring all LML node files (e.g., class_definition, attribute_definition, model, etc.) in lib/lutaml/lml/nodes.rb so consumers can load the full node DSL from a single require.
require_relative "nodes/property"

classes: sequence(:classes),
) do
Lutaml::Lml::Nodes::Model.new(
# requires: requires, TODO: Handle requires

Copilot AI Jul 7, 2025

Copy link

Choose a reason for hiding this comment

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

The TODO indicates that require statements are parsed but not applied to the resulting Model node. Implement mapping of the parsed requires into the Model (e.g., pass a requires: requires argument) or remove the TODO if not needed.

Suggested change
# requires: requires, TODO: Handle requires
requires: requires, # Pass requires to the Model constructor

Copilot uses AI. Check for mistakes.
@ronaldtse

Copy link
Copy Markdown
Contributor

Thanks @HassanAkbar , we will need to provide documentation and also replace (and complete) the old Lutaml::Uml parsing implementation.

@HassanAkbar

Copy link
Copy Markdown
Member Author

Thanks @HassanAkbar , we will need to provide documentation and also replace (and complete) the old Lutaml::Uml parsing implementation.

Yes, I will do that once this functionality is complete, the previous implementation is more ruby based so I will have to convert it into transformer class which is recommended by parslet.

@ronaldtse

Copy link
Copy Markdown
Contributor

@HassanAkbar I’m not a fan of the Parslet transformer architecture: there is no need to intentionally adopt it. My experience is it makes things hard to debug and its signature based rules make it hard to handle a number of cases.

@HassanAkbar

HassanAkbar commented Jul 16, 2025

Copy link
Copy Markdown
Member Author

@HassanAkbar I’m not a fan of the Parslet transformer architecture: there is no need to intentionally adopt it. My experience is it makes things hard to debug and its signature based rules make it hard to handle a number of cases.

@ronaldtse Then I think these can be merged into the UML rules, I'll update this PR by merging it with current UML implementation.

@ronaldtse

Copy link
Copy Markdown
Contributor

@HassanAkbar for this PR, we have actually redone the UML portion (by @kwkwan in #162 ) to strictly deal with UML. Could we move the LML language to a separate class/namespace, as LML is very much an extended superset of basic UML? Thanks!

@HassanAkbar

Copy link
Copy Markdown
Member Author

@ronaldtse I have updated the PR to separate the LML from UML by extending the UML classes. Can you take a look and let me know if this is how we want to do it? or do we need to change something here? Thanks

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.

3 participants