From 373b1118ce0ab3092e3dd2b53e530508b2434ada Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 9 Jun 2025 11:36:50 -0700 Subject: [PATCH 1/3] Mark CompilerOptions as non-copyable --- internal/core/compileroptions.go | 33 ++++++++++++++++ internal/testutil/harnessutil/harnessutil.go | 14 +++---- internal/transformers/commonjsmodule_test.go | 24 ++++++------ internal/transformers/esmodule_test.go | 40 ++++++++++---------- internal/tsoptions/commandlineparser_test.go | 12 +++--- 5 files changed, 78 insertions(+), 45 deletions(-) diff --git a/internal/core/compileroptions.go b/internal/core/compileroptions.go index 76c9d27fd2..bd8b3012d5 100644 --- a/internal/core/compileroptions.go +++ b/internal/core/compileroptions.go @@ -1,6 +1,7 @@ package core import ( + "reflect" "strings" "github.com/microsoft/typescript-go/internal/collections" @@ -11,6 +12,8 @@ import ( //go:generate go tool mvdan.cc/gofumpt -lang=go1.24 -w compileroptions_stringer_generated.go type CompilerOptions struct { + _ noCopy + AllowJs Tristate `json:"allowJs,omitzero"` AllowArbitraryExtensions Tristate `json:"allowArbitraryExtensions,omitzero"` AllowSyntheticDefaultImports Tristate `json:"allowSyntheticDefaultImports,omitzero"` @@ -145,6 +148,36 @@ type CompilerOptions struct { Quiet Tristate `json:"quiet,omitzero"` } +// noCopy may be embedded into structs which must not be copied +// after the first use. +// +// See https://golang.org/issues/8005#issuecomment-190753527 +// for details. +type noCopy struct{} + +// Lock is a no-op used by -copylocks checker from `go vet`. +func (*noCopy) Lock() {} +func (*noCopy) Unlock() {} + +var optionsType = reflect.TypeFor[CompilerOptions]() + +// Clone creates a shallow copy of the CompilerOptions. +func (options *CompilerOptions) Clone() *CompilerOptions { + // TODO: this could be generated code instead of reflection. + target := &CompilerOptions{} + + sourceValue := reflect.ValueOf(options).Elem() + targetValue := reflect.ValueOf(target).Elem() + + for i := 0; i < sourceValue.NumField(); i++ { + if optionsType.Field(i).IsExported() { + targetValue.Field(i).Set(sourceValue.Field(i)) + } + } + + return target +} + func (options *CompilerOptions) GetEmitScriptTarget() ScriptTarget { if options.Target != ScriptTargetNone { return options.Target diff --git a/internal/testutil/harnessutil/harnessutil.go b/internal/testutil/harnessutil/harnessutil.go index 7cc51ed90c..fbe813f3f5 100644 --- a/internal/testutil/harnessutil/harnessutil.go +++ b/internal/testutil/harnessutil/harnessutil.go @@ -84,9 +84,9 @@ func CompileFiles( currentDirectory string, symlinks map[string]string, ) *CompilationResult { - var compilerOptions core.CompilerOptions + var compilerOptions *core.CompilerOptions if tsconfig != nil { - compilerOptions = *tsconfig.ParsedConfig.CompilerOptions + compilerOptions = tsconfig.ParsedConfig.CompilerOptions.Clone() } // Set default options for tests if compilerOptions.NewLine == core.NewLineKindNone { @@ -100,10 +100,10 @@ func CompileFiles( // Parse harness and compiler options from the test configuration if testConfig != nil { - setOptionsFromTestConfig(t, testConfig, &compilerOptions, &harnessOptions) + setOptionsFromTestConfig(t, testConfig, compilerOptions, &harnessOptions) } - return CompileFilesEx(t, inputFiles, otherFiles, &harnessOptions, &compilerOptions, currentDirectory, symlinks, tsconfig) + return CompileFilesEx(t, inputFiles, otherFiles, &harnessOptions, compilerOptions, currentDirectory, symlinks, tsconfig) } func CompileFilesEx( @@ -225,9 +225,9 @@ func CompileFilesEx( result.Symlinks = symlinks result.Repeat = func(testConfig TestConfiguration) *CompilationResult { newHarnessOptions := *harnessOptions - newCompilerOptions := *compilerOptions - setOptionsFromTestConfig(t, testConfig, &newCompilerOptions, &newHarnessOptions) - return CompileFilesEx(t, inputFiles, otherFiles, &newHarnessOptions, &newCompilerOptions, currentDirectory, symlinks, tsconfig) + newCompilerOptions := compilerOptions.Clone() + setOptionsFromTestConfig(t, testConfig, newCompilerOptions, &newHarnessOptions) + return CompileFilesEx(t, inputFiles, otherFiles, &newHarnessOptions, newCompilerOptions, currentDirectory, symlinks, tsconfig) } return result } diff --git a/internal/transformers/commonjsmodule_test.go b/internal/transformers/commonjsmodule_test.go index 045876f1c6..92f48d3d15 100644 --- a/internal/transformers/commonjsmodule_test.go +++ b/internal/transformers/commonjsmodule_test.go @@ -23,7 +23,7 @@ func TestCommonJSModuleTransformer(t *testing.T) { output string other string jsx bool - options core.CompilerOptions + options *core.CompilerOptions }{ // ImportDeclaration { @@ -100,7 +100,7 @@ var __importStar = (this && this.__importStar) || function (mod) { }; Object.defineProperty(exports, "__esModule", { value: true }); const a = __importStar(require("other"));`, - options: core.CompilerOptions{ESModuleInterop: core.TSTrue}, + options: &core.CompilerOptions{ESModuleInterop: core.TSTrue}, }, { title: "ImportDeclaration#8", @@ -109,7 +109,7 @@ const a = __importStar(require("other"));`, Object.defineProperty(exports, "__esModule", { value: true }); const tslib_1 = require("tslib"); const a = tslib_1.__importStar(require("other"));`, - options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, }, // ImportEqualsDeclaration @@ -175,7 +175,7 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.a = void 0; const tslib_1 = require("tslib"); exports.a = tslib_1.__importStar(require("other"));`, - options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, }, { title: "ExportDeclaration#5", @@ -184,7 +184,7 @@ exports.a = tslib_1.__importStar(require("other"));`, Object.defineProperty(exports, "__esModule", { value: true }); const tslib_1 = require("tslib"); tslib_1.__exportStar(require("other"), exports);`, - options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue}, }, // ExportAssignment @@ -855,7 +855,7 @@ import("./other.ts");`, output: `"use strict"; Object.defineProperty(exports, "__esModule", { value: true }); Promise.resolve().then(() => require("./other.js"));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "CallExpression#4", @@ -872,7 +872,7 @@ var __rewriteRelativeImportExtension = (this && this.__rewriteRelativeImportExte }; Object.defineProperty(exports, "__esModule", { value: true }); Promise.resolve(` + "`" + `${__rewriteRelativeImportExtension(x)}` + "`" + `).then(s => require(s));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "CallExpression#5", @@ -889,7 +889,7 @@ var __rewriteRelativeImportExtension = (this && this.__rewriteRelativeImportExte }; Object.defineProperty(exports, "__esModule", { value: true }); Promise.resolve(` + "`" + `${__rewriteRelativeImportExtension(x, true)}` + "`" + `).then(s => require(s));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, }, { title: "CallExpression#6", @@ -899,7 +899,7 @@ import(x);`, Object.defineProperty(exports, "__esModule", { value: true }); const tslib_1 = require("tslib"); Promise.resolve(` + "`" + `${tslib_1.__rewriteRelativeImportExtension(x)}` + "`" + `).then(s => require(s));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, }, { title: "CallExpression#7", @@ -1031,10 +1031,10 @@ exports.a = a;`, } emitContext := printer.NewEmitContext() - resolver := binder.NewReferenceResolver(&compilerOptions, binder.ReferenceResolverHooks{}) + resolver := binder.NewReferenceResolver(compilerOptions, binder.ReferenceResolverHooks{}) - file = NewRuntimeSyntaxTransformer(emitContext, &compilerOptions, resolver).TransformSourceFile(file) - file = NewCommonJSModuleTransformer(emitContext, &compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file) + file = NewRuntimeSyntaxTransformer(emitContext, compilerOptions, resolver).TransformSourceFile(file) + file = NewCommonJSModuleTransformer(emitContext, compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file) emittestutil.CheckEmit(t, emitContext, file, rec.output) }) } diff --git a/internal/transformers/esmodule_test.go b/internal/transformers/esmodule_test.go index 2e71653bc5..5400db6696 100644 --- a/internal/transformers/esmodule_test.go +++ b/internal/transformers/esmodule_test.go @@ -19,7 +19,7 @@ func TestESModuleTransformer(t *testing.T) { output string other string jsx bool - options core.CompilerOptions + options *core.CompilerOptions }{ // ImportDeclaration { @@ -31,19 +31,19 @@ func TestESModuleTransformer(t *testing.T) { title: "ImportDeclaration#2", input: `import "./other.ts"`, output: `import "./other.js";`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "ImportDeclaration#3", input: `import "./other.tsx"`, output: `import "./other.js";`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "ImportDeclaration#4", input: `import "./other.tsx"`, output: `import "./other.jsx";`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, }, // ImportEqualsDeclaration @@ -58,7 +58,7 @@ func TestESModuleTransformer(t *testing.T) { output: `import { createRequire as _createRequire } from "module"; const __require = _createRequire(import.meta.url); const x = __require("other");`, - options: core.CompilerOptions{Module: core.ModuleKindNode16}, + options: &core.CompilerOptions{Module: core.ModuleKindNode16}, }, { title: "ImportEqualsDeclaration#3", @@ -66,7 +66,7 @@ const x = __require("other");`, output: `import { createRequire as _createRequire } from "module"; const __require = _createRequire(import.meta.url); const x = __require("./other.js");`, - options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "ImportEqualsDeclaration#4", @@ -74,7 +74,7 @@ const x = __require("./other.js");`, output: `import { createRequire as _createRequire } from "module"; const __require = _createRequire(import.meta.url); const x = __require("./other.js");`, - options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "ImportEqualsDeclaration#5", @@ -82,7 +82,7 @@ const x = __require("./other.js");`, output: `import { createRequire as _createRequire } from "module"; const __require = _createRequire(import.meta.url); const x = __require("./other.jsx");`, - options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, + options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, }, { title: "ImportEqualsDeclaration#6", @@ -91,7 +91,7 @@ const x = __require("./other.jsx");`, const __require = _createRequire(import.meta.url); const x = __require("other"); export { x };`, - options: core.CompilerOptions{Module: core.ModuleKindNode16}, + options: &core.CompilerOptions{Module: core.ModuleKindNode16}, }, // ExportAssignment @@ -104,7 +104,7 @@ export { x };`, title: "ExportAssignment#2", input: `export = x`, output: `module.exports = x;`, - options: core.CompilerOptions{Module: core.ModuleKindPreserve}, + options: &core.CompilerOptions{Module: core.ModuleKindPreserve}, }, // ExportDeclaration @@ -117,13 +117,13 @@ export { x };`, title: "ExportDeclaration#2", input: `export * from "./other.ts";`, output: `export * from "./other.js";`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "ExportDeclaration#3", input: `export * as x from "other";`, output: `export * as x from "other";`, - options: core.CompilerOptions{Module: core.ModuleKindESNext}, + options: &core.CompilerOptions{Module: core.ModuleKindESNext}, }, { title: "ExportDeclaration#4", @@ -160,7 +160,7 @@ export default default_1;`, import("./other.ts");`, output: `export {}; import("./other.js");`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "CallExpression#4", @@ -176,7 +176,7 @@ import(x);`, }; export {}; import(__rewriteRelativeImportExtension(x));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue}, }, { title: "CallExpression#5", @@ -192,7 +192,7 @@ import(x);`, }; export {}; import(__rewriteRelativeImportExtension(x, true));`, - options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, + options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve}, }, { title: "CallExpression#6", @@ -201,7 +201,7 @@ import(x);`, output: `import { __rewriteRelativeImportExtension } from "tslib"; export {}; import(__rewriteRelativeImportExtension(x));`, - options: core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, }, { title: "CallExpression#7", @@ -212,7 +212,7 @@ var __rewriteRelativeImportExtension;`, export {}; import(__rewriteRelativeImportExtension_1(x)); var __rewriteRelativeImportExtension;`, - options: core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, + options: &core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue}, }, } for _, rec := range data { @@ -233,10 +233,10 @@ var __rewriteRelativeImportExtension;`, } emitContext := printer.NewEmitContext() - resolver := binder.NewReferenceResolver(&compilerOptions, binder.ReferenceResolverHooks{}) + resolver := binder.NewReferenceResolver(compilerOptions, binder.ReferenceResolverHooks{}) - file = NewRuntimeSyntaxTransformer(emitContext, &compilerOptions, resolver).TransformSourceFile(file) - file = NewESModuleTransformer(emitContext, &compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file) + file = NewRuntimeSyntaxTransformer(emitContext, compilerOptions, resolver).TransformSourceFile(file) + file = NewESModuleTransformer(emitContext, compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file) emittestutil.CheckEmit(t, emitContext, file, rec.output) }) } diff --git a/internal/tsoptions/commandlineparser_test.go b/internal/tsoptions/commandlineparser_test.go index 1a6df56803..019e890768 100644 --- a/internal/tsoptions/commandlineparser_test.go +++ b/internal/tsoptions/commandlineparser_test.go @@ -170,8 +170,8 @@ func (f commandLineSubScenario) assertParseResult(t *testing.T) { assert.Equal(t, tsBaseline.fileNames, newBaselineFileNames) o, _ := json.Marshal(parsed.Options) - newParsedCompilerOptions := core.CompilerOptions{} - e := json.Unmarshal(o, &newParsedCompilerOptions) + newParsedCompilerOptions := &core.CompilerOptions{} + e := json.Unmarshal(o, newParsedCompilerOptions) assert.NilError(t, e) assert.DeepEqual(t, tsBaseline.options, newParsedCompilerOptions) @@ -215,8 +215,8 @@ func parseExistingCompilerBaseline(t *testing.T, baseline string) *TestCommandLi } return &TestCommandLineParser{ - options: *baselineCompilerOptions, - watchoptions: *baselineWatchOptions, + options: baselineCompilerOptions, + watchoptions: baselineWatchOptions, fileNames: fileNames, errors: errors, } @@ -300,8 +300,8 @@ type verifyNull struct { } type TestCommandLineParser struct { - options core.CompilerOptions - watchoptions core.WatchOptions + options *core.CompilerOptions + watchoptions *core.WatchOptions fileNames, errors string } From 4fd8815ab2b71187dca1b98cc2899115d5f631a2 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 9 Jun 2025 11:53:06 -0700 Subject: [PATCH 2/3] Fixups --- internal/testutil/harnessutil/harnessutil.go | 3 +++ internal/transformers/commonjsmodule_test.go | 4 ++++ internal/transformers/esmodule_test.go | 4 ++++ internal/tsoptions/decls_test.go | 4 ++++ internal/tsoptions/parsinghelpers_test.go | 4 ++++ 5 files changed, 19 insertions(+) diff --git a/internal/testutil/harnessutil/harnessutil.go b/internal/testutil/harnessutil/harnessutil.go index fbe813f3f5..3d66dfb0de 100644 --- a/internal/testutil/harnessutil/harnessutil.go +++ b/internal/testutil/harnessutil/harnessutil.go @@ -88,6 +88,9 @@ func CompileFiles( if tsconfig != nil { compilerOptions = tsconfig.ParsedConfig.CompilerOptions.Clone() } + if compilerOptions == nil { + compilerOptions = &core.CompilerOptions{} + } // Set default options for tests if compilerOptions.NewLine == core.NewLineKindNone { compilerOptions.NewLine = core.NewLineKindCRLF diff --git a/internal/transformers/commonjsmodule_test.go b/internal/transformers/commonjsmodule_test.go index 92f48d3d15..1b84dbfc8e 100644 --- a/internal/transformers/commonjsmodule_test.go +++ b/internal/transformers/commonjsmodule_test.go @@ -1016,6 +1016,10 @@ exports.a = a;`, t.Parallel() compilerOptions := rec.options + if compilerOptions == nil { + compilerOptions = &core.CompilerOptions{} + } + compilerOptions.Module = core.ModuleKindCommonJS sourceFileAffecting := compilerOptions.SourceFileAffecting() diff --git a/internal/transformers/esmodule_test.go b/internal/transformers/esmodule_test.go index 5400db6696..94e592978d 100644 --- a/internal/transformers/esmodule_test.go +++ b/internal/transformers/esmodule_test.go @@ -220,6 +220,10 @@ var __rewriteRelativeImportExtension;`, t.Parallel() compilerOptions := rec.options + if compilerOptions == nil { + compilerOptions = &core.CompilerOptions{} + } + sourceFileAffecting := compilerOptions.SourceFileAffecting() file := parsetestutil.ParseTypeScript(rec.input, rec.jsx) parsetestutil.CheckDiagnostics(t, file) diff --git a/internal/tsoptions/decls_test.go b/internal/tsoptions/decls_test.go index 5cf574f3bc..38ef65a8d3 100644 --- a/internal/tsoptions/decls_test.go +++ b/internal/tsoptions/decls_test.go @@ -37,6 +37,10 @@ func TestCompilerOptionsDeclaration(t *testing.T) { compilerOptionsType := reflect.TypeFor[core.CompilerOptions]() for i := range compilerOptionsType.NumField() { field := compilerOptionsType.Field(i) + if !field.IsExported() { + continue + } + lowerName := strings.ToLower(field.Name) decl := decls[lowerName] diff --git a/internal/tsoptions/parsinghelpers_test.go b/internal/tsoptions/parsinghelpers_test.go index 6a99e35b50..555a53c745 100644 --- a/internal/tsoptions/parsinghelpers_test.go +++ b/internal/tsoptions/parsinghelpers_test.go @@ -12,6 +12,10 @@ func TestParseCompilerOptionNoMissingFields(t *testing.T) { t.Parallel() var missingKeys []string for _, field := range reflect.VisibleFields(reflect.TypeFor[core.CompilerOptions]()) { + if !field.IsExported() { + continue + } + keyName := field.Name // use the JSON key from the tag, if present // e.g. `json:"dog[,anythingelse]"` --> dog From 73314119d6151e53ea3618ffb1227e5f879bde40 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 9 Jun 2025 12:30:14 -0700 Subject: [PATCH 3/3] Fix lint --- internal/core/compileroptions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/compileroptions.go b/internal/core/compileroptions.go index bd8b3012d5..36c3d665d6 100644 --- a/internal/core/compileroptions.go +++ b/internal/core/compileroptions.go @@ -169,7 +169,7 @@ func (options *CompilerOptions) Clone() *CompilerOptions { sourceValue := reflect.ValueOf(options).Elem() targetValue := reflect.ValueOf(target).Elem() - for i := 0; i < sourceValue.NumField(); i++ { + for i := range sourceValue.NumField() { if optionsType.Field(i).IsExported() { targetValue.Field(i).Set(sourceValue.Field(i)) }