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

First attempt at enable noUncheckedIndexedAccess TS compiler option #4039

Closed
wants to merge 1 commit into from

Conversation

syhol
Copy link
Contributor

@syhol syhol commented Dec 30, 2023

Implementing as requested in the issue #937

I'm aware this is a controversial topic, however I would find it useful if Deno's std library was compatible with Typescript's new noUncheckedIndexedAccess flag, since I find my projects that use it severely handicapped [...]

Comment on lines +44 to +46
const major = parseInt(groups.major!);
const minor = parseInt(groups.minor!);
const patch = parseInt(groups.patch!);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignoring the lint error with ! as parseInt(undefined) will parse to NaN and I've added a new isNaN check below.

if (i >= 0) path = pathSegments[i];
else {
if (i >= 0) {
path = pathSegments[i]!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i starts at pathSegments.length and decrements down to -1, however this statement is protected with i >= 0 so the i should always be inbounds.

Comment on lines +68 to 69
for (; newLength > 1 && c.seps.includes(glob[newLength - 1]!); newLength--);
glob = glob.slice(0, newLength);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there might be a bug here, the semicolon at the end of the for statement means line below is only ever run once. The for loop itself does nothing.

@iuioiua
Copy link
Contributor

iuioiua commented Dec 30, 2023

Great to see this being worked on! Implementing this feature will require a bit of work across the entire codebase. We should create a tracking issue for this to break PRs up. Perhaps, this PR can focus on the sub-modules it already touches. WDYT?

@syhol
Copy link
Contributor Author

syhol commented Dec 30, 2023

Never mind, there are many more issues than I first thought. Will spend some more time on this before I submit a PR.

@syhol syhol closed this Dec 30, 2023
@iuioiua
Copy link
Contributor

iuioiua commented Dec 30, 2023

To elaborate, I think the tracking issue should list each sub-module as a to-do list and include instructions on how to work on the issue. It'd be good to get this done before the Standard Library reaches stability.

@syhol syhol changed the title chore: enable noUncheckedIndexedAccess TS compiler option First attempt at enable noUncheckedIndexedAccess TS compiler option Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants