Skip to content

fix(1196): extend parameter matching with index fallback when name matching fails #1241

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
45 changes: 34 additions & 11 deletions internal/parser/reparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ func (p *Parser) reparseJSDocSignature(jsSignature *ast.Node, fun *ast.Node, jsD
}

func (p *Parser) reparseJSDocTypeLiteral(t *ast.TypeNode, host *ast.Node) *ast.Node {
if t != nil && t.Kind == ast.KindJSDocTypeLiteral {
if t == nil {
return nil
}
if t.Kind == ast.KindJSDocTypeLiteral {
isArrayType := t.AsJSDocTypeLiteral().IsArrayType
properties := p.nodeSlicePool.NewSlice(0)
for _, prop := range t.AsJSDocTypeLiteral().JSDocPropertyTags {
jsprop := prop.AsJSDocParameterOrPropertyTag()
Expand All @@ -180,7 +184,9 @@ func (p *Parser) reparseJSDocTypeLiteral(t *ast.TypeNode, host *ast.Node) *ast.N
name = name.AsQualifiedName().Right
}
property := p.factory.NewPropertySignatureDeclaration(nil, name, p.makeQuestionIfOptional(jsprop), nil, nil)
property.AsPropertySignatureDeclaration().Type = p.reparseJSDocTypeLiteral(jsprop.TypeExpression, property)
if jsprop.TypeExpression != nil {
property.AsPropertySignatureDeclaration().Type = p.reparseJSDocTypeLiteral(jsprop.TypeExpression.Type(), property)
}
property.Loc = prop.Loc
property.Flags = p.contextFlags | ast.NodeFlagsReparsed
properties = append(properties, property)
Expand All @@ -189,6 +195,11 @@ func (p *Parser) reparseJSDocTypeLiteral(t *ast.TypeNode, host *ast.Node) *ast.N
t = p.factory.NewTypeLiteralNode(p.newNodeList(loc, properties))
t.Loc = loc
t.Flags = p.contextFlags | ast.NodeFlagsReparsed
if isArrayType {
t = p.factory.NewArrayTypeNode(t)
t.Flags = p.contextFlags | ast.NodeFlagsReparsed
t.Loc = loc
}
}
return t
}
Expand Down Expand Up @@ -327,13 +338,17 @@ func (p *Parser) reparseHosted(tag *ast.Node, parent *ast.Node, jsDoc *ast.Node)
}
case ast.KindJSDocParameterTag:
if fun, ok := getFunctionLikeHost(parent); ok {
jsparam := tag.AsJSDocParameterOrPropertyTag()
if param, ok := findMatchingParameter(fun, jsparam); ok {
parameterTag := tag.AsJSDocParameterOrPropertyTag()
if param, ok := findMatchingParameter(fun, parameterTag, jsDoc); ok {
if param.Type == nil {
param.Type = setHost(jsparam.TypeExpression, param.AsNode())
paramType := setHost(parameterTag.TypeExpression, param.AsNode())
if parameterTag.IsNameFirst && parameterTag.TypeExpression != nil && parameterTag.TypeExpression.Type().Kind == ast.KindJSDocTypeLiteral {
paramType = p.reparseJSDocTypeLiteral(parameterTag.TypeExpression.Type(), param.AsNode())
}
param.AsParameterDeclaration().Type = paramType
}
if param.QuestionToken == nil && param.Initializer == nil {
if question := p.makeQuestionIfOptional(jsparam); question != nil {
if question := p.makeQuestionIfOptional(parameterTag); question != nil {
param.QuestionToken = question
}
}
Expand Down Expand Up @@ -461,11 +476,19 @@ func (p *Parser) makeQuestionIfOptional(parameter *ast.JSDocParameterTag) *ast.N
return questionToken
}

func findMatchingParameter(fun *ast.Node, tag *ast.JSDocParameterTag) (*ast.ParameterDeclaration, bool) {
for _, parameter := range fun.Parameters() {
if parameter.Name().Kind == ast.KindIdentifier && tag.Name().Kind == ast.KindIdentifier &&
parameter.Name().Text() == tag.Name().Text() {
return parameter.AsParameterDeclaration(), true
func findMatchingParameter(fun *ast.Node, tag *ast.JSDocParameterTag, jsDoc *ast.Node) (*ast.ParameterDeclaration, bool) {
tagIndex := core.FindIndex(jsDoc.AsJSDoc().Tags.Nodes, func(n *ast.Node) bool {
return n.Kind == ast.KindJSDocParameterTag && n.AsJSDocParameterOrPropertyTag() == tag
})
for parameterIndex, parameter := range fun.Parameters() {
if parameter.Name().Kind == ast.KindIdentifier {
if tag.Name().Kind == ast.KindIdentifier && parameter.Name().Text() == tag.Name().Text() {
return parameter.AsParameterDeclaration(), true
}
} else {
if parameterIndex == tagIndex {
return parameter.AsParameterDeclaration(), true
}
}
}
return nil, false
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/jsdoc/jsdocDestructuringParameterDeclaration.ts] ////

//// [a.js]
/**
* @param {{ a: number; b: string }} args
*/
function f({ a, b }) {}




//// [a.d.ts]
/**
* @param {{ a: number; b: string }} args
*/
declare function f({ a, b }: {
a: number;
b: string;
}): void;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//// [tests/cases/conformance/jsdoc/jsdocDestructuringParameterDeclaration.ts] ////

=== /a.js ===
/**
* @param {{ a: number; b: string }} args
*/
function f({ a, b }) {}
>f : Symbol(f, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 3, 12))
>b : Symbol(b, Decl(a.js, 3, 15))

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//// [tests/cases/conformance/jsdoc/jsdocDestructuringParameterDeclaration.ts] ////

=== /a.js ===
/**
* @param {{ a: number; b: string }} args
*/
function f({ a, b }) {}
>f : ({ a, b }: { a: number; b: string; }) => void
>a : number
>b : string

Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const a = 1;
/** @type {T} */
const b = {
>b : T
>{ await: false,} : { await: boolean; }
>{ await: false,} : { await: false; }

await: false,
>await : boolean
>await : false
>false : false

};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@

/** @type {Foo} */
export default {
>{ a: 'a', b: 'b'} : { a: string; b: string; }
>{ a: 'a', b: 'b'} : { a: string; b: "b"; }

a: 'a',
>a : string
>'a' : "a"

b: 'b'
>b : string
>b : "b"
>'b' : "b"
}

Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ export declare class Foo {
*
* @param {{ prop: string }} baz Baz.
*/
set bar({}: {});
set bar({}: {
prop: string;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,4 @@
+export declare class Foo {
/**
* Bar.
*
* @param {{ prop: string }} baz Baz.
*/
- set bar({}: {
- prop: string;
- });
+ set bar({}: {});
}
*
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ export class Foo {
* @param {{ prop: string }} baz Baz.
*/
set bar({}) {}
>bar : any
>bar : { prop: string; }
}

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ export declare class Foo {
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop }: {
prop?: string;
prop: string | undefined;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,4 @@
+export declare class Foo {
/**
* Bar.
*
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop }: {
- prop: string | undefined;
+ prop?: string;
});
}
*
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export class Foo {
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop = 'foo' }) {}
>bar : any
>bar : { prop: string; }
>prop : string
>'foo' : "foo"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ class X extends Test {
>super : typeof Test

if (options.test) {
>options.test : any
>options.test : typeof import("./Test.js").default | undefined
>options : Options
>test : any
>test : typeof import("./Test.js").default | undefined

this.test = new options.test();
>this.test = new options.test() : any
>this.test = new options.test() : import("./Test.js").default
>this.test : any
>this : this
>test : any
>new options.test() : any
>options.test : any
>new options.test() : import("./Test.js").default
>options.test : typeof import("./Test.js").default
>options : Options
>test : any
>test : typeof import("./Test.js").default
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/a.js(3,15): error TS2552: Cannot find name 'sting'. Did you mean 'string'?


==== /a.js (1 errors) ====
/**
* @typedef MyType
* @property {sting} [x]
~~~~~
!!! error TS2552: Cannot find name 'sting'. Did you mean 'string'?
*/

/** @param {MyType} p */
export function f(p) { }

==== /b.js (0 errors) ====
import { f } from "./a.js"
f({ x: 42 })

Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
a.js(7,7): error TS2375: Type '{ value: undefined; }' is not assignable to type 'A' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
Types of property 'value' are incompatible.
Type 'undefined' is not assignable to type 'number'.
a.js(14,7): error TS2375: Type '{ value: undefined; }' is not assignable to type '{ value?: number; }' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
Types of property 'value' are incompatible.
Type 'undefined' is not assignable to type 'number'.


==== a.js (1 errors) ====
==== a.js (2 errors) ====
/**
* @typedef {object} A
* @property {number} [value]
*/

/** @type {A} */
const a = { value: undefined }; // error
~
!!! error TS2375: Type '{ value: undefined; }' is not assignable to type 'A' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
!!! error TS2375: Types of property 'value' are incompatible.
!!! error TS2375: Type 'undefined' is not assignable to type 'number'.

/**
* @typedef {{ value?: number }} B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ const x = /** @type {Foo} */ ({});
>{} : {}

x.foo; // number | undefined
>x.foo : any
>x.foo : number | undefined
>x : Foo
>foo : any
>foo : number | undefined

const y = /** @type {Required<Foo>} */ ({});
>y : Required<Foo>
>({}) : Required<Foo>
>{} : {}

y.foo; // number
>y.foo : any
>y.foo : number
>y : Required<Foo>
>foo : any
>foo : number

Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ const t3 = /** @satisfies {T1} */ ({});
/** @type {T2} */
const t4 = /** @satisfies {T2} */ ({ a: "a" });
>t4 : T2
>({ a: "a" }) : { a: string; }
>{ a: "a" } : { a: string; }
>a : string
>({ a: "a" }) : { a: "a"; }
>{ a: "a" } : { a: "a"; }
>a : "a"
>"a" : "a"

/** @type {(m: string) => string} */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ function foo(opts) {
>opts : Opts

opts.x;
>opts.x : any
>opts.x : string
>opts : Opts
>x : any
>x : string
}

foo({x: 'abc'});
Expand All @@ -40,9 +40,9 @@ function foo1(opts) {
>opts : AnotherOpts

opts.anotherX;
>opts.anotherX : any
>opts.anotherX : string
>opts : AnotherOpts
>anotherX : any
>anotherX : string
}

foo1({anotherX: "world"});
Expand All @@ -66,9 +66,9 @@ function foo2(opts) {
>opts : Opts1

opts.x;
>opts.x : any
>opts.x : string
>opts : Opts1
>x : any
>x : string
}
foo2({x: 'abc'});
>foo2({x: 'abc'}) : void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function prepareConfig({
>prepareConfig : Symbol(prepareConfig, Decl(index.js, 0, 0))

additionalFiles: {
>additionalFiles : Symbol(additionalFiles)
>additionalFiles : Symbol(additionalFiles, Decl(index.js, 2, 3))

json = []
>json : Symbol(json, Decl(index.js, 5, 22))
Expand Down Expand Up @@ -60,7 +60,7 @@ export const prepareConfigWithContextualSignature = ({
*/
function f1({ a: { json = [] } = {} } = {}) { return json }
>f1 : Symbol(f1, Decl(index.js, 29, 1))
>a : Symbol(a)
>a : Symbol(a, Decl(index.js, 34, 12))
>json : Symbol(json, Decl(index.js, 36, 18))
>json : Symbol(json, Decl(index.js, 36, 18))

Expand Down
Loading