getify/TypL

Always allow assignment to tagged:"any"

mraak opened this issue · 18 comments

mraak commented
  • Always allow assignment to tagged:"any" but don't overwrite the type
  • allow call-expression arguments if the corresponding parameter type is any
  • if a function signature's return is set to any, return statements need to allow any return values
  • apply isAssignemntAllowed in other relevant places: #28 (review)

Example:

var a = any``;
a = "hello";  // OK
a = 1; // also OK because `a` is still type `any`

var b = a + 2;   // error: mixed operand types: 'any' and 'number'
mraak commented

Note (to self):

function typesMatch(type1,type2) {
 var type1ID = getTypeID(type1);
 var type2ID = getTypeID(type2);
 return (
  type1ID == "any" ||         // <==============
  type1ID != "unknown" &&
  type2ID != "unknown" &&
  type1ID === type2ID
};
}

I don't think this check can go here, since typesMatch(..) is used in lots of places that don't relate to assignment, and this exception only applies to assignment. I think it needs to go into handleAssignmentExpressionType(..).

mraak commented
	if (!typesMatch(sourceType,targetType)) {
						if (targetType.inferred == "undef"){
							delete targetType.inferred;
							Object.assign(targetType,sourceType);

							// NOTE: temporary debugging output
							if (isTaggedType(sourceType)) {
								addOutputMessage({
									id: MSG.INFO_REIMPLY_UNDEF_TAGGED,
									text: `Re-implying ${targetNode.name} with tagged-type '${sourceTypeID}'`,
									node: targetNode,
								});
							}
							else {
								addOutputMessage({
									id: MSG.INFO_REIMPLY_UNDEF_INFERRED,
									text: `Re-implying ${targetNode.name} to inferred-type '${sourceTypeID}'`,
									node: targetNode,
								});
							}
						}	else if(targetTypeID != "any") {  // <===========
							reportUnexpectedType(
								MSG.ERR_ASSIGNMENT_TYPE,
								"Assignment type mismatch",
								sourceType,
								targetType,
								exprNode
							);
						}
					}

Something like this maybe....

Yeah I think that might work for the assignment. But we'll also need to add some other handling as well. For example:

  • allow call-expression arguments if the corresponding parameter type is any
  • if a function signature's return is set to any, return statements need to allow any return values
mraak commented

Shall these two cases rather go into tickets for arguments/return handling, or is it better doing it here?

Yeah I want to handle all cases with any in this ticket... there may be a few others we haven't fully thought through yet.

mraak commented

allow call-expression arguments if the corresponding parameter type is any

Is this test case good enough?

function foo(a = string){}
function bar(a = any){}

foo(1) // error: argument type missmatch
bar(1) // pass

if a function signature's return is set to any, return statements need to allow any return values

Need test case for this one.

That's good. Also:

function foo(x) { return x; }
function bar(y) { return y; }

var a = foo(any`3`);
var b = bar(4);

foo(b);  // no error
bar(a);  // argument type error

a = bar(5);  // ok
b = foo("oops");  // assignment type error (b)
mraak commented

It reports this, so I think we might be good here.

(pass 1) ------------------------
Implying foo as inferred-type 'func'
Function 'foo' signature: {"type":"func","params":[{"inferred":"unknown"}],"hasRestParam":false,"return":{"default":true,"inferred":"undef"}}
Implying bar as inferred-type 'func'
Function 'bar' signature: {"type":"func","params":[{"inferred":"unknown"}],"hasRestParam":false,"return":{"default":true,"inferred":"undef"}}
Implying parameter x from argument, as tagged-type 'any'
Implying parameter y from argument, as inferred-type 'number'
(pass 2) ------------------------
Function 'foo' signature: {"type":"func","params":[{"tagged":"any"}],"hasRestParam":false,"return":{"tagged":"any"}}
Function 'bar' signature: {"type":"func","params":[{"inferred":"number"}],"hasRestParam":false,"return":{"inferred":"number"}}
Implying a as tagged-type 'any'
Implying b as inferred-type 'number'
Argument type mismatch: expected type 'number', but found type 'undef'
Assignment type mismatch: expected type 'number', but found type 'any

There's a bug here somehow:

Argument type mismatch: expected type 'number', but found type 'undef'

That should say "expected type 'number' but found type 'any'".

The return value of foo(..) is any on the second pass, and thus a gets implied as any, so then when passing a as the argument, its type should still be any.

mraak commented

I see. Is this to be resolved in this ticket or is it a part of multipass? Because without this, I can already send a PR.

I don't think it's related to multi-pass. But it might be.

If you don't think you can find/fix the bug, let's just file it as a separate issue, then you can close/merge the PR and branch, and I can take a look at why it's happening.

mraak commented

Check it out, I created a PR. I'll also have a look. I added line numbers to the output, I think it helps a lot.

Interesting that in the output you can see that it is implying it as any, but then sees it as undef later.

Fixed. See PR #26.


nice job on the line numbers, btw. That was on my list of things to get to (that's why I added the whole node value to addOutputMessage(..).

mraak commented

Merged and I see the test case now works as it should.

BTW, make sure you look at #27 that I just filed. Nothing to do (it's not a bug), but it's a nuanced quirk that we should both keep in mind.

mraak commented

I believe this one could be closed now.