typeToTypeNode behavior changed for boolean literals

[Gerrit0]

TypeScript Version: 4.1.0-dev.20200823, 4.0.2

Search Terms:
TypeLiteral, TypeLiteralNode, boolean literal type

Code

//@ts-check
const fs = require("fs");
const ts = require("typescript");

fs.writeFileSync("test.ts", "export const x = true");

const program = ts.createProgram({ options: {}, rootNames: ["test.ts"] });
const checker = program.getTypeChecker();
const sourceFile = program.getSourceFile("test.ts");
const fileSymbol = checker.getSymbolAtLocation(sourceFile);

const xSymbol = fileSymbol.exports.values().next().value;
const xType = checker.getTypeOfSymbolAtLocation(xSymbol, sourceFile);

console.log(`${xSymbol.name}: ${checker.typeToString(xType)}`); // x: true

const xTypeNode = checker.typeToTypeNode(
  xType,
  sourceFile,
  ts.NodeBuilderFlags.IgnoreErrors
);

console.log("Literal according to type?", xType.isLiteral());
console.log("Literal according to node?", ts.isLiteralTypeNode(xTypeNode));

Expected behavior:

TS 3:

C:\Users\gtbir\Desktop\issue> node .\test.js       
x: true
Literal according to type? false
Literal according to node? false

Actual behavior:

TS 4:

C:\Users\gtbir\Desktop\issue> node .\test.js
x: true
Literal according to type? false
Literal according to node? true 

Playground Link: N/A

Related Issues: #26075 also discusses whether the intrinsic boolean types should be considered literals, the consensus there seemed to be that true and false ought to be literals... and TS 4 seems to be halfway there, which results in a confusing mismatch.

[rbuckton]

When we shipped the new NodeFactory API in 4.0, we tried to clean up some confusing API designs and unsoundness. In general, it is bad for keyword tokens in "type"-space and "value"-space to overlap, as it makes it much more difficult for emit transformers to know whether they're working with the value true, vs. the type true. We were already parsing a true type as a LiteralTypeNode, but the checker was incorrectly returning a TrueKeyword or FalseKeyword token for typeToTypeNode.

Another thing we changed was that the null type is now also represented as a LiteralTypeNode, even though its not precisely a literal type. This is again because we need to avoid confusion when transformers encounter a NullKeyword token and need to be able to determine whether they are looking at the value null, vs the type null.

Regarding .isLiteral(), this function is poorly named and currently only checks whether a type is either a StringLiteral or NumericLiteral type:

TypeScript/src/services/services.ts

Lines 480 to 482 in 9dfbf07

	isLiteral(): this is LiteralType {
	return !!(this.flags & TypeFlags.StringOrNumberLiteral);
	}

I would expect the output to be the following:

C:\Users\gtbir\Desktop\issue> node .\test.js
x: true
Literal according to type? false
Literal according to node? true 

The only thing that might change is if we decide to change isLiteral() to check for TypeFlags.Literal instead (widening its set of possible values), however it is unknown whether that would be a breaking change for service consumers using the API.

@RyanCavanaugh, @DanielRosenwasser any thoughts on whether isLiteral() should include other literal types like BooleanLiteral or BigIntLiteral? Seems like it may have been an oversight.

[Gerrit0]

This makes sense to me - I'd be happy to close this if the API breaking changes page on the wiki was updated to make a note of this.

[rbuckton]

I've updated the API Breaking Changes page in the wiki to reflect that typeToTypeNode now correctly produces a LiteralTypeNode for true and false types to match the existing parser behavior.

[Gerrit0]

Thanks!