Skip to content

Return type accuracy and usage improvements #7891

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

Open
wants to merge 8 commits into
base: dev/feature
Choose a base branch
from

Conversation

APickledWalrus
Copy link
Member

@APickledWalrus APickledWalrus commented May 23, 2025

Problem

Some Expressions/Classes are not taking full advantage of the multiple return types system. Others have return types that are incorrectly setup. Ensuring detailed return types is also important for features like local variable type hints.

Solution

I aim to integrate usage of Expression#possibleReturnTypes and Expression#canReturn where possible. I've also cleaned up some classes that were in need of it (to improve their return type logic).

A major change that I made is Expression#canReturn will always return true if one of the possible return types is Object. This appears to work without issue, but further testing is needed.

ExprTernary and ExprDefaultValue have been rewritten and their generics removed. I do not believe these are necessary anymore. The generic ConvertedExpression implementation is capable of ensuring type safety.

Testing Completed

There is existing coverage of most of the modified syntax. I have added tests for ExprTernary and ExprDefaultValue since they were rewritten and previously did not have any tests.

Supporting Information

n/a


Completes: Fixes #5060
Related: none

@APickledWalrus APickledWalrus requested a review from a team as a code owner May 23, 2025 17:34
@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label May 23, 2025
@APickledWalrus APickledWalrus requested a review from a team as a code owner May 23, 2025 17:34
@APickledWalrus APickledWalrus added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label May 23, 2025
@APickledWalrus APickledWalrus removed the request for review from a team May 23, 2025 17:34
@APickledWalrus APickledWalrus added the needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. label May 23, 2025
@APickledWalrus APickledWalrus requested review from UnderscoreTud and cheeezburga and removed request for a team May 23, 2025 17:34
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label May 23, 2025
@APickledWalrus APickledWalrus changed the title Return type improvements Return type accuracy and usage improvements May 23, 2025
@APickledWalrus APickledWalrus requested a review from sovdeeth June 2, 2025 20:29
@sovdeeth sovdeeth moved this to Needs Reviews in 2.12 Release Jun 3, 2025
@APickledWalrus APickledWalrus mentioned this pull request Jun 3, 2025
@APickledWalrus APickledWalrus linked an issue Jun 4, 2025 that may be closed by this pull request
1 task
@skriptlang-automation skriptlang-automation bot added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 12, 2025
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Release Jun 12, 2025
@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls.
Projects
Status: Awaiting Merge
Development

Successfully merging this pull request may close these issues.

Some elements don't handle Object return types well
3 participants