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.

Sign up for GitHub

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

initial formatter schema #12

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

initial formatter schema #12

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
795b99c
initial formatter schema
maytheu Sep 11, 2023
a4d238e
conceptual entity formatter
maytheu Sep 12, 2023
4517414
formatter for conceptual element
maytheu Sep 14, 2023
16d7123
.
maytheu Sep 14, 2023
39b3605
test passes with no formmater
maytheu Sep 18, 2023
cfe58ae
improve test cases
maytheu Sep 19, 2023
731cd27
remove extra lines from test
maytheu Sep 28, 2023
5c61e26
logical model formatter
maytheu Sep 28, 2023
7693f72
platform data model formatter
maytheu Sep 29, 2023
f2333d4
format association before entities
maytheu Oct 2, 2023
5a56353
ldm test
maytheu Oct 4, 2023
f196456
formatting platform element
maytheu Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 150 additions & 4 deletions src/language-server/universal-data-definition-language-formatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,43 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
}
else if(ast.isConceptualEntity(node)){
this.formatConceptualEntity(node)
}
}
else if(ast.isLogicalEnumerated(node)){
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you choose the ordering of if-else if ... here? Make sure to document it. You don't want someone else to come along after you and make some change to the ordering that causes problems.

this.formatLogicalEnumerated(node)
}
else if(ast.isLogicalMeasurement(node)){
this.formatLogicalMeasurement(node)
}
else if(ast.isLogicalMeasurementSystemAxis(node)){
this.formatLogicalMeasurementSystemAxis(node)
}
else if(ast.isLogicalValueTypeUnit(node)){
this.formatLogicalValueTypeUnit(node)
}
else if(ast.isLogicalMeasurement(node)){
this.formatLogicalMeasurement(node)
}
else if(ast.isLogicalEntity(node)){
this.formatLogicalEntity(node)
}
else if(ast.isLogicalAssociation(node)){
Copy link
Contributor

Choose a reason for hiding this comment

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

All LogicalAssociations are also LogicalEntities => Swap lines 55-57 and 58-60 so you check for LogicalAssociation first

this.formatLogicalAssociation(node)
}
else if(ast.isLogicalParticipantPathNode(node)){
this.formatLogicalParticipantPathNode(node)
}
else if(ast.isPlatformEntity(node)){
this.formatPlatformEntity(node)
}
else if(ast.isPlatformStruct(node)){
this.formatPlatformStruct(node)
}
else if(ast.isPlatformAssociation(node)){
Copy link
Contributor

Choose a reason for hiding this comment

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

All PlatformAssociations are also PlatformEntities. The check isPlatformAssociation must be before the check isPlatformEntity .

this.formatPlatformAssociation(node);
}
else if(ast.isPlatformCompositeQuery(node)){
this.formatPlatformCompositeQuery(node);
}
}


Expand Down Expand Up @@ -103,7 +139,6 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter

protected formatConceptualAssociation(cassoc: ast.ConceptualAssociation): void {
this.formatContainer(cassoc)

cassoc.composition.forEach(comp => {
this.formatConceptualComposition(comp)
})
Expand All @@ -114,8 +149,7 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
}

protected formatConceptualParticipant(participant: ast.ConceptualParticipant): void {
this.formatContainer(participant)

this.formatContainer(participant)
if(participant?.type){
this.formatConceptualEntity(participant.type.ref!)
}
Expand All @@ -128,11 +162,123 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
this.formatObj(elem);
}

protected formatLogicalEnumerated(lenum: ast.LogicalEnumerated): void {
this.formatContainer(lenum)
lenum.label.forEach(label => {
this.formatContainer(label)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you calling formatContainer on label? It's not a container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The label is an array of LogicalEnumeratedSet | LogicalEnumerationLabel which are both containers

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be a LogicalEnumeratedSet, which is a container, or a LogicalEnumerationLabel, which is not a container. In these circumstances, remember that the top level format method will figure out the type of label - so all you need to do is call format, which will then call the appropriate formatter for either LogicalEnumerationLabel or LogicalEnumeratedSet. Your current approach will not handle multiple levels of nesting appropriately.

})
}

protected formatLogicalMeasurementSystem(sys: ast.LogicalMeasurementSystem): void {
this.formatContainer(sys);
sys.constraint.forEach(sysConst => {
Copy link
Contributor

Choose a reason for hiding this comment

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

measurementSystemAxis is also a collection.

this.formatContainer(sysConst)
})
sys.referencePoint.forEach(ref => {
this.formatLogicalReferencePoint(ref);
})
}

protected formatLogicalMeasurementSystemAxis(sysAxis: ast.LogicalMeasurementSystemAxis): void {
this.formatContainer(sysAxis);
sysAxis.constraint.forEach(sysConst => {
this.formatContainer(sysConst);
Copy link
Contributor

Choose a reason for hiding this comment

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

Individual constraints are not containers.

})
}

protected formatLogicalReferencePoint(refPoint: ast.LogicalReferencePoint): void {
this.formatContainer(refPoint);
refPoint.referencePointPart.forEach(pointPart => {
this.formatContainer(pointPart);
Copy link
Contributor

Choose a reason for hiding this comment

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

An individual ReferencePointPart is not a container

})
}

protected formatLogicalValueTypeUnit(typeUnit: ast.LogicalValueTypeUnit):void {
this.formatContainer(typeUnit);
if(typeUnit.constraint){
this.formatContainer(typeUnit);
}
}

protected formatLogicalMeasurement(measure: ast.LogicalMeasurement): void {
this.formatContainer(measure);
measure.attribute.forEach(attr => {
this.formatContainer(attr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Individual attributes are not containers

})
measure.constraint.forEach(mesConst => {
this.formatContainer(mesConst)
Copy link
Contributor

Choose a reason for hiding this comment

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

individual constraints are not containers

})
}

protected formatLogicalEntity(entity: ast.LogicalEntity): void {
this.formatContainer(entity);
entity.composition.forEach(comp => {
this.formatContainer(comp);
Copy link
Contributor

Choose a reason for hiding this comment

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

Individual composition elements are not containers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads up, will make the changes and revert. I will get back if I am stuck.

})
}

protected formatLogicalAssociation(asso: ast.LogicalAssociation): void {
this.formatContainer(asso);
asso.composition.forEach(comp => {
this.formatContainer(comp);
})
asso.participant.forEach(part => {
this.formatLogicalParticipant(part);
})
}

protected formatLogicalParticipant(part: ast.LogicalParticipant): void {
this.formatContainer(part)
}

protected formatLogicalParticipantPathNode(pathBode: ast.LogicalParticipantPathNode): void {
this.formatContainer(pathBode)
}

protected formatLogicalCompositeQuery(query: ast.LogicalCompositeQuery): void {
this.formatContainer(query)
query.composition.forEach(comp => {
this.formatContainer(query)
})
}

protected formatPlatformElement(elem: ast.PlatformElement): void {
const formatter = this.getNodeFormatter(elem);
formatter.property('name').prepend(Formatting.newLine()).surround(Formatting.oneSpace({allowMore: true}));
this.formatObj(elem);
}

protected formatPlatformEntity(entity: ast.PlatformEntity): void {
this.formatContainer(entity)
entity.composition.forEach(comp => {
this.formatContainer(comp)
})
}

protected formatPlatformStruct(str: ast.PlatformStruct): void {
this.formatContainer(str)
str.member.forEach(mem => {
this.formatContainer(mem)
})
}

protected formatPlatformAssociation(asso: ast.PlatformAssociation): void {
this.formatContainer(asso)
asso.participant.forEach(part => {
this.formatPlatformParticipant(part);
})
}

protected formatPlatformParticipant(part: ast.PlatformParticipant): void {
this.formatContainer(part)
}

protected formatPlatformCompositeQuery(query: ast.PlatformCompositeQuery): void {
this.formatContainer(query)
query.composition.forEach(comp => {
this.formatContainer(comp);
})
}
}


Expand Down
15 changes: 5 additions & 10 deletions uddl_test/uddl_test_formatting/universal-data-definition-language-formatter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ const universalDataDefinitionLanguageFormatting = expectFormatting(universalData
describe("Universal Data Definition Language Formatter", () => {
it("should format cdm array", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cdm anothercdm "A cdm with another data" {observable Information "Something a party can learn";}}}',
before:'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cdm anothercdm "A cdm with another data" {observable Information "Something a party can learn";}}}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

Expand All @@ -30,8 +29,7 @@ describe("Universal Data Definition Language Formatter", () => {

it("should format cdm with conceptual association", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cassoc Training "Information delivered over time from one party to another " {};}}',
before:'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cassoc Training "Information delivered over time from one party to another " {};}}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

Expand All @@ -51,8 +49,7 @@ describe("Universal Data Definition Language Formatter", () => {

it("should format cdm with conceptual entity", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{centity AddressableEntity "Any entity that is addressable in some way" {};}}',
before:'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{centity AddressableEntity "Any entity that is addressable in some way" {};}}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

Expand All @@ -72,8 +69,7 @@ describe("Universal Data Definition Language Formatter", () => {

it('should format cdm with conceptual basis',async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

While you can have tests for each individual type, you may find that a test that combines multiple types is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a previous commit, I merged conceptual entities and conceptual associations into one test case. During this process, only formatter for conceptual entities was checked. I then attempted to verify whether I would encounter similar behavior for conceptual basis and conceptual domain as well.

await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{basis uddlBasis "Formating conceptual basis entity"; }}',
before: 'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{basis uddlBasis "Formating conceptual basis entity"; }}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

Expand All @@ -89,8 +85,7 @@ describe("Universal Data Definition Language Formatter", () => {

it('should format cdm with conceptual domain',async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{domain NaturalPerson "Base definition of a natural person"; }}',
before: 'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{domain NaturalPerson "Base definition of a natural person"; }}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

Expand Down