-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
#6 Number subtypes #28
Conversation
…into #6_number-sub-types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than those two tweaks I just commented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, line 372 needs to change to the isAssignmentAllowed(..)
check.
@@ -1457,7 +1449,7 @@ function handleCallExpression(scope,callExprNode,isSelfRecursivePTC = false) { | |||
let argSignature = typeSignatures.get(argType); | |||
|
|||
if (paramTypeID != "unknown") { | |||
if (!typesMatch(argType,paramType)) { | |||
if (!typesMatch(argType,paramType) && paramTypeID != "any") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line can just reference the isAssignmentAllowed(..)
check now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You think this and line 372 are safe to change to use isAssignmentAllowed
? I haven't tested for this. Also, these locations don't actually deal with assignments so maybe a more generic function name would be appropriate. Maybe to change isAssignmentAllowed
to typesCompatible
, that way it would function similar to typesMatch
but more permissive.
@@ -1672,19 +1671,20 @@ function isAssignmentAllowed(sourceType, targetType){ | |||
} | |||
|
|||
let matches = { | |||
any: recognizedTypeIDs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice... this is a great way to handle it. Thanks!
No description provided.