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

♻️ Use bigint for long types in mcdoc #1487

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NeunEinser
Copy link
Contributor

No description provided.

@misode misode changed the title Use bigint for long types in mcdoc ♻️ Use bigint for long types in mcdoc Jul 7, 2024
Copy link
Member

@misode misode left a comment

Choose a reason for hiding this comment

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

Some of these unsafe casts make me a bit nervous, but I don't immediately see a clean way to get rid of them

@@ -498,7 +500,8 @@ function checkShallowly<T>(
if (
typeDef.valueRange
&& simplifiedInferred.kind === 'literal'
&& typeof simplifiedInferred.value.value === 'number'
&& simplifiedInferred.value.kind !== 'string'
&& simplifiedInferred.value.kind !== 'boolean'
Copy link
Member

Choose a reason for hiding this comment

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

There are probably a few other places, maybe in attributes, where this pattern was used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure that's untrue. This one I cought because it caused a build error, so I don't see why this wouldn't happen in other places.

@@ -1001,7 +1009,7 @@ function convertLiteralValue(node: LiteralTypeValueNode, ctx: McdocBinderContext
kind: convertLiteralNumberSuffix(suffix, ctx)
?? (value.type === 'integer' ? 'int' : 'double'),
value: value.value,
}
} as LiteralValue
Copy link
Member

Choose a reason for hiding this comment

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

This PR adds a couple of these unsafe casts, if you find a way to get rid of them that would be nice

Copy link
Contributor Author

@NeunEinser NeunEinser Jul 15, 2024

Choose a reason for hiding this comment

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

In some places, you can do

return kind === 'long'
  ? { kind, value }
  : { kind, value }

to satisfy the type checker, but it's also a bit weird ¯\_(ツ)_/¯

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