Skip to content

Fixes: #135 Type inference assigns boxed type#271

Open
AnAvailableName wants to merge 15 commits into
mainfrom
type-inference
Open

Fixes: #135 Type inference assigns boxed type#271
AnAvailableName wants to merge 15 commits into
mainfrom
type-inference

Conversation

@AnAvailableName

Copy link
Copy Markdown
Collaborator

No description provided.

""".formatted(summeryBody);
}

private record CheckInferredFeatureTypeFixture(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would move this record to before it's used for the test sources. Also, we could create a separate test class for inferring the primitive types correctly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As for splitting the test class in tow, I concur, but it I would isolate the test cases for checking the
inferred type first. If you like me to do so I could also separate the primitive type casts as suggested.

As for the record, your sentence is incomplete; I assume you disliked the (static) inner class.
My thinking here was that it only makes sense in the context of this class.
Thus it is not needed any where else and I thought would only pollute the public namespace if moved to top level.
Also by isolating the tests for checking the inferred type the record (which is now called CheckInferredFeatureTypeTuple)
now lives as an (static) inner class of a separate class.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isolating the test for the inferred types is fine for me, and so is the static inner class. I just meant that, reading the test from top to bottom, it takes a long time until I see what a CheckInferredFeatureTypeFixture contains. Therefore, I wondered whether we should move the static inner class to "before it's used", i.e., towards the top of the surrounding class.

@AnAvailableName AnAvailableName requested a review from larsk21 June 24, 2026 12:07
Comment on lines +114 to +118
var result = internalParse("""
export package to "http://example.com"

assertThat(result).hasIssues("Cannot infer type");
}
import "http://example.org/restaurant" as rest
import "http://example.org/reviewpage"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need the internalParse with the two imports here? Is the reviewpage metamodel used in this test?

.isEqualTo(fixture.expectedClassifier);
}

private EClassifier getClassifierOrFail(TypeInfo typeInfo) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is getFeatureOrFail in a separate class, but getClassifierOrFail and inferredTypeOrFail are not?

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