Skip to content

Commit fdd1de5

Browse files
jakebaileyjakebailey
authored
Mark CompilerOptions as noCopy (#1106)
1 parent 46b5284 commit fdd1de5

File tree

7 files changed

+97
-45
lines changed

7 files changed

+97
-45
lines changed

‎internal/core/compileroptions.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package core
22

33
import (
4+
"reflect"
45
"strings"
56

67
"github.com/microsoft/typescript-go/internal/collections"
@@ -11,6 +12,8 @@ import (
1112
//go:generate go tool mvdan.cc/gofumpt -lang=go1.24 -w compileroptions_stringer_generated.go
1213

1314
type CompilerOptions struct {
15+
_ noCopy
16+
1417
AllowJs Tristate `json:"allowJs,omitzero"`
1518
AllowArbitraryExtensions Tristate `json:"allowArbitraryExtensions,omitzero"`
1619
AllowSyntheticDefaultImports Tristate `json:"allowSyntheticDefaultImports,omitzero"`
@@ -145,6 +148,36 @@ type CompilerOptions struct {
145148
Quiet Tristate `json:"quiet,omitzero"`
146149
}
147150

151+
// noCopy may be embedded into structs which must not be copied
152+
// after the first use.
153+
//
154+
// See https://golang.org/issues/8005#issuecomment-190753527
155+
// for details.
156+
type noCopy struct{}
157+
158+
// Lock is a no-op used by -copylocks checker from `go vet`.
159+
func (*noCopy) Lock() {}
160+
func (*noCopy) Unlock() {}
161+
162+
var optionsType = reflect.TypeFor[CompilerOptions]()
163+
164+
// Clone creates a shallow copy of the CompilerOptions.
165+
func (options *CompilerOptions) Clone() *CompilerOptions {
166+
// TODO: this could be generated code instead of reflection.
167+
target := &CompilerOptions{}
168+
169+
sourceValue := reflect.ValueOf(options).Elem()
170+
targetValue := reflect.ValueOf(target).Elem()
171+
172+
for i := range sourceValue.NumField() {
173+
if optionsType.Field(i).IsExported() {
174+
targetValue.Field(i).Set(sourceValue.Field(i))
175+
}
176+
}
177+
178+
return target
179+
}
180+
148181
func (options *CompilerOptions) GetEmitScriptTarget() ScriptTarget {
149182
if options.Target != ScriptTargetNone {
150183
return options.Target

‎internal/testutil/harnessutil/harnessutil.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,12 @@ func CompileFiles(
8484
currentDirectory string,
8585
symlinks map[string]string,
8686
) *CompilationResult {
87-
var compilerOptions core.CompilerOptions
87+
var compilerOptions *core.CompilerOptions
8888
if tsconfig != nil {
89-
compilerOptions = *tsconfig.ParsedConfig.CompilerOptions
89+
compilerOptions = tsconfig.ParsedConfig.CompilerOptions.Clone()
90+
}
91+
if compilerOptions == nil {
92+
compilerOptions = &core.CompilerOptions{}
9093
}
9194
// Set default options for tests
9295
if compilerOptions.NewLine == core.NewLineKindNone {
@@ -100,10 +103,10 @@ func CompileFiles(
100103

101104
// Parse harness and compiler options from the test configuration
102105
if testConfig != nil {
103-
setOptionsFromTestConfig(t, testConfig, &compilerOptions, &harnessOptions)
106+
setOptionsFromTestConfig(t, testConfig, compilerOptions, &harnessOptions)
104107
}
105108

106-
return CompileFilesEx(t, inputFiles, otherFiles, &harnessOptions, &compilerOptions, currentDirectory, symlinks, tsconfig)
109+
return CompileFilesEx(t, inputFiles, otherFiles, &harnessOptions, compilerOptions, currentDirectory, symlinks, tsconfig)
107110
}
108111

109112
func CompileFilesEx(
@@ -225,9 +228,9 @@ func CompileFilesEx(
225228
result.Symlinks = symlinks
226229
result.Repeat = func(testConfig TestConfiguration) *CompilationResult {
227230
newHarnessOptions := *harnessOptions
228-
newCompilerOptions := *compilerOptions
229-
setOptionsFromTestConfig(t, testConfig, &newCompilerOptions, &newHarnessOptions)
230-
return CompileFilesEx(t, inputFiles, otherFiles, &newHarnessOptions, &newCompilerOptions, currentDirectory, symlinks, tsconfig)
231+
newCompilerOptions := compilerOptions.Clone()
232+
setOptionsFromTestConfig(t, testConfig, newCompilerOptions, &newHarnessOptions)
233+
return CompileFilesEx(t, inputFiles, otherFiles, &newHarnessOptions, newCompilerOptions, currentDirectory, symlinks, tsconfig)
231234
}
232235
return result
233236
}

‎internal/transformers/commonjsmodule_test.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestCommonJSModuleTransformer(t *testing.T) {
2323
output string
2424
other string
2525
jsx bool
26-
options core.CompilerOptions
26+
options *core.CompilerOptions
2727
}{
2828
// ImportDeclaration
2929
{
@@ -100,7 +100,7 @@ var __importStar = (this && this.__importStar) || function (mod) {
100100
};
101101
Object.defineProperty(exports, "__esModule", { value: true });
102102
const a = __importStar(require("other"));`,
103-
options: core.CompilerOptions{ESModuleInterop: core.TSTrue},
103+
options: &core.CompilerOptions{ESModuleInterop: core.TSTrue},
104104
},
105105
{
106106
title: "ImportDeclaration#8",
@@ -109,7 +109,7 @@ const a = __importStar(require("other"));`,
109109
Object.defineProperty(exports, "__esModule", { value: true });
110110
const tslib_1 = require("tslib");
111111
const a = tslib_1.__importStar(require("other"));`,
112-
options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
112+
options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
113113
},
114114

115115
// ImportEqualsDeclaration
@@ -175,7 +175,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
175175
exports.a = void 0;
176176
const tslib_1 = require("tslib");
177177
exports.a = tslib_1.__importStar(require("other"));`,
178-
options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
178+
options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
179179
},
180180
{
181181
title: "ExportDeclaration#5",
@@ -184,7 +184,7 @@ exports.a = tslib_1.__importStar(require("other"));`,
184184
Object.defineProperty(exports, "__esModule", { value: true });
185185
const tslib_1 = require("tslib");
186186
tslib_1.__exportStar(require("other"), exports);`,
187-
options: core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
187+
options: &core.CompilerOptions{ESModuleInterop: core.TSTrue, ImportHelpers: core.TSTrue},
188188
},
189189

190190
// ExportAssignment
@@ -855,7 +855,7 @@ import("./other.ts");`,
855855
output: `"use strict";
856856
Object.defineProperty(exports, "__esModule", { value: true });
857857
Promise.resolve().then(() => require("./other.js"));`,
858-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
858+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
859859
},
860860
{
861861
title: "CallExpression#4",
@@ -872,7 +872,7 @@ var __rewriteRelativeImportExtension = (this && this.__rewriteRelativeImportExte
872872
};
873873
Object.defineProperty(exports, "__esModule", { value: true });
874874
Promise.resolve(` + "`" + `${__rewriteRelativeImportExtension(x)}` + "`" + `).then(s => require(s));`,
875-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
875+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
876876
},
877877
{
878878
title: "CallExpression#5",
@@ -889,7 +889,7 @@ var __rewriteRelativeImportExtension = (this && this.__rewriteRelativeImportExte
889889
};
890890
Object.defineProperty(exports, "__esModule", { value: true });
891891
Promise.resolve(` + "`" + `${__rewriteRelativeImportExtension(x, true)}` + "`" + `).then(s => require(s));`,
892-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
892+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
893893
},
894894
{
895895
title: "CallExpression#6",
@@ -899,7 +899,7 @@ import(x);`,
899899
Object.defineProperty(exports, "__esModule", { value: true });
900900
const tslib_1 = require("tslib");
901901
Promise.resolve(` + "`" + `${tslib_1.__rewriteRelativeImportExtension(x)}` + "`" + `).then(s => require(s));`,
902-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
902+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
903903
},
904904
{
905905
title: "CallExpression#7",
@@ -1016,6 +1016,10 @@ exports.a = a;`,
10161016
t.Parallel()
10171017

10181018
compilerOptions := rec.options
1019+
if compilerOptions == nil {
1020+
compilerOptions = &core.CompilerOptions{}
1021+
}
1022+
10191023
compilerOptions.Module = core.ModuleKindCommonJS
10201024
sourceFileAffecting := compilerOptions.SourceFileAffecting()
10211025

@@ -1031,10 +1035,10 @@ exports.a = a;`,
10311035
}
10321036

10331037
emitContext := printer.NewEmitContext()
1034-
resolver := binder.NewReferenceResolver(&compilerOptions, binder.ReferenceResolverHooks{})
1038+
resolver := binder.NewReferenceResolver(compilerOptions, binder.ReferenceResolverHooks{})
10351039

1036-
file = NewRuntimeSyntaxTransformer(emitContext, &compilerOptions, resolver).TransformSourceFile(file)
1037-
file = NewCommonJSModuleTransformer(emitContext, &compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file)
1040+
file = NewRuntimeSyntaxTransformer(emitContext, compilerOptions, resolver).TransformSourceFile(file)
1041+
file = NewCommonJSModuleTransformer(emitContext, compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file)
10381042
emittestutil.CheckEmit(t, emitContext, file, rec.output)
10391043
})
10401044
}

‎internal/transformers/esmodule_test.go

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func TestESModuleTransformer(t *testing.T) {
1919
output string
2020
other string
2121
jsx bool
22-
options core.CompilerOptions
22+
options *core.CompilerOptions
2323
}{
2424
// ImportDeclaration
2525
{
@@ -31,19 +31,19 @@ func TestESModuleTransformer(t *testing.T) {
3131
title: "ImportDeclaration#2",
3232
input: `import "./other.ts"`,
3333
output: `import "./other.js";`,
34-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
34+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
3535
},
3636
{
3737
title: "ImportDeclaration#3",
3838
input: `import "./other.tsx"`,
3939
output: `import "./other.js";`,
40-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
40+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
4141
},
4242
{
4343
title: "ImportDeclaration#4",
4444
input: `import "./other.tsx"`,
4545
output: `import "./other.jsx";`,
46-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
46+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
4747
},
4848

4949
// ImportEqualsDeclaration
@@ -58,31 +58,31 @@ func TestESModuleTransformer(t *testing.T) {
5858
output: `import { createRequire as _createRequire } from "module";
5959
const __require = _createRequire(import.meta.url);
6060
const x = __require("other");`,
61-
options: core.CompilerOptions{Module: core.ModuleKindNode16},
61+
options: &core.CompilerOptions{Module: core.ModuleKindNode16},
6262
},
6363
{
6464
title: "ImportEqualsDeclaration#3",
6565
input: `import x = require("./other.ts")`,
6666
output: `import { createRequire as _createRequire } from "module";
6767
const __require = _createRequire(import.meta.url);
6868
const x = __require("./other.js");`,
69-
options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue},
69+
options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue},
7070
},
7171
{
7272
title: "ImportEqualsDeclaration#4",
7373
input: `import x = require("./other.tsx")`,
7474
output: `import { createRequire as _createRequire } from "module";
7575
const __require = _createRequire(import.meta.url);
7676
const x = __require("./other.js");`,
77-
options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue},
77+
options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue},
7878
},
7979
{
8080
title: "ImportEqualsDeclaration#5",
8181
input: `import x = require("./other.tsx")`,
8282
output: `import { createRequire as _createRequire } from "module";
8383
const __require = _createRequire(import.meta.url);
8484
const x = __require("./other.jsx");`,
85-
options: core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
85+
options: &core.CompilerOptions{Module: core.ModuleKindNode16, RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
8686
},
8787
{
8888
title: "ImportEqualsDeclaration#6",
@@ -91,7 +91,7 @@ const x = __require("./other.jsx");`,
9191
const __require = _createRequire(import.meta.url);
9292
const x = __require("other");
9393
export { x };`,
94-
options: core.CompilerOptions{Module: core.ModuleKindNode16},
94+
options: &core.CompilerOptions{Module: core.ModuleKindNode16},
9595
},
9696

9797
// ExportAssignment
@@ -104,7 +104,7 @@ export { x };`,
104104
title: "ExportAssignment#2",
105105
input: `export = x`,
106106
output: `module.exports = x;`,
107-
options: core.CompilerOptions{Module: core.ModuleKindPreserve},
107+
options: &core.CompilerOptions{Module: core.ModuleKindPreserve},
108108
},
109109

110110
// ExportDeclaration
@@ -117,13 +117,13 @@ export { x };`,
117117
title: "ExportDeclaration#2",
118118
input: `export * from "./other.ts";`,
119119
output: `export * from "./other.js";`,
120-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
120+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
121121
},
122122
{
123123
title: "ExportDeclaration#3",
124124
input: `export * as x from "other";`,
125125
output: `export * as x from "other";`,
126-
options: core.CompilerOptions{Module: core.ModuleKindESNext},
126+
options: &core.CompilerOptions{Module: core.ModuleKindESNext},
127127
},
128128
{
129129
title: "ExportDeclaration#4",
@@ -160,7 +160,7 @@ export default default_1;`,
160160
import("./other.ts");`,
161161
output: `export {};
162162
import("./other.js");`,
163-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
163+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
164164
},
165165
{
166166
title: "CallExpression#4",
@@ -176,7 +176,7 @@ import(x);`,
176176
};
177177
export {};
178178
import(__rewriteRelativeImportExtension(x));`,
179-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
179+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue},
180180
},
181181
{
182182
title: "CallExpression#5",
@@ -192,7 +192,7 @@ import(x);`,
192192
};
193193
export {};
194194
import(__rewriteRelativeImportExtension(x, true));`,
195-
options: core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
195+
options: &core.CompilerOptions{RewriteRelativeImportExtensions: core.TSTrue, Jsx: core.JsxEmitPreserve},
196196
},
197197
{
198198
title: "CallExpression#6",
@@ -201,7 +201,7 @@ import(x);`,
201201
output: `import { __rewriteRelativeImportExtension } from "tslib";
202202
export {};
203203
import(__rewriteRelativeImportExtension(x));`,
204-
options: core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
204+
options: &core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
205205
},
206206
{
207207
title: "CallExpression#7",
@@ -212,14 +212,18 @@ var __rewriteRelativeImportExtension;`,
212212
export {};
213213
import(__rewriteRelativeImportExtension_1(x));
214214
var __rewriteRelativeImportExtension;`,
215-
options: core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
215+
options: &core.CompilerOptions{Module: core.ModuleKindESNext, RewriteRelativeImportExtensions: core.TSTrue, ImportHelpers: core.TSTrue},
216216
},
217217
}
218218
for _, rec := range data {
219219
t.Run(rec.title, func(t *testing.T) {
220220
t.Parallel()
221221

222222
compilerOptions := rec.options
223+
if compilerOptions == nil {
224+
compilerOptions = &core.CompilerOptions{}
225+
}
226+
223227
sourceFileAffecting := compilerOptions.SourceFileAffecting()
224228
file := parsetestutil.ParseTypeScript(rec.input, rec.jsx)
225229
parsetestutil.CheckDiagnostics(t, file)
@@ -233,10 +237,10 @@ var __rewriteRelativeImportExtension;`,
233237
}
234238

235239
emitContext := printer.NewEmitContext()
236-
resolver := binder.NewReferenceResolver(&compilerOptions, binder.ReferenceResolverHooks{})
240+
resolver := binder.NewReferenceResolver(compilerOptions, binder.ReferenceResolverHooks{})
237241

238-
file = NewRuntimeSyntaxTransformer(emitContext, &compilerOptions, resolver).TransformSourceFile(file)
239-
file = NewESModuleTransformer(emitContext, &compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file)
242+
file = NewRuntimeSyntaxTransformer(emitContext, compilerOptions, resolver).TransformSourceFile(file)
243+
file = NewESModuleTransformer(emitContext, compilerOptions, resolver, fakeGetEmitModuleFormatOfFile).TransformSourceFile(file)
240244
emittestutil.CheckEmit(t, emitContext, file, rec.output)
241245
})
242246
}

0 commit comments

Comments
 (0)