From e9a5496648b020d9e7f7f7fff59accbc1557ad53 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:22:31 -0700 Subject: [PATCH 1/3] Construct nodes instead of retagging their kinds A node's Kind identifies the concrete allocation that unsafe accessors reinterpret it as. The declarations transformer cloned a JS import declaration and overwrote its Kind; that only works while the two kinds happen to share one layout. Some tests similarly built bare Node headers or retagged one node as another kind. Build the import declaration through the factory instead, and create real nodes in the tests, so that nothing depends on retagging. --- tsc/internal/ast/nodeaccessors_test.go | 10 +- tsc/internal/scanner/scanner_test.go | 30 +- .../transformers/declarations/transform.go | 9 +- .../reference/conformance/importTag5.js | 46 ++- .../reference/conformance/importTag5.js.map | 11 + .../conformance/importTag5.sourcemap.txt | 389 ++++++++++++++++++ .../reference/conformance/importTag5.symbols | 34 ++ .../reference/conformance/importTag5.types | 33 ++ .../cases/conformance/jsdoc/importTag5.ts | 24 ++ 9 files changed, 564 insertions(+), 22 deletions(-) create mode 100644 tsc/testdata/baselines/reference/conformance/importTag5.js.map create mode 100644 tsc/testdata/baselines/reference/conformance/importTag5.sourcemap.txt diff --git a/tsc/internal/ast/nodeaccessors_test.go b/tsc/internal/ast/nodeaccessors_test.go index c18342076dbdb..f842ed1fc2f89 100644 --- a/tsc/internal/ast/nodeaccessors_test.go +++ b/tsc/internal/ast/nodeaccessors_test.go @@ -81,11 +81,11 @@ func TestNodeAccessorsSharedKinds(t *testing.T) { factory := ast.NewNodeFactory(ast.NodeFactoryHooks{}) name := factory.NewIdentifier("T") modifiers := factory.NewModifierList([]*ast.Node{factory.NewToken(ast.KindExportKeyword)}) - node := factory.NewTypeAliasDeclaration(modifiers, name, nil, nil) - data := node.AsTypeAliasDeclaration() - - for _, kind := range []ast.Kind{ast.KindTypeAliasDeclaration, ast.KindJSTypeAliasDeclaration} { - node.Kind = kind + for _, node := range []*ast.Node{ + factory.NewTypeAliasDeclaration(modifiers, name, nil, nil), + factory.NewJSTypeAliasDeclaration(modifiers, name, nil, nil), + } { + data := node.AsTypeAliasDeclaration() assert.Equal(t, node.Name(), name) assert.Equal(t, node.Modifiers(), modifiers) assert.Equal(t, node.DeclarationData(), &data.DeclarationBase) diff --git a/tsc/internal/scanner/scanner_test.go b/tsc/internal/scanner/scanner_test.go index 8f970f2af8467..701c6a01377ef 100644 --- a/tsc/internal/scanner/scanner_test.go +++ b/tsc/internal/scanner/scanner_test.go @@ -49,20 +49,26 @@ func TestNormalizeJSDocTypeSourceText(t *testing.T) { func TestIsJSDocTypeExpressionOrChild(t *testing.T) { t.Parallel() - jsDocType := &ast.Node{Kind: ast.KindTypeReference, Flags: ast.NodeFlagsJSDoc} - jsDocTypeChild := &ast.Node{Kind: ast.KindIdentifier, Flags: ast.NodeFlagsJSDoc, Parent: jsDocType} - reparsedType := &ast.Node{Kind: ast.KindTypeLiteral, Flags: ast.NodeFlagsReparsed} - reparsedTypeChild := &ast.Node{Kind: ast.KindIdentifier, Flags: ast.NodeFlagsReparsed, Parent: reparsedType} - ordinaryType := &ast.Node{Kind: ast.KindTypeReference} - jsDocTag := &ast.Node{Kind: ast.KindJSDocParameterTag, Flags: ast.NodeFlagsJSDoc} - jsDocTagChild := &ast.Node{Kind: ast.KindIdentifier, Flags: ast.NodeFlagsJSDoc, Parent: jsDocTag} + factory := ast.NewNodeFactory(ast.NodeFactoryHooks{}) + node := func(n *ast.Node, flags ast.NodeFlags, parent *ast.Node) *ast.Node { + n.Flags = flags + n.Parent = parent + return n + } + jsDocType := node(factory.NewTypeReferenceNode(factory.NewIdentifier("T"), nil), ast.NodeFlagsJSDoc, nil) + jsDocTypeChild := node(factory.NewIdentifier("T"), ast.NodeFlagsJSDoc, jsDocType) + reparsedType := node(factory.NewTypeLiteralNode(nil), ast.NodeFlagsReparsed, nil) + reparsedTypeChild := node(factory.NewIdentifier("T"), ast.NodeFlagsReparsed, reparsedType) + ordinaryType := factory.NewTypeReferenceNode(factory.NewIdentifier("T"), nil) + jsDocTag := node(factory.NewJSDocParameterOrPropertyTag(ast.KindJSDocParameterTag, factory.NewIdentifier("param"), factory.NewIdentifier("p"), false, nil, false, nil), ast.NodeFlagsJSDoc, nil) + jsDocTagChild := node(factory.NewIdentifier("T"), ast.NodeFlagsJSDoc, jsDocTag) tests := []struct { name string node *ast.Node expected bool }{ - {name: "type expression", node: &ast.Node{Kind: ast.KindJSDocTypeExpression}, expected: true}, + {name: "type expression", node: factory.NewJSDocTypeExpression(ordinaryType), expected: true}, {name: "JSDoc type", node: jsDocType, expected: true}, {name: "JSDoc type child", node: jsDocTypeChild, expected: true}, {name: "reparsed type", node: reparsedType, expected: true}, @@ -83,11 +89,9 @@ func TestGetTextOfNodeFromJSDocTypePreservesAsteriskType(t *testing.T) { t.Parallel() sourceText := strings.Join([]string{"", " * *"}, core.NewLineKindLF.GetNewLineCharacter()) - node := &ast.Node{ - Kind: ast.KindJSDocAllType, - Flags: ast.NodeFlagsJSDoc, - Loc: core.NewTextRange(0, len(sourceText)), - } + node := ast.NewNodeFactory(ast.NodeFactoryHooks{}).NewJSDocAllType() + node.Flags = ast.NodeFlagsJSDoc + node.Loc = core.NewTextRange(0, len(sourceText)) assert.Equal(t, GetTextOfNodeFromSourceText(sourceText, node, false /*includeTrivia*/), "*") } diff --git a/tsc/internal/transformers/declarations/transform.go b/tsc/internal/transformers/declarations/transform.go index 9333392d53fb8..1eb9d3cf63838 100644 --- a/tsc/internal/transformers/declarations/transform.go +++ b/tsc/internal/transformers/declarations/transform.go @@ -1738,9 +1738,12 @@ func (tx *DeclarationTransformer) transformTopLevelDeclaration(input *ast.Node) if input.Kind == ast.KindImportDeclaration || input.Kind == ast.KindJSImportDeclaration { res := tx.transformImportDeclaration(input.AsImportDeclaration()) if res != nil && res.Kind != ast.KindImportDeclaration { - res := res.Clone(tx.Factory()) - res.Kind = ast.KindImportDeclaration - return res + decl := res.AsImportDeclaration() + replacement := tx.Factory().NewImportDeclaration(decl.Modifiers(), decl.ImportClause, decl.ModuleSpecifier, decl.Attributes) + replacement.Flags = res.Flags + replacement.Loc = res.Loc + tx.EmitContext().SetOriginal(replacement, res) + return replacement } return res } diff --git a/tsc/testdata/baselines/reference/conformance/importTag5.js b/tsc/testdata/baselines/reference/conformance/importTag5.js index 3d2361c1ad8fb..b8856f6c0ba42 100644 --- a/tsc/testdata/baselines/reference/conformance/importTag5.js +++ b/tsc/testdata/baselines/reference/conformance/importTag5.js @@ -4,6 +4,9 @@ export interface Foo { a: number; } +export interface Unused { + unused: true; +} //// [foo.js] /** @@ -15,6 +18,26 @@ export interface Foo { */ function f(foo) {} +//// [imports.js] +/** + * @import { Foo, Unused } from "./types" + */ +/** + * @param {Foo} foo + */ +export function filtered(foo) { + return foo; +} +/** + * @import * as types from "./types" + */ +/** + * @param {types.Foo} foo + */ +export function namespace(foo) { + return foo; +} + @@ -22,7 +45,10 @@ function f(foo) {} export interface Foo { a: number; } -//// [foo.d.ts] +export interface Unused { + unused: true; +} +//# sourceMappingURL=types.d.ts.map//// [foo.d.ts] /** * @import { Foo } from "./types" */ @@ -31,3 +57,21 @@ import type { Foo } from "./types"; * @param { Foo } foo */ declare function f(foo: Foo): void; +//# sourceMappingURL=foo.d.ts.map//// [imports.d.ts] +import type { Foo } from "./types"; +/** + * @import { Foo, Unused } from "./types" + */ +/** + * @param {Foo} foo + */ +export declare function filtered(foo: Foo): Foo; +import type * as types from "./types"; +/** + * @import * as types from "./types" + */ +/** + * @param {types.Foo} foo + */ +export declare function namespace(foo: types.Foo): Foo; +//# sourceMappingURL=imports.d.ts.map \ No newline at end of file diff --git a/tsc/testdata/baselines/reference/conformance/importTag5.js.map b/tsc/testdata/baselines/reference/conformance/importTag5.js.map new file mode 100644 index 0000000000000..e26abef78ca3b --- /dev/null +++ b/tsc/testdata/baselines/reference/conformance/importTag5.js.map @@ -0,0 +1,11 @@ +//// [foo.d.ts.map] +{"version":3,"file":"foo.d.ts","sources":["foo.js"],"names":[],"mappings":"AAAA;;GAEG;AADA,OAAQ,KAAA,EAAE,GAAG,EAAE,MAAM,SAAS,CAAA;AAGjC;;GAEG;AACH,iBAAS,CAAC,CAAC,GAAG,EAFF,GAEE,QAAI"} +//// https://sokra.github.io/source-map-visualization#base64,LyoqDQogKiBAaW1wb3J0IHsgRm9vIH0gZnJvbSAiLi90eXBlcyINCiAqLw0KaW1wb3J0IHR5cGUgeyBGb28gfSBmcm9tICIuL3R5cGVzIjsNCi8qKg0KICogQHBhcmFtIHsgRm9vIH0gZm9vDQogKi8NCmRlY2xhcmUgZnVuY3Rpb24gZihmb286IEZvbyk6IHZvaWQ7DQovLyMgc291cmNlTWFwcGluZ1VSTD1mb28uZC50cy5tYXA=,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZm9vLmQudHMiLCJzb3VyY2VzIjpbImZvby5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTs7R0FFRztBQURBLE9BQVEsS0FBQSxFQUFFLEdBQUcsRUFBRSxNQUFNLFNBQVMsQ0FBQTtBQUdqQzs7R0FFRztBQUNILGlCQUFTLENBQUMsQ0FBQyxHQUFHLEVBRkYsR0FFRSxRQUFJIn0=,LyoqCiAqIEBpbXBvcnQgeyBGb28gfSBmcm9tICIuL3R5cGVzIgogKi8KCi8qKgogKiBAcGFyYW0geyBGb28gfSBmb28KICovCmZ1bmN0aW9uIGYoZm9vKSB7fQo= + +//// [imports.d.ts.map] +{"version":3,"file":"imports.d.ts","sources":["imports.js"],"names":[],"mappings":"AACG,OAAQ,KAAA,EAAE,GAAG,EAAU,MAAM,SAAS,CAAA;AADzC;;GAEG;AACH;;GAEG;AACH,wBAAgB,QAAQ,CAAC,GAAG,EAFjB,GAEiB,OAE3B;AAEE,OAAQ,KAAA,KAAK,KAAK,MAAM,SAAS,CAAA;AADpC;;GAEG;AACH;;GAEG;AACH,wBAAgB,SAAS,CAAC,GAAG,EAFlB,KAAK,CAAC,GAEY,OAE5B"} +//// https://sokra.github.io/source-map-visualization#base64,aW1wb3J0IHR5cGUgeyBGb28gfSBmcm9tICIuL3R5cGVzIjsNCi8qKg0KICogQGltcG9ydCB7IEZvbywgVW51c2VkIH0gZnJvbSAiLi90eXBlcyINCiAqLw0KLyoqDQogKiBAcGFyYW0ge0Zvb30gZm9vDQogKi8NCmV4cG9ydCBkZWNsYXJlIGZ1bmN0aW9uIGZpbHRlcmVkKGZvbzogRm9vKTogRm9vOw0KaW1wb3J0IHR5cGUgKiBhcyB0eXBlcyBmcm9tICIuL3R5cGVzIjsNCi8qKg0KICogQGltcG9ydCAqIGFzIHR5cGVzIGZyb20gIi4vdHlwZXMiDQogKi8NCi8qKg0KICogQHBhcmFtIHt0eXBlcy5Gb299IGZvbw0KICovDQpleHBvcnQgZGVjbGFyZSBmdW5jdGlvbiBuYW1lc3BhY2UoZm9vOiB0eXBlcy5Gb28pOiBGb287DQovLyMgc291cmNlTWFwcGluZ1VSTD1pbXBvcnRzLmQudHMubWFw,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW1wb3J0cy5kLnRzIiwic291cmNlcyI6WyJpbXBvcnRzLmpzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUNHLE9BQVEsS0FBQSxFQUFFLEdBQUcsRUFBVSxNQUFNLFNBQVMsQ0FBQTtBQUR6Qzs7R0FFRztBQUNIOztHQUVHO0FBQ0gsd0JBQWdCLFFBQVEsQ0FBQyxHQUFHLEVBRmpCLEdBRWlCLE9BRTNCO0FBRUUsT0FBUSxLQUFBLEtBQUssS0FBSyxNQUFNLFNBQVMsQ0FBQTtBQURwQzs7R0FFRztBQUNIOztHQUVHO0FBQ0gsd0JBQWdCLFNBQVMsQ0FBQyxHQUFHLEVBRmxCLEtBQUssQ0FBQyxHQUVZLE9BRTVCIn0=,LyoqCiAqIEBpbXBvcnQgeyBGb28sIFVudXNlZCB9IGZyb20gIi4vdHlwZXMiCiAqLwovKioKICogQHBhcmFtIHtGb299IGZvbwogKi8KZXhwb3J0IGZ1bmN0aW9uIGZpbHRlcmVkKGZvbykgewogICAgcmV0dXJuIGZvbzsKfQovKioKICogQGltcG9ydCAqIGFzIHR5cGVzIGZyb20gIi4vdHlwZXMiCiAqLwovKioKICogQHBhcmFtIHt0eXBlcy5Gb299IGZvbwogKi8KZXhwb3J0IGZ1bmN0aW9uIG5hbWVzcGFjZShmb28pIHsKICAgIHJldHVybiBmb287Cn0K + +//// [types.d.ts.map] +{"version":3,"file":"types.d.ts","sources":["types.ts"],"names":[],"mappings":"AAAA,MAAM,WAAW,GAAG;IAChB,CAAC,EAAE,MAAM,CAAC;CACb;AACD,MAAM,WAAW,MAAM;IACnB,MAAM,EAAE,IAAI,CAAC;CAChB"} +//// https://sokra.github.io/source-map-visualization#base64,ZXhwb3J0IGludGVyZmFjZSBGb28gew0KICAgIGE6IG51bWJlcjsNCn0NCmV4cG9ydCBpbnRlcmZhY2UgVW51c2VkIHsNCiAgICB1bnVzZWQ6IHRydWU7DQp9DQovLyMgc291cmNlTWFwcGluZ1VSTD10eXBlcy5kLnRzLm1hcA==,eyJ2ZXJzaW9uIjozLCJmaWxlIjoidHlwZXMuZC50cyIsInNvdXJjZXMiOlsidHlwZXMudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsTUFBTSxXQUFXLEdBQUc7SUFDaEIsQ0FBQyxFQUFFLE1BQU0sQ0FBQztDQUNiO0FBQ0QsTUFBTSxXQUFXLE1BQU07SUFDbkIsTUFBTSxFQUFFLElBQUksQ0FBQztDQUNoQiJ9,ZXhwb3J0IGludGVyZmFjZSBGb28gewogICAgYTogbnVtYmVyOwp9CmV4cG9ydCBpbnRlcmZhY2UgVW51c2VkIHsKICAgIHVudXNlZDogdHJ1ZTsKfQo= diff --git a/tsc/testdata/baselines/reference/conformance/importTag5.sourcemap.txt b/tsc/testdata/baselines/reference/conformance/importTag5.sourcemap.txt new file mode 100644 index 0000000000000..5afb9fdff1454 --- /dev/null +++ b/tsc/testdata/baselines/reference/conformance/importTag5.sourcemap.txt @@ -0,0 +1,389 @@ +=================================================================== +JsFile: types.d.ts +mapUrl: types.d.ts.map +sourceRoot: +sources: types.ts +=================================================================== +------------------------------------------------------------------- +emittedFile:/types.d.ts +sourceFile:types.ts +------------------------------------------------------------------- +>>>export interface Foo { +1 > +2 >^^^^^^ +3 > ^^^^^^^^^^^ +4 > ^^^ +1 > +2 >export +3 > interface +4 > Foo +1 >Emitted(1, 1) Source(1, 1) + SourceIndex(0) +2 >Emitted(1, 7) Source(1, 7) + SourceIndex(0) +3 >Emitted(1, 18) Source(1, 18) + SourceIndex(0) +4 >Emitted(1, 21) Source(1, 21) + SourceIndex(0) +--- +>>> a: number; +1 >^^^^ +2 > ^ +3 > ^^ +4 > ^^^^^^ +5 > ^ +1 > { + > +2 > a +3 > : +4 > number +5 > ; +1 >Emitted(2, 5) Source(2, 5) + SourceIndex(0) +2 >Emitted(2, 6) Source(2, 6) + SourceIndex(0) +3 >Emitted(2, 8) Source(2, 8) + SourceIndex(0) +4 >Emitted(2, 14) Source(2, 14) + SourceIndex(0) +5 >Emitted(2, 15) Source(2, 15) + SourceIndex(0) +--- +>>>} +1 >^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^-> +1 > + >} +1 >Emitted(3, 2) Source(3, 2) + SourceIndex(0) +--- +>>>export interface Unused { +1-> +2 >^^^^^^ +3 > ^^^^^^^^^^^ +4 > ^^^^^^ +1-> + > +2 >export +3 > interface +4 > Unused +1->Emitted(4, 1) Source(4, 1) + SourceIndex(0) +2 >Emitted(4, 7) Source(4, 7) + SourceIndex(0) +3 >Emitted(4, 18) Source(4, 18) + SourceIndex(0) +4 >Emitted(4, 24) Source(4, 24) + SourceIndex(0) +--- +>>> unused: true; +1 >^^^^ +2 > ^^^^^^ +3 > ^^ +4 > ^^^^ +5 > ^ +1 > { + > +2 > unused +3 > : +4 > true +5 > ; +1 >Emitted(5, 5) Source(5, 5) + SourceIndex(0) +2 >Emitted(5, 11) Source(5, 11) + SourceIndex(0) +3 >Emitted(5, 13) Source(5, 13) + SourceIndex(0) +4 >Emitted(5, 17) Source(5, 17) + SourceIndex(0) +5 >Emitted(5, 18) Source(5, 18) + SourceIndex(0) +--- +>>>} +1 >^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1 > + >} +1 >Emitted(6, 2) Source(6, 2) + SourceIndex(0) +--- +>>>//# sourceMappingURL=types.d.ts.map=================================================================== +JsFile: foo.d.ts +mapUrl: foo.d.ts.map +sourceRoot: +sources: foo.js +=================================================================== +------------------------------------------------------------------- +emittedFile:/foo.d.ts +sourceFile:foo.js +------------------------------------------------------------------- +>>>/** +1 > +2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1 > +1 >Emitted(1, 1) Source(1, 1) + SourceIndex(0) +--- +>>> * @import { Foo } from "./types" +>>> */ +1->^^^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1->/** + > * @import { Foo } from "./types" + > */ +1->Emitted(3, 4) Source(3, 4) + SourceIndex(0) +--- +>>>import type { Foo } from "./types"; +1-> +2 >^^^^^^^ +3 > ^^^^^ +4 > ^^ +5 > ^^^ +6 > ^^ +7 > ^^^^^^ +8 > ^^^^^^^^^ +9 > ^ +1-> +2 >@import +3 > +4 > { +5 > Foo +6 > } +7 > from +8 > "./types" +9 > +1->Emitted(4, 1) Source(2, 4) + SourceIndex(0) +2 >Emitted(4, 8) Source(2, 12) + SourceIndex(0) +3 >Emitted(4, 13) Source(2, 12) + SourceIndex(0) +4 >Emitted(4, 15) Source(2, 14) + SourceIndex(0) +5 >Emitted(4, 18) Source(2, 17) + SourceIndex(0) +6 >Emitted(4, 20) Source(2, 19) + SourceIndex(0) +7 >Emitted(4, 26) Source(2, 25) + SourceIndex(0) +8 >Emitted(4, 35) Source(2, 34) + SourceIndex(0) +9 >Emitted(4, 36) Source(2, 34) + SourceIndex(0) +--- +>>>/** +1 > +2 >^^^^^^^^^^^^^^^^^^^^^^-> +1 > + > */ + > + > +1 >Emitted(5, 1) Source(5, 1) + SourceIndex(0) +--- +>>> * @param { Foo } foo +>>> */ +1->^^^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1->/** + > * @param { Foo } foo + > */ +1->Emitted(7, 4) Source(7, 4) + SourceIndex(0) +--- +>>>declare function f(foo: Foo): void; +1-> +2 >^^^^^^^^^^^^^^^^^ +3 > ^ +4 > ^ +5 > ^^^ +6 > ^^ +7 > ^^^ +8 > ^^^^^^^^ +1-> + > +2 >function +3 > f +4 > ( +5 > foo +6 > +7 > Foo } foo + > */ + > function f(foo +8 > ) {} +1->Emitted(8, 1) Source(8, 1) + SourceIndex(0) +2 >Emitted(8, 18) Source(8, 10) + SourceIndex(0) +3 >Emitted(8, 19) Source(8, 11) + SourceIndex(0) +4 >Emitted(8, 20) Source(8, 12) + SourceIndex(0) +5 >Emitted(8, 23) Source(8, 15) + SourceIndex(0) +6 >Emitted(8, 25) Source(6, 13) + SourceIndex(0) +7 >Emitted(8, 28) Source(8, 15) + SourceIndex(0) +8 >Emitted(8, 36) Source(8, 19) + SourceIndex(0) +--- +>>>//# sourceMappingURL=foo.d.ts.map=================================================================== +JsFile: imports.d.ts +mapUrl: imports.d.ts.map +sourceRoot: +sources: imports.js +=================================================================== +------------------------------------------------------------------- +emittedFile:/imports.d.ts +sourceFile:imports.js +------------------------------------------------------------------- +>>>import type { Foo } from "./types"; +1 > +2 >^^^^^^^ +3 > ^^^^^ +4 > ^^ +5 > ^^^ +6 > ^^ +7 > ^^^^^^ +8 > ^^^^^^^^^ +9 > ^ +1 >/** + > * +2 >@import +3 > +4 > { +5 > Foo +6 > , Unused } +7 > from +8 > "./types" +9 > +1 >Emitted(1, 1) Source(2, 4) + SourceIndex(0) +2 >Emitted(1, 8) Source(2, 12) + SourceIndex(0) +3 >Emitted(1, 13) Source(2, 12) + SourceIndex(0) +4 >Emitted(1, 15) Source(2, 14) + SourceIndex(0) +5 >Emitted(1, 18) Source(2, 17) + SourceIndex(0) +6 >Emitted(1, 20) Source(2, 27) + SourceIndex(0) +7 >Emitted(1, 26) Source(2, 33) + SourceIndex(0) +8 >Emitted(1, 35) Source(2, 42) + SourceIndex(0) +9 >Emitted(1, 36) Source(2, 42) + SourceIndex(0) +--- +>>>/** +1 > +2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1 > +1 >Emitted(2, 1) Source(1, 1) + SourceIndex(0) +--- +>>> * @import { Foo, Unused } from "./types" +>>> */ +1->^^^ +2 > ^-> +1->/** + > * @import { Foo, Unused } from "./types" + > */ +1->Emitted(4, 4) Source(3, 4) + SourceIndex(0) +--- +>>>/** +1-> +2 >^^^^^^^^^^^^^^^^^^^^-> +1-> + > +1->Emitted(5, 1) Source(4, 1) + SourceIndex(0) +--- +>>> * @param {Foo} foo +>>> */ +1->^^^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1->/** + > * @param {Foo} foo + > */ +1->Emitted(7, 4) Source(6, 4) + SourceIndex(0) +--- +>>>export declare function filtered(foo: Foo): Foo; +1-> +2 >^^^^^^^^^^^^^^^^^^^^^^^^ +3 > ^^^^^^^^ +4 > ^ +5 > ^^^ +6 > ^^ +7 > ^^^ +8 > ^^^^^^^ +1-> + > +2 >export function +3 > filtered +4 > ( +5 > foo +6 > +7 > Foo} foo + > */ + > export function filtered(foo +8 > ) { + > return foo; + > } +1->Emitted(8, 1) Source(7, 1) + SourceIndex(0) +2 >Emitted(8, 25) Source(7, 17) + SourceIndex(0) +3 >Emitted(8, 33) Source(7, 25) + SourceIndex(0) +4 >Emitted(8, 34) Source(7, 26) + SourceIndex(0) +5 >Emitted(8, 37) Source(7, 29) + SourceIndex(0) +6 >Emitted(8, 39) Source(5, 12) + SourceIndex(0) +7 >Emitted(8, 42) Source(7, 29) + SourceIndex(0) +8 >Emitted(8, 49) Source(9, 2) + SourceIndex(0) +--- +>>>import type * as types from "./types"; +1 > +2 >^^^^^^^ +3 > ^^^^^ +4 > ^^^^^ +5 > ^^^^^ +6 > ^^^^^^ +7 > ^^^^^^^^^ +8 > ^ +1 > + >/** + > * +2 >@import +3 > +4 > * as +5 > types +6 > from +7 > "./types" +8 > +1 >Emitted(9, 1) Source(11, 4) + SourceIndex(0) +2 >Emitted(9, 8) Source(11, 12) + SourceIndex(0) +3 >Emitted(9, 13) Source(11, 12) + SourceIndex(0) +4 >Emitted(9, 18) Source(11, 17) + SourceIndex(0) +5 >Emitted(9, 23) Source(11, 22) + SourceIndex(0) +6 >Emitted(9, 29) Source(11, 28) + SourceIndex(0) +7 >Emitted(9, 38) Source(11, 37) + SourceIndex(0) +8 >Emitted(9, 39) Source(11, 37) + SourceIndex(0) +--- +>>>/** +1 > +2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1 > +1 >Emitted(10, 1) Source(10, 1) + SourceIndex(0) +--- +>>> * @import * as types from "./types" +>>> */ +1->^^^ +2 > ^-> +1->/** + > * @import * as types from "./types" + > */ +1->Emitted(12, 4) Source(12, 4) + SourceIndex(0) +--- +>>>/** +1-> +2 >^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1-> + > +1->Emitted(13, 1) Source(13, 1) + SourceIndex(0) +--- +>>> * @param {types.Foo} foo +>>> */ +1->^^^ +2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-> +1->/** + > * @param {types.Foo} foo + > */ +1->Emitted(15, 4) Source(15, 4) + SourceIndex(0) +--- +>>>export declare function namespace(foo: types.Foo): Foo; +1-> +2 >^^^^^^^^^^^^^^^^^^^^^^^^ +3 > ^^^^^^^^^ +4 > ^ +5 > ^^^ +6 > ^^ +7 > ^^^^^ +8 > ^ +9 > ^^^ +10> ^^^^^^^ +1-> + > +2 >export function +3 > namespace +4 > ( +5 > foo +6 > +7 > types +8 > . +9 > Foo} foo + > */ + > export function namespace(foo +10> ) { + > return foo; + > } +1->Emitted(16, 1) Source(16, 1) + SourceIndex(0) +2 >Emitted(16, 25) Source(16, 17) + SourceIndex(0) +3 >Emitted(16, 34) Source(16, 26) + SourceIndex(0) +4 >Emitted(16, 35) Source(16, 27) + SourceIndex(0) +5 >Emitted(16, 38) Source(16, 30) + SourceIndex(0) +6 >Emitted(16, 40) Source(14, 12) + SourceIndex(0) +7 >Emitted(16, 45) Source(14, 17) + SourceIndex(0) +8 >Emitted(16, 46) Source(14, 18) + SourceIndex(0) +9 >Emitted(16, 49) Source(16, 30) + SourceIndex(0) +10>Emitted(16, 56) Source(18, 2) + SourceIndex(0) +--- +>>>//# sourceMappingURL=imports.d.ts.map \ No newline at end of file diff --git a/tsc/testdata/baselines/reference/conformance/importTag5.symbols b/tsc/testdata/baselines/reference/conformance/importTag5.symbols index 35242e6c8988e..9775067933615 100644 --- a/tsc/testdata/baselines/reference/conformance/importTag5.symbols +++ b/tsc/testdata/baselines/reference/conformance/importTag5.symbols @@ -7,6 +7,12 @@ export interface Foo { a: number; >a : Symbol(Foo.a, Decl(types.ts, 0, 22)) } +export interface Unused { +>Unused : Symbol(Unused, Decl(types.ts, 2, 1)) + + unused: true; +>unused : Symbol(Unused.unused, Decl(types.ts, 3, 25)) +} === /foo.js === /** @@ -20,3 +26,31 @@ function f(foo) {} >f : Symbol(f, Decl(foo.js, 0, 0)) >foo : Symbol(foo, Decl(foo.js, 7, 11)) +=== /imports.js === +/** + * @import { Foo, Unused } from "./types" + */ +/** + * @param {Foo} foo + */ +export function filtered(foo) { +>filtered : Symbol(filtered, Decl(imports.js, 0, 0)) +>foo : Symbol(foo, Decl(imports.js, 6, 25)) + + return foo; +>foo : Symbol(foo, Decl(imports.js, 6, 25)) +} +/** + * @import * as types from "./types" + */ +/** + * @param {types.Foo} foo + */ +export function namespace(foo) { +>namespace : Symbol(namespace, Decl(imports.js, 8, 1)) +>foo : Symbol(foo, Decl(imports.js, 15, 26)) + + return foo; +>foo : Symbol(foo, Decl(imports.js, 15, 26)) +} + diff --git a/tsc/testdata/baselines/reference/conformance/importTag5.types b/tsc/testdata/baselines/reference/conformance/importTag5.types index 7d9641be55328..c2ff4244e0694 100644 --- a/tsc/testdata/baselines/reference/conformance/importTag5.types +++ b/tsc/testdata/baselines/reference/conformance/importTag5.types @@ -5,6 +5,11 @@ export interface Foo { a: number; >a : number } +export interface Unused { + unused: true; +>unused : true +>true : true +} === /foo.js === /** @@ -18,3 +23,31 @@ function f(foo) {} >f : (foo: Foo) => void >foo : Foo +=== /imports.js === +/** + * @import { Foo, Unused } from "./types" + */ +/** + * @param {Foo} foo + */ +export function filtered(foo) { +>filtered : (foo: Foo) => Foo +>foo : Foo + + return foo; +>foo : Foo +} +/** + * @import * as types from "./types" + */ +/** + * @param {types.Foo} foo + */ +export function namespace(foo) { +>namespace : (foo: types.Foo) => Foo +>foo : Foo + + return foo; +>foo : Foo +} + diff --git a/tsc/testdata/tests/cases/conformance/jsdoc/importTag5.ts b/tsc/testdata/tests/cases/conformance/jsdoc/importTag5.ts index eb934954c4dba..f8f7836a2eb9c 100644 --- a/tsc/testdata/tests/cases/conformance/jsdoc/importTag5.ts +++ b/tsc/testdata/tests/cases/conformance/jsdoc/importTag5.ts @@ -3,11 +3,15 @@ // @allowJs: true // @declaration: true // @emitDeclarationOnly: true +// @declarationMap: true // @filename: /types.ts export interface Foo { a: number; } +export interface Unused { + unused: true; +} // @filename: /foo.js /** @@ -18,3 +22,23 @@ export interface Foo { * @param { Foo } foo */ function f(foo) {} + +// @filename: /imports.js +/** + * @import { Foo, Unused } from "./types" + */ +/** + * @param {Foo} foo + */ +export function filtered(foo) { + return foo; +} +/** + * @import * as types from "./types" + */ +/** + * @param {types.Foo} foo + */ +export function namespace(foo) { + return foo; +} From 424618b8aa3524cb729ec531ef1f69ca2dc3618f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:22:32 -0700 Subject: [PATCH 2/3] Lint writes to protected record kind fields Unsafe casts trust the kind stored in a node, type, or type mapper header to describe the allocation around it. Assigning that field, or copying a record by value, can make a header claim to be a larger record than its storage holds. Mark those fields with a customlint:"protected" struct tag and reject writes, address-taking, keyed literals, and by-value copies of structs that contain a protected field. The constructors that legitimately set a kind opt out with nolint comments, so new exceptions stay visible in review. Casts already check the kind at runtime, and kind zero is never valid, so zero-valued records cannot be cast into larger ones. --- tools/customlint/plugin.go | 1 + tools/customlint/protectedfield.go | 208 ++++++++++++++++++ .../protectedfield/consumer/consumer.go | 53 +++++ .../consumer/consumer.go.golden | 96 ++++++++ .../testdata/protectedfield/record/record.go | 53 +++++ .../protectedfield/record/record.go.golden | 60 +++++ tsc/internal/ast/ast.go | 5 +- tsc/internal/checker/checker.go | 2 +- tsc/internal/checker/mapper.go | 18 +- tsc/internal/checker/types.go | 2 +- 10 files changed, 485 insertions(+), 13 deletions(-) create mode 100644 tools/customlint/protectedfield.go create mode 100644 tools/customlint/testdata/protectedfield/consumer/consumer.go create mode 100644 tools/customlint/testdata/protectedfield/consumer/consumer.go.golden create mode 100644 tools/customlint/testdata/protectedfield/record/record.go create mode 100644 tools/customlint/testdata/protectedfield/record/record.go.golden diff --git a/tools/customlint/plugin.go b/tools/customlint/plugin.go index fd9c5b9e7dfce..e80ddd3907e21 100644 --- a/tools/customlint/plugin.go +++ b/tools/customlint/plugin.go @@ -20,6 +20,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { cleanupAnalyzer, caseBodyAnalyzer, forbidParentAccessAnalyzer, + protectedFieldAnalyzer, shadowAnalyzer, typedPathsAnalyzer, unexportedAPIAnalyzer, diff --git a/tools/customlint/protectedfield.go b/tools/customlint/protectedfield.go new file mode 100644 index 0000000000000..abd0cd9671b33 --- /dev/null +++ b/tools/customlint/protectedfield.go @@ -0,0 +1,208 @@ +package customlint + +import ( + "go/ast" + "go/token" + "go/types" + "reflect" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// protectedFieldAnalyzer guards fields tagged `customlint:"protected"`, such as +// the kind headers that unsafe casts trust to identify the enclosing allocation. +// Writing such a field, or copying a struct that contains one, can make a value +// claim to be a larger record than its storage holds. Intentional writes are +// suppressed with //nolint:customlint. +var protectedFieldAnalyzer = &analysis.Analyzer{ + Name: "protectedfield", + Doc: "forbids writes to protected fields and by-value copies of structs containing them", + Requires: []*analysis.Analyzer{ + inspect.Analyzer, + }, + Run: func(pass *analysis.Pass) (any, error) { + return (&protectedFieldPass{pass: pass, contains: make(map[types.Type]bool)}).run() + }, +} + +type protectedFieldPass struct { + pass *analysis.Pass + contains map[types.Type]bool +} + +func (p *protectedFieldPass) run() (any, error) { + in := p.pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + for c := range in.Root().Preorder( + (*ast.AssignStmt)(nil), + (*ast.IncDecStmt)(nil), + (*ast.UnaryExpr)(nil), + (*ast.CompositeLit)(nil), + (*ast.FuncType)(nil), + (*ast.FuncDecl)(nil), + ) { + switch node := c.Node().(type) { + case *ast.AssignStmt: + for _, lhs := range node.Lhs { + p.checkWrite(lhs, "write to") + } + case *ast.IncDecStmt: + p.checkWrite(node.X, "write to") + case *ast.UnaryExpr: + if node.Op == token.AND { + p.checkWrite(node.X, "address of") + } + case *ast.CompositeLit: + p.checkLiteral(node) + case *ast.FuncType: + p.checkFields(node.Params, "parameter") + p.checkFields(node.Results, "result") + case *ast.FuncDecl: + p.checkFields(node.Recv, "receiver") + } + } + + for id, obj := range p.pass.TypesInfo.Defs { + if v, ok := obj.(*types.Var); ok && (v.Kind() == types.PackageVar || v.Kind() == types.LocalVar) && p.containsProtected(v.Type()) { + p.pass.Reportf(id.Pos(), "variable %s holds %s by value, which contains a protected field", id.Name, v.Type()) + } + } + + for c := range in.Root().Preorder((*ast.StarExpr)(nil), (*ast.IndexExpr)(nil), (*ast.CallExpr)(nil), (*ast.CompositeLit)(nil), (*ast.SelectorExpr)(nil)) { + expr := c.Node().(ast.Expr) + tv, ok := p.pass.TypesInfo.Types[expr] + if !ok || !tv.IsValue() || !p.containsProtected(tv.Type) || p.allowedValue(c) { + continue + } + p.pass.Reportf(expr.Pos(), "by-value copy of %s, which contains a protected field", tv.Type) + } + + return nil, nil +} + +func (p *protectedFieldPass) checkWrite(expr ast.Expr, what string) { + sel, ok := ast.Unparen(expr).(*ast.SelectorExpr) + if !ok { + return + } + selection := p.pass.TypesInfo.Selections[sel] + if selection == nil || selection.Kind() != types.FieldVal { + return + } + typ := selection.Recv() + var tag string + for _, index := range selection.Index() { + if ptr, ok := typ.Underlying().(*types.Pointer); ok { + typ = ptr.Elem() + } + st := typ.Underlying().(*types.Struct) + tag = st.Tag(index) + typ = st.Field(index).Type() + } + if isProtectedTag(tag) { + p.pass.Reportf(sel.Sel.Pos(), "%s protected field %s", what, sel.Sel.Name) + } +} + +func (p *protectedFieldPass) checkLiteral(lit *ast.CompositeLit) { + st, ok := p.pass.TypesInfo.TypeOf(lit).Underlying().(*types.Struct) + if !ok { + return + } + for i, elt := range lit.Elts { + index := i + if kv, ok := elt.(*ast.KeyValueExpr); ok { + key, ok := kv.Key.(*ast.Ident) + if !ok { + continue + } + index = -1 + for j := range st.NumFields() { + if st.Field(j).Name() == key.Name { + index = j + break + } + } + if index < 0 { + continue + } + } + if index < st.NumFields() && isProtectedTag(st.Tag(index)) { + p.pass.Reportf(elt.Pos(), "write to protected field %s", st.Field(index).Name()) + } + } +} + +func (p *protectedFieldPass) checkFields(fields *ast.FieldList, what string) { + if fields == nil { + return + } + for _, field := range fields.List { + if typ := p.pass.TypesInfo.TypeOf(field.Type); typ != nil && p.containsProtected(typ) { + p.pass.Reportf(field.Type.Pos(), "%s holds %s by value, which contains a protected field", what, typ) + } + } +} + +// allowedValue reports whether a record-valued expression is used without +// copying it: selecting a field or method, taking its address, indexing it, or +// as an operand of unsafe.Sizeof, Alignof, or Offsetof. +func (p *protectedFieldPass) allowedValue(c inspector.Cursor) bool { + child := c.Node() + for parent := c.Parent(); ; parent = parent.Parent() { + switch node := parent.Node().(type) { + case *ast.ParenExpr: + child = node + continue + case *ast.SelectorExpr: + return node.X == child + case *ast.UnaryExpr: + return node.Op == token.AND + case *ast.IndexExpr: + return node.X == child + case *ast.SliceExpr: + return node.X == child + } + for call := range parent.Enclosing((*ast.CallExpr)(nil)) { + if p.isUnsafeLayoutCall(call.Node().(*ast.CallExpr)) { + return true + } + } + return false + } +} + +func (p *protectedFieldPass) isUnsafeLayoutCall(call *ast.CallExpr) bool { + sel, ok := ast.Unparen(call.Fun).(*ast.SelectorExpr) + if !ok { + return false + } + builtin, ok := p.pass.TypesInfo.Uses[sel.Sel].(*types.Builtin) + return ok && (builtin.Name() == "Sizeof" || builtin.Name() == "Alignof" || builtin.Name() == "Offsetof") +} + +func (p *protectedFieldPass) containsProtected(typ types.Type) bool { + if result, ok := p.contains[typ]; ok { + return result + } + result := false + switch t := typ.Underlying().(type) { + case *types.Struct: + for i := range t.NumFields() { + if isProtectedTag(t.Tag(i)) || p.containsProtected(t.Field(i).Type()) { + result = true + break + } + } + case *types.Array: + result = p.containsProtected(t.Elem()) + } + p.contains[typ] = result + return result +} + +func isProtectedTag(tag string) bool { + return reflect.StructTag(tag).Get("customlint") == "protected" +} diff --git a/tools/customlint/testdata/protectedfield/consumer/consumer.go b/tools/customlint/testdata/protectedfield/consumer/consumer.go new file mode 100644 index 0000000000000..49c22227903dd --- /dev/null +++ b/tools/customlint/testdata/protectedfield/consumer/consumer.go @@ -0,0 +1,53 @@ +package consumer + +import "testdata/protectedfield/record" + +type Container struct { + header *record.Header + small record.Small +} + +func writes(h *record.Header, s *record.Small, d *record.Defined) { + h.Kind = record.KindLarge + s.Kind = record.KindLarge + s.Header.Kind++ + d.Kind = record.KindLarge + h.Kind, h.Flags = record.KindLarge, 1 + _ = &h.Kind + h.Flags = 1 + _ = &h.Flags + _ = h.Kind +} + +func literals() { + _ = &record.Header{Kind: record.KindLarge} + _ = &record.Header{record.KindLarge, 1} + _ = &record.Header{Flags: 1} + _ = &record.Small{} + _ = &record.Alias{Kind: record.KindSmall} +} + +func copies(h *record.Header, s *record.Small, hs []record.Header) { + *h = *record.NewSmall(1) + small := *s + var local record.Header + _ = record.Small{} + _ = hs[0] + _ = &hs[0] + _ = hs[0].Flags + _ = (*h).Flags + _ = &(*h) + _ = s.Header + _ = &s.Header + for _, v := range hs { + _ = v + } + _, _ = small, local + _ = h.AsLarge() +} + +func param(h record.Header) {} + +func result() (h *record.Header) { return nil } + +func valueResult() record.Small { return record.Small{} } diff --git a/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden b/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden new file mode 100644 index 0000000000000..cd8db282a59c6 --- /dev/null +++ b/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden @@ -0,0 +1,96 @@ + package consumer + + import "testdata/protectedfield/record" + + type Container struct { + header *record.Header + small record.Small + } + + func writes(h *record.Header, s *record.Small, d *record.Defined) { + h.Kind = record.KindLarge + ~ +!!! protectedfield: write to protected field Kind + s.Kind = record.KindLarge + ~ +!!! protectedfield: write to protected field Kind + s.Header.Kind++ + ~ +!!! protectedfield: write to protected field Kind + d.Kind = record.KindLarge + ~ +!!! protectedfield: write to protected field Kind + h.Kind, h.Flags = record.KindLarge, 1 + ~ +!!! protectedfield: write to protected field Kind + _ = &h.Kind + ~ +!!! protectedfield: address of protected field Kind + h.Flags = 1 + _ = &h.Flags + _ = h.Kind + } + + func literals() { + _ = &record.Header{Kind: record.KindLarge} + ~ +!!! protectedfield: write to protected field Kind + _ = &record.Header{record.KindLarge, 1} + ~ +!!! protectedfield: write to protected field Kind + _ = &record.Header{Flags: 1} + _ = &record.Small{} + _ = &record.Alias{Kind: record.KindSmall} + ~ +!!! protectedfield: write to protected field Kind + } + + func copies(h *record.Header, s *record.Small, hs []record.Header) { + *h = *record.NewSmall(1) + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Header, which contains a protected field + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Header, which contains a protected field + small := *s + ~ +!!! protectedfield: variable small holds testdata/protectedfield/record.Small by value, which contains a protected field + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Small, which contains a protected field + var local record.Header + ~ +!!! protectedfield: variable local holds testdata/protectedfield/record.Header by value, which contains a protected field + _ = record.Small{} + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Small, which contains a protected field + _ = hs[0] + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Header, which contains a protected field + _ = &hs[0] + _ = hs[0].Flags + _ = (*h).Flags + _ = &(*h) + _ = s.Header + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Header, which contains a protected field + _ = &s.Header + for _, v := range hs { + ~ +!!! protectedfield: variable v holds testdata/protectedfield/record.Header by value, which contains a protected field + _ = v + } + _, _ = small, local + _ = h.AsLarge() + } + + func param(h record.Header) {} + ~ +!!! protectedfield: parameter holds testdata/protectedfield/record.Header by value, which contains a protected field + + func result() (h *record.Header) { return nil } + + func valueResult() record.Small { return record.Small{} } + ~ +!!! protectedfield: result holds testdata/protectedfield/record.Small by value, which contains a protected field + ~ +!!! protectedfield: by-value copy of testdata/protectedfield/record.Small, which contains a protected field + diff --git a/tools/customlint/testdata/protectedfield/record/record.go b/tools/customlint/testdata/protectedfield/record/record.go new file mode 100644 index 0000000000000..63ece2c39915d --- /dev/null +++ b/tools/customlint/testdata/protectedfield/record/record.go @@ -0,0 +1,53 @@ +package record + +import "unsafe" + +type Kind uint8 + +const ( + KindUnknown Kind = iota + KindSmall + KindLarge +) + +type Header struct { + Kind Kind `customlint:"protected"` + Flags uint32 +} + +type Small struct { + Header + value int +} + +type Large struct { + Header + values [4]int +} + +type Alias = Header + +type Defined Header + +var _ [0 - unsafe.Offsetof(Large{}.Header)]byte + +var _ = unsafe.Sizeof(Header{}) + +func NewSmall(value int) *Header { + s := &Small{value: value} + s.Kind = KindSmall //nolint:customlint + return &s.Header +} + +func (h *Header) AsLarge() *Large { + if h.Kind != KindLarge { + panic("bad cast") + } + return (*Large)(unsafe.Pointer(h)) +} + +func (h Header) ByValue() Kind { + return h.Kind +} + +var zero Header diff --git a/tools/customlint/testdata/protectedfield/record/record.go.golden b/tools/customlint/testdata/protectedfield/record/record.go.golden new file mode 100644 index 0000000000000..2ad4052ed5b37 --- /dev/null +++ b/tools/customlint/testdata/protectedfield/record/record.go.golden @@ -0,0 +1,60 @@ + package record + + import "unsafe" + + type Kind uint8 + + const ( + KindUnknown Kind = iota + KindSmall + KindLarge + ) + + type Header struct { + Kind Kind `customlint:"protected"` + Flags uint32 + } + + type Small struct { + Header + value int + } + + type Large struct { + Header + values [4]int + } + + type Alias = Header + + type Defined Header + + var _ [0 - unsafe.Offsetof(Large{}.Header)]byte + + var _ = unsafe.Sizeof(Header{}) + + func NewSmall(value int) *Header { + s := &Small{value: value} + s.Kind = KindSmall //nolint:customlint + ~ +!!! protectedfield: write to protected field Kind + return &s.Header + } + + func (h *Header) AsLarge() *Large { + if h.Kind != KindLarge { + panic("bad cast") + } + return (*Large)(unsafe.Pointer(h)) + } + + func (h Header) ByValue() Kind { + ~ +!!! protectedfield: receiver holds testdata/protectedfield/record.Header by value, which contains a protected field + return h.Kind + } + + var zero Header + ~ +!!! protectedfield: variable zero holds testdata/protectedfield/record.Header by value, which contains a protected field + diff --git a/tsc/internal/ast/ast.go b/tsc/internal/ast/ast.go index 48ff1a8d63d56..a1b000886b208 100644 --- a/tsc/internal/ast/ast.go +++ b/tsc/internal/ast/ast.go @@ -75,7 +75,7 @@ func NewNodeFactory(hooks NodeFactoryHooks) *NodeFactory { func newNode(kind Kind, n *Node, hooks NodeFactoryHooks) *Node { n.Loc = core.UndefinedTextRange() - n.Kind = kind + n.Kind = kind //nolint:customlint if hooks.OnCreate != nil { hooks.OnCreate(n) } @@ -174,9 +174,10 @@ func (list *ModifierList) Clone(f *NodeFactory) *ModifierList { // Every concrete AST node embeds Node at offset zero. Kind identifies the // enclosing allocation, which generated accessors recover without a data pointer. +// Kind is set only by newNode; the protectedfield lint rejects other writes. type Node struct { - Kind Kind + Kind Kind `customlint:"protected"` Flags NodeFlags Loc core.TextRange id atomic.Uint64 diff --git a/tsc/internal/checker/checker.go b/tsc/internal/checker/checker.go index e58e96048e94b..cf464d685ea63 100644 --- a/tsc/internal/checker/checker.go +++ b/tsc/internal/checker/checker.go @@ -25603,7 +25603,7 @@ func (c *Checker) newType(flags TypeFlags, objectFlags ObjectFlags, t *Type, kin t.flags = flags t.objectFlags = objectFlags &^ (ObjectFlagsCouldContainTypeVariablesComputed | ObjectFlagsCouldContainTypeVariables | ObjectFlagsMembersResolved) t.id = TypeId(c.TypeCount) - t.kind = kind + t.kind = kind //nolint:customlint t.checker = c if c.tracer != nil { c.tracer.RecordType(t) diff --git a/tsc/internal/checker/mapper.go b/tsc/internal/checker/mapper.go index cb900e7372f89..cae1bac56d2e1 100644 --- a/tsc/internal/checker/mapper.go +++ b/tsc/internal/checker/mapper.go @@ -10,7 +10,7 @@ import ( // TypeMapper type TypeMapper struct { - kind TypeMapperKind + kind TypeMapperKind `customlint:"protected"` } type TypeMapperKind uint8 @@ -168,7 +168,7 @@ type SimpleTypeMapper struct { func newSimpleTypeMapper(source *Type, target *Type) *TypeMapper { m := &SimpleTypeMapper{} - m.kind = TypeMapperKindSimple + m.kind = TypeMapperKindSimple //nolint:customlint m.source = source m.target = target return &m.TypeMapper @@ -195,7 +195,7 @@ type ArrayTypeMapper struct { func newArrayTypeMapper(sources []*Type, targets []*Type) *TypeMapper { m := &ArrayTypeMapper{} - m.kind = TypeMapperKindArray + m.kind = TypeMapperKindArray //nolint:customlint m.sources = sources m.targets = targets return &m.TypeMapper @@ -224,7 +224,7 @@ type ArrayToSingleTypeMapper struct { func newArrayToSingleTypeMapper(sources []*Type, target *Type) *TypeMapper { m := &ArrayToSingleTypeMapper{} - m.kind = TypeMapperKindArrayToSingle + m.kind = TypeMapperKindArrayToSingle //nolint:customlint m.sources = sources m.target = target return &m.TypeMapper @@ -251,7 +251,7 @@ type DeferredTypeMapper struct { func newDeferredTypeMapper(sources []*Type, targets []func() *Type) *TypeMapper { m := &DeferredTypeMapper{} - m.kind = TypeMapperKindDeferred + m.kind = TypeMapperKindDeferred //nolint:customlint m.sources = sources m.targets = targets return &m.TypeMapper @@ -279,7 +279,7 @@ type FunctionTypeMapper struct { func newFunctionTypeMapper(fn func(*Type) *Type) *TypeMapper { m := &FunctionTypeMapper{} - m.kind = TypeMapperKindFunction + m.kind = TypeMapperKindFunction //nolint:customlint m.fn = fn return &m.TypeMapper } @@ -298,7 +298,7 @@ type MergedTypeMapper struct { func newMergedTypeMapper(m1 *TypeMapper, m2 *TypeMapper) *TypeMapper { m := &MergedTypeMapper{} - m.kind = TypeMapperKindMerged + m.kind = TypeMapperKindMerged //nolint:customlint m.m1 = m1 m.m2 = m2 return &m.TypeMapper @@ -319,7 +319,7 @@ type CompositeTypeMapper struct { func newCompositeTypeMapper(c *Checker, m1 *TypeMapper, m2 *TypeMapper) *TypeMapper { m := &CompositeTypeMapper{} - m.kind = TypeMapperKindComposite + m.kind = TypeMapperKindComposite //nolint:customlint m.c = c m.m1 = m1 m.m2 = m2 @@ -344,7 +344,7 @@ type InferenceTypeMapper struct { } func (c *Checker) initInferenceTypeMapper(m *InferenceTypeMapper, n *InferenceContext, fixing bool) *TypeMapper { - m.kind = TypeMapperKindInference + m.kind = TypeMapperKindInference //nolint:customlint m.c = c m.n = n m.fixing = fixing diff --git a/tsc/internal/checker/types.go b/tsc/internal/checker/types.go index a3cfe28c2e735..1ebb5b3fe752e 100644 --- a/tsc/internal/checker/types.go +++ b/tsc/internal/checker/types.go @@ -787,7 +787,7 @@ type Type struct { flags TypeFlags objectFlags ObjectFlags id TypeId - kind typeKind + kind typeKind `customlint:"protected"` symbol *ast.Symbol alias *TypeAlias checker *Checker From f4d9da17e9b2f153486134ee23b351335bff2d86 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:19:57 -0700 Subject: [PATCH 3/3] Catch range and elided-literal writes to protected fields Range clauses can assign a protected field directly, and an elided literal inside a []*T or map[K]*T composite initializes a fresh header just like &T{...} does. Both bypassed the write check. --- tools/customlint/protectedfield.go | 15 ++++++++++++- .../protectedfield/consumer/consumer.go | 12 ++++++++++ .../consumer/consumer.go.golden | 22 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tools/customlint/protectedfield.go b/tools/customlint/protectedfield.go index abd0cd9671b33..0d8e414b281fe 100644 --- a/tools/customlint/protectedfield.go +++ b/tools/customlint/protectedfield.go @@ -38,6 +38,7 @@ func (p *protectedFieldPass) run() (any, error) { for c := range in.Root().Preorder( (*ast.AssignStmt)(nil), (*ast.IncDecStmt)(nil), + (*ast.RangeStmt)(nil), (*ast.UnaryExpr)(nil), (*ast.CompositeLit)(nil), (*ast.FuncType)(nil), @@ -50,6 +51,13 @@ func (p *protectedFieldPass) run() (any, error) { } case *ast.IncDecStmt: p.checkWrite(node.X, "write to") + case *ast.RangeStmt: + if node.Key != nil { + p.checkWrite(node.Key, "write to") + } + if node.Value != nil { + p.checkWrite(node.Value, "write to") + } case *ast.UnaryExpr: if node.Op == token.AND { p.checkWrite(node.X, "address of") @@ -107,7 +115,12 @@ func (p *protectedFieldPass) checkWrite(expr ast.Expr, what string) { } func (p *protectedFieldPass) checkLiteral(lit *ast.CompositeLit) { - st, ok := p.pass.TypesInfo.TypeOf(lit).Underlying().(*types.Struct) + typ := p.pass.TypesInfo.TypeOf(lit) + // Elided literals in []*T or map[K]*T composites have type *T. + if ptr, ok := typ.Underlying().(*types.Pointer); ok { + typ = ptr.Elem() + } + st, ok := typ.Underlying().(*types.Struct) if !ok { return } diff --git a/tools/customlint/testdata/protectedfield/consumer/consumer.go b/tools/customlint/testdata/protectedfield/consumer/consumer.go index 49c22227903dd..2611813bbd0be 100644 --- a/tools/customlint/testdata/protectedfield/consumer/consumer.go +++ b/tools/customlint/testdata/protectedfield/consumer/consumer.go @@ -17,6 +17,14 @@ func writes(h *record.Header, s *record.Small, d *record.Defined) { h.Flags = 1 _ = &h.Flags _ = h.Kind + for _, h.Kind = range []record.Kind{record.KindLarge} { + } + for h.Kind = range record.Kind(2) { + } + var kinds []record.Kind + for _, kind := range kinds { + _ = kind + } } func literals() { @@ -25,6 +33,10 @@ func literals() { _ = &record.Header{Flags: 1} _ = &record.Small{} _ = &record.Alias{Kind: record.KindSmall} + _ = []*record.Header{{Kind: record.KindLarge}} + _ = []*record.Header{{record.KindLarge, 1}} + _ = map[int]*record.Header{0: {Kind: record.KindLarge}} + _ = []*record.Header{{Flags: 1}} } func copies(h *record.Header, s *record.Small, hs []record.Header) { diff --git a/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden b/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden index cd8db282a59c6..eba9bc857453c 100644 --- a/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden +++ b/tools/customlint/testdata/protectedfield/consumer/consumer.go.golden @@ -29,6 +29,18 @@ h.Flags = 1 _ = &h.Flags _ = h.Kind + for _, h.Kind = range []record.Kind{record.KindLarge} { + ~ +!!! protectedfield: write to protected field Kind + } + for h.Kind = range record.Kind(2) { + ~ +!!! protectedfield: write to protected field Kind + } + var kinds []record.Kind + for _, kind := range kinds { + _ = kind + } } func literals() { @@ -43,6 +55,16 @@ _ = &record.Alias{Kind: record.KindSmall} ~ !!! protectedfield: write to protected field Kind + _ = []*record.Header{{Kind: record.KindLarge}} + ~ +!!! protectedfield: write to protected field Kind + _ = []*record.Header{{record.KindLarge, 1}} + ~ +!!! protectedfield: write to protected field Kind + _ = map[int]*record.Header{0: {Kind: record.KindLarge}} + ~ +!!! protectedfield: write to protected field Kind + _ = []*record.Header{{Flags: 1}} } func copies(h *record.Header, s *record.Small, hs []record.Header) {