Skip to content

Conversation

@NilsWeidmann
Copy link
Member

When the type system expects a certain concept, we should not use the colon cast (:) to cast nodes to a different type but rather use the as cast. This cast returns null if the cast is not successful, for example when a user provides a node of a wrong type or the RuntimeErrorType is returned. I've fixed a few of such issues in the past but there are so many hidden cases, it's probably better to just ban this cast and replace all instances in the type system.

@NilsWeidmann NilsWeidmann requested a review from nkoester December 5, 2025 12:01
@NilsWeidmann NilsWeidmann marked this pull request as ready for review December 5, 2025 12:03
@nkoester
Copy link
Member

nkoester commented Dec 9, 2025

fixes #688

@nkoester
Copy link
Member

nkoester commented Dec 9, 2025

This looks good to me, but because there are so many affected nodes, I would like to ask some of the others reviewers to also have a look. @arimer @jonaskraemer thanks!

nkoester
nkoester previously approved these changes Dec 9, 2025
Copy link
Member

@arimer arimer left a comment

Choose a reason for hiding this comment

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

Could you please update the changelog as well?

@arimer arimer self-requested a review December 10, 2025 12:04
Copy link
Member

@arimer arimer left a comment

Choose a reason for hiding this comment

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

Please updated the changelog.

@nkoester
Copy link
Member

Please updated the changelog.

done

@arimer arimer self-requested a review December 10, 2025 13:05
@arimer
Copy link
Member

arimer commented Dec 10, 2025

@nkoester what was the motivation for not only converting TS rules but also: quickfixes, helper classes, behaviour methods etc.?

@nkoester
Copy link
Member

All of these were within the typesystem and the context thereof. Or at least called from the typesystem. From my perspective as casts do not introduce harm if they are even used somewhere else. I vote to keep this PR as is, however if you have doubts or other considerations we can surely adapt the PR accordingly.

Copy link
Member

@arimer arimer left a comment

Choose a reason for hiding this comment

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

let's go 🚀

@arimer arimer enabled auto-merge December 10, 2025 18:40
@arimer arimer merged commit 4302b06 into maintenance/mps20241 Dec 10, 2025
2 checks passed
@arimer arimer deleted the bugfix/colon-cast branch December 10, 2025 18:59
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.

4 participants