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.

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

Resolution of null allocation and type ambiguity issues #3

Merged
merged 13 commits into from
Aug 29, 2024

Conversation

Republicof1
Copy link
Contributor

This branch corrects both issues #1 and #2 with all tests passing

this avoids an ambiguity between nothing and nothing[] types, replacing it with a more permissive any[] type.
This allows the item to carry an effective null without making at a nullable object or breaching type correctness. Item will still report null when queried.
@pahjbo
Copy link
Member

pahjbo commented Aug 27, 2024

I am a little uneasy about this PR as it does not seem to be fully addressing the type and structural issues that this code has - the code is mostly autogenerated from jsofa by jsweet and does pass all its unit tests (with the tsconfig options as saved). One problem is the speed of breaking changes that happen in the Typescript/Javascript world, and for instance the ESLint current version has a different configuration that is not backwards compatible.

Beginning on the path of making manual edits, I feel should start to address more of the "faults" that exist - An example of one of the transformations that has been done is to replace let x = [] with let x = new Array() - this occurs many times where what is actually desired is "array of number" so the replacement does not express this. Looking at the even bigger picture, this occurs in a locally defined functions that are created at every point where an array allocation is occurring - it would be nicer to have a single function definition that was reused. There is another even bigger picture view of this particular issue, in that the array allocation is done in some places where it is immediately thrown away, or that array initialisation would be a more 'natural' way to have written the code

In other places the typing should probably be tightened to express the real intended difference in meaning between undefined and null

pahjbo and others added 8 commits August 27, 2024 15:11
@Republicof1
Copy link
Contributor Author

Following the ESLint update, I have remedied all of the issues it currently raised

I replaced the anonymous function array allocation with static function that encapsulates the behaviour in a slightly clearer way. I've also resolved all residual ESLint complaints.

these are returned as the function value - so were overwritten.
Copy link
Member

@pahjbo pahjbo left a comment

Choose a reason for hiding this comment

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

I think this looks better now with the single array allocation function

@pahjbo pahjbo merged commit 7b908ab into main Aug 29, 2024
1 check passed
@pahjbo pahjbo deleted the 2-allocation-of-null-error branch August 29, 2024 15:18
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