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

Add transpileDeclaration API method #58261

Merged
merged 8 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ function getSourceMapFilePath(jsFilePath: string, options: CompilerOptions) {
}

/** @internal */
export function getOutputExtension(fileName: string, options: CompilerOptions): Extension {
export function getOutputExtension(fileName: string, options: Pick<CompilerOptions, "jsx">): Extension {
return fileExtensionIs(fileName, Extension.Json) ? Extension.Json :
options.jsx === JsxEmit.Preserve && fileExtensionIsOneOf(fileName, [Extension.Jsx, Extension.Tsx]) ? Extension.Jsx :
fileExtensionIsOneOf(fileName, [Extension.Mts, Extension.Mjs]) ? Extension.Mjs :
Expand Down
1 change: 1 addition & 0 deletions src/harness/harnessIO.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ export namespace Compiler {
{ name: "noTypesAndSymbols", type: "boolean", defaultValueDescription: false },
// Emitted js baseline will print full paths for every output file
{ name: "fullEmitPaths", type: "boolean", defaultValueDescription: false },
{ name: "reportDiagnostics", type: "boolean", defaultValueDescription: false }, // used to enable error collection in `transpile` baselines
];

let optionsIndex: Map<string, ts.CommandLineOption>;
Expand Down
2 changes: 1 addition & 1 deletion src/harness/runnerbase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
} from "./_namespaces/Harness";
import * as ts from "./_namespaces/ts";

export type TestRunnerKind = CompilerTestKind | FourslashTestKind | "project";
export type TestRunnerKind = CompilerTestKind | FourslashTestKind | "project" | "transpile";
export type CompilerTestKind = "conformance" | "compiler";
export type FourslashTestKind = "fourslash" | "fourslash-server";

Expand Down
76 changes: 70 additions & 6 deletions src/services/transpile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
normalizePath,
optionDeclarations,
parseCustomTypeOption,
ScriptTarget,
toPath,
transpileOptionValueCompilerOptions,
} from "./_namespaces/ts";
Expand All @@ -51,14 +52,64 @@ const optionsRedundantWithVerbatimModuleSyntax = new Set([

/*
* This function will compile source text from 'input' argument using specified compiler options.
* If not options are provided - it will use a set of default compiler options.
* If no options are provided - it will use a set of default compiler options.
* Extra compiler options that will unconditionally be used by this function are:
* - isolatedModules = true
* - allowNonTsExtensions = true
* - noLib = true
* - noResolve = true
* - declaration = false
*/
export function transpileModule(input: string, transpileOptions: TranspileOptions): TranspileOutput {
return transpileWorker(input, transpileOptions, /*declaration*/ false);
}

/*
* This function will create a declaration file from 'input' argument using specified compiler options.
* If no options are provided - it will use a set of default compiler options.
* Extra compiler options that will unconditionally be used by this function are:
* - isolatedDeclarations = true
* - isolatedModules = true
* - allowNonTsExtensions = true
* - noLib = true
* - noResolve = true
* - declaration = true
* - emitDeclarationOnly = true
* Note that this declaration file may differ from one produced by a full program typecheck,
* in that only types in the single input file are available to be used in the generated declarations.
*/
export function transpileDeclaration(input: string, transpileOptions: TranspileOptions): TranspileOutput {
return transpileWorker(input, transpileOptions, /*declaration*/ true);
}

// Declaration emit works without a `lib`, but some local inferences you'd expect to work won't without
// at least a minimal `lib` available, since the checker will `any` their types without these defined.
// Late bound symbol names, in particular, are impossible to define without `Symbol` at least partially defined.
// TODO: This should *probably* just load the full, real `lib` for the `target`.
const barebonesLibContent = `/// <reference no-default-lib="true"/>
interface Boolean {}
Copy link
Member

@DanielRosenwasser DanielRosenwasser Apr 19, 2024

Choose a reason for hiding this comment

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

I suppose transpileDeclaration truly becomes unsafe to use without isolatedDeclarations because an inferred Boolean | Function will get reduced.

Copy link
Member Author

Choose a reason for hiding this comment

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

Without loading a full lib at least (which is doable! Just probably not worth if you're going to force on isolatedDeclarations anyway), yeah. Also your imports are all going to be unknown/error any.

Copy link
Member Author

Choose a reason for hiding this comment

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

Specifically, this barebones lib is the strategic mini-lib we use in our test harness to silence all global checker-construction errors, with the Symbol addenda so unique symbol types can actually be inferred.

Copy link
Member

Choose a reason for hiding this comment

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

It would be interesting to see what you end up with if you just add index signatures like [prop: string | symbol]: any

Copy link
Member

Choose a reason for hiding this comment

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

(probably don't do that in this PR though)

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷‍♂️ even if the checker is recording the diagnostics when getEmitResolver is called right now, nothing goes and gets the semantic errors to report them. The emit might change if you add a test case with an inferred type that's sensitive to that, though.

interface Function {}
interface CallableFunction {}
interface NewableFunction {}
interface IArguments {}
interface Number {}
interface Object {}
interface RegExp {}
interface String {}
interface Array<T> { length: number; [n: number]: T; }
interface SymbolConstructor {
(desc?: string | number): symbol;
for(name: string): symbol;
readonly toStringTag: symbol;
}
declare var Symbol: SymbolConstructor;
interface Symbol {
readonly [Symbol.toStringTag]: string;
}`;
const barebonesLibName = "lib.d.ts";
Copy link
Member

Choose a reason for hiding this comment

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

Honestly this sorta weirds me out; is this something we should be formalizing in our real dts files somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, it's weirder that we can't typecheck unique symbol literals/calls without the skeleton of the global Symbol definitions present (since it's not like symbol types are ES6+ specific). That's probably the real underlying issue that needs fixing - we really aughta bake their presence into the checker like undefined if they're so central to checking a basic type-syntax construct. Though, there's probably some other annoying things, too, though - like I don't think we'll emit Omit types for generic spreads without the global Omit type present, which even this is currently missing. Eg,

export function f<T extends {x: number, y: number}>(x: T) {
    const {x: num, ...rest} = x;
    return rest;
}

has an inferred return type of Omit<T, "x">, but without Omit in the lib, it's just any (the error kind). There are a lot of "builtins" like this, whose fallback behavior when they aren't present isn't particularly good.

Personally, by default I think I'd rather load the fully correct lib for the options passed in, at least for transpileDeclaration, just because I think emitting export const a: any for export const a = Math.random() feels bad (even if it is an error under ID), but I don't know if that runs counter to the perf goals of the API (even if every invocation with the same lib could share cached lib AST copies). Still, I guess it comes down to a question of scope - loading a real lib to get the builtins to work is easy, but should you need to do that for isolatedDeclarations-compatible input? I appended this barebones lib to the input because I assumed at least unique symbol-related inferences are planned to work, and this is the minimal lib (well, OK, I could omit the empty Array interfaces and the like) to get that.

We could leave it up to the caller - and load the default lib for the options provided unless an override (like this minimal one or a completely empty/missing one) is passed in, or load no lib at all by default unless one is passed in as a transpile option. It's really a question of the intended usage, I suppose.

const barebonesLibSourceFile = createSourceFile(barebonesLibName, barebonesLibContent, { languageVersion: ScriptTarget.Latest });

function transpileWorker(input: string, transpileOptions: TranspileOptions, declaration?: boolean): TranspileOutput {
const diagnostics: Diagnostic[] = [];

const options: CompilerOptions = transpileOptions.compilerOptions ? fixupCompilerOptions(transpileOptions.compilerOptions, diagnostics) : {};
Expand Down Expand Up @@ -86,10 +137,19 @@ export function transpileModule(input: string, transpileOptions: TranspileOption
// Filename can be non-ts file.
options.allowNonTsExtensions = true;

if (declaration) {
options.declaration = true;
options.emitDeclarationOnly = true;
options.isolatedDeclarations = true;
}
else {
options.declaration = false;
}

const newLine = getNewLineCharacter(options);
// Create a compilerHost object to allow the compiler to read and write files
const compilerHost: CompilerHost = {
getSourceFile: fileName => fileName === normalizePath(inputFileName) ? sourceFile : undefined,
getSourceFile: fileName => fileName === normalizePath(inputFileName) ? sourceFile : fileName === normalizePath(barebonesLibName) ? barebonesLibSourceFile : undefined,
writeFile: (name, text) => {
if (fileExtensionIs(name, ".map")) {
Debug.assertEqual(sourceMapText, undefined, "Unexpected multiple source map outputs, file:", name);
Expand All @@ -100,12 +160,12 @@ export function transpileModule(input: string, transpileOptions: TranspileOption
outputText = text;
}
},
getDefaultLibFileName: () => "lib.d.ts",
getDefaultLibFileName: () => barebonesLibName,
useCaseSensitiveFileNames: () => false,
getCanonicalFileName: fileName => fileName,
getCurrentDirectory: () => "",
getNewLine: () => newLine,
fileExists: (fileName): boolean => fileName === inputFileName,
fileExists: (fileName): boolean => fileName === inputFileName || (!!declaration && fileName === barebonesLibName),
readFile: () => "",
directoryExists: () => true,
getDirectories: () => [],
Expand Down Expand Up @@ -135,14 +195,18 @@ export function transpileModule(input: string, transpileOptions: TranspileOption
let outputText: string | undefined;
let sourceMapText: string | undefined;

const program = createProgram([inputFileName], options, compilerHost);
const inputs = declaration ? [inputFileName, barebonesLibName] : [inputFileName];
const program = createProgram(inputs, options, compilerHost);

if (transpileOptions.reportDiagnostics) {
addRange(/*to*/ diagnostics, /*from*/ program.getSyntacticDiagnostics(sourceFile));
addRange(/*to*/ diagnostics, /*from*/ program.getOptionsDiagnostics());
}
// Emit
program.emit(/*targetSourceFile*/ undefined, /*writeFile*/ undefined, /*cancellationToken*/ undefined, /*emitOnlyDtsFiles*/ undefined, transpileOptions.transformers);
const result = program.emit(/*targetSourceFile*/ undefined, /*writeFile*/ undefined, /*cancellationToken*/ undefined, /*emitOnlyDtsFiles*/ declaration, transpileOptions.transformers, /*forceDtsEmit*/ declaration);
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we should set forceDtsEmit this one generates d.ts file even if declaration options are not set correctly and thats not what we are looking for right? It also generates output even if there are diagnostics. is that intentional.

Eg normally d.ts is not generated for json but this will generate it and i dont think we want that.

Copy link
Member Author

@weswigham weswigham Apr 24, 2024

Choose a reason for hiding this comment

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

is that intentional.

Yes. transpileModule only fails (with a debug assert!) when the output isn't emitted at all, and will happily emit buggy output alongside diagnostics (if you ask for them). This keeps that behavior aligned between the two.

forceDtsEmit this one generates d.ts file even if declaration options are not set correctly and thats not what we are looking for right?

We set those options right above. We need to set forceDtsEmit so we still get output when the input has a parse error (because being able to return those errors is contingent on there being output at all).

Copy link
Member Author

Choose a reason for hiding this comment

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

Eg normally d.ts is not generated for json but this will generate it and i dont think we want that.

Honestly, if someone explicitly passes json into this API, it's because they wanted a declaration file for it.

Choose a reason for hiding this comment

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

JSON support sounds like a useful feature we might use. Our system treats JSON modules as frozen so potentially we could postprocess the output to express this readonly nature.

Copy link
Member

Choose a reason for hiding this comment

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

Just noting the differences between our normal emit vs setting forceDtsEmit which i had added for incremental checks


// TODO: Should this require `reportDiagnostics`?
Copy link
Member

Choose a reason for hiding this comment

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

Is it the case that the JS emit pipeline never reports errors?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct - emit pipeline errors were added for normal declaration emit when we ported them to transforms (and they add visibility errors very late) and are unused by js emit transforms -isolatedDeclarations continues to use the mechanism extensively. It's a longstanding issue I've wanted to fix, since it means we can't get declaration emit errors without actually running declaration emit (remind yourself how getPreEmitDiagnostics works some time 😑), but it's super annoying to try and decouple the errors from the node generation step.

addRange(/*to*/ diagnostics, /*from*/ result.diagnostics);

if (outputText === undefined) return Debug.fail("Output generation failed");

Expand Down
1 change: 1 addition & 0 deletions src/testRunner/_namespaces/Harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export { Parallel };

export * from "../fourslashRunner";
export * from "../compilerRunner";
export * from "../transpileRunner";
export * from "../runner";

// If running as emitted CJS, don't start executing the tests here; instead start in runner.ts.
Expand Down
9 changes: 9 additions & 0 deletions src/testRunner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
setShardId,
setShards,
TestRunnerKind,
TranspileRunner,
} from "./_namespaces/Harness";
import * as project from "./_namespaces/project";
import * as ts from "./_namespaces/ts";
Expand Down Expand Up @@ -66,6 +67,8 @@ export function createRunner(kind: TestRunnerKind): RunnerBase {
return new FourSlashRunner(FourSlash.FourSlashTestType.Server);
case "project":
return new project.ProjectRunner();
case "transpile":
return new TranspileRunner();
Copy link
Member

Choose a reason for hiding this comment

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

this is a mega good idea

Copy link
Member Author

Choose a reason for hiding this comment

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

When we have time, we should port the bulk of the existing transpile unit tests to this runner, probably. (The ones that don't pass in a custom transformer, at least.)

I also have a backlogged idea to make a runner for tscWatch-type tests, because the incredible boilerplate for those gets me every time I write one; just need to figure out a good way to encode the edits you usually want to apply in sequence into the input compiler-test-type fs description.

}
return ts.Debug.fail(`Unknown runner kind ${kind}`);
}
Expand Down Expand Up @@ -190,6 +193,9 @@ function handleTestConfig() {
case "fourslash-generated":
runners.push(new GeneratedFourslashRunner(FourSlash.FourSlashTestType.Native));
break;
case "transpile":
runners.push(new TranspileRunner());
break;
}
}
}
Expand All @@ -206,6 +212,9 @@ function handleTestConfig() {
runners.push(new FourSlashRunner(FourSlash.FourSlashTestType.Native));
runners.push(new FourSlashRunner(FourSlash.FourSlashTestType.Server));
// runners.push(new GeneratedFourslashRunner());

// transpile
runners.push(new TranspileRunner());
}
if (runUnitTests === undefined) {
runUnitTests = runners.length !== 1; // Don't run unit tests when running only one runner if unit tests were not explicitly asked for
Expand Down
128 changes: 128 additions & 0 deletions src/testRunner/transpileRunner.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import {
Baseline,
Compiler,
getFileBasedTestConfigurations,
IO,
RunnerBase,
TestCaseParser,
TestRunnerKind,
} from "./_namespaces/Harness";
import * as ts from "./_namespaces/ts";
import * as vpath from "./_namespaces/vpath";

export class TranspileRunner extends RunnerBase {
protected basePath = "tests/cases/transpile";
protected testSuiteName: TestRunnerKind = "transpile";

public enumerateTestFiles() {
// see also: `enumerateTestFiles` in tests/webTestServer.ts
return this.enumerateFiles(this.basePath, /\.[cm]?[tj]sx?/i, { recursive: true });
}

public kind() {
return this.testSuiteName;
}

public initializeTests() {
if (this.tests.length === 0) {
this.tests = IO.enumerateTestFiles(this);
}

describe(this.testSuiteName + " tests", () => {
this.tests.forEach(file => {
file = vpath.normalizeSeparators(file);
describe(file, () => {
const tests = TranspileTestCase.getConfigurations(file);
for (const test of tests) {
test.run();
}
});
});
});
}
}

enum TranspileKind {
Module,
Declaration,
}

class TranspileTestCase {
static varyBy = [];

static getConfigurations(file: string): TranspileTestCase[] {
const ext = vpath.extname(file);
const baseName = vpath.basename(file);
const justName = baseName.slice(0, baseName.length - ext.length);
const content = IO.readFile(file)!;
const settings = TestCaseParser.extractCompilerSettings(content);
const settingConfigurations = getFileBasedTestConfigurations(settings, TranspileTestCase.varyBy);
return settingConfigurations?.map(c => {
const desc = Object.entries(c).map(([key, value]) => `${key}=${value}`).join(",");
return new TranspileTestCase(`${justName}(${desc})`, ext, content, { ...settings, ...c });
}) ?? [new TranspileTestCase(justName, ext, content, settings)];
}

private jsOutName;
private dtsOutName;
private units;
constructor(
private justName: string,
private ext: string,
private content: string,
private settings: TestCaseParser.CompilerSettings,
) {
this.jsOutName = justName + this.getJsOutputExtension(`${justName}${ext}`);
this.dtsOutName = justName + ts.getDeclarationEmitExtensionForPath(`${justName}${ext}`);
this.units = TestCaseParser.makeUnitsFromTest(content, `${justName}${ext}`, settings);
}

getJsOutputExtension(name: string) {
return ts.getOutputExtension(name, { jsx: this.settings.jsx === "preserve" ? ts.JsxEmit.Preserve : undefined });
}

runKind(kind: TranspileKind) {
it(`transpile test ${this.justName} has expected ${kind === TranspileKind.Module ? "js" : "declaration"} output`, () => {
let baselineText = "";

// include inputs in output so how the test is parsed and broken down is more obvious
this.units.testUnitData.forEach(unit => {
baselineText += `//// [${unit.name}] ////\r\n`;
baselineText += unit.content;
if (!unit.content.endsWith("\n")) {
baselineText += "\r\n";
}
});

this.units.testUnitData.forEach(unit => {
const opts: ts.CompilerOptions = {};
Compiler.setCompilerOptionsFromHarnessSetting(this.settings, opts);
const result = (kind === TranspileKind.Module ? ts.transpileModule : ts.transpileDeclaration)(unit.content, { compilerOptions: opts, fileName: unit.name, reportDiagnostics: this.settings.reportDiagnostics === "true" });

baselineText += `//// [${ts.changeExtension(unit.name, kind === TranspileKind.Module ? this.getJsOutputExtension(unit.name) : ts.getDeclarationEmitExtensionForPath(unit.name))}] ////\r\n`;
baselineText += result.outputText;
if (!result.outputText.endsWith("\n")) {
baselineText += "\r\n";
}
if (result.diagnostics && result.diagnostics.length) {
baselineText += "\r\n\r\n//// [Diagnostics reported]\r\n";
baselineText += Compiler.getErrorBaseline([{ content: unit.content, unitName: unit.name }], result.diagnostics, !!opts.pretty);
if (!baselineText.endsWith("\n")) {
baselineText += "\r\n";
}
}
});

Baseline.runBaseline(`transpile/${kind === TranspileKind.Module ? this.jsOutName : this.dtsOutName}`, baselineText);
});
}

run() {
if (!this.settings.emitDeclarationOnly) {
this.runKind(TranspileKind.Module);
}
if (this.settings.declaration) {
this.runKind(TranspileKind.Declaration);
}
}
}
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11190,6 +11190,7 @@ declare namespace ts {
};
function preProcessFile(sourceText: string, readImportFiles?: boolean, detectJavaScriptImports?: boolean): PreProcessedFileInfo;
function transpileModule(input: string, transpileOptions: TranspileOptions): TranspileOutput;
function transpileDeclaration(input: string, transpileOptions: TranspileOptions): TranspileOutput;
function transpile(input: string, compilerOptions?: CompilerOptions, fileName?: string, diagnostics?: Diagnostic[], moduleName?: string): string;
interface TranspileOptions {
compilerOptions?: CompilerOptions;
Expand Down
Loading