enhance: Add tests for premature caching of contextual parameter types

Open Beginner friendly
#64,278 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go, typescript

Research direction

Start with contextuallyTypedParametersWithInitializers2.ts and the existing contextual parameter initializer cases, then run the associated compiler tests. Add the proposed test4 case and update its type and symbol baselines. Done means the test passes with the current caching guard and does not produce the spurious TS2322 diagnostic.

Written by the indexing model from the issue text.

Description

Possible Improvement
Acknowledgement
  • I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.
Comment

Add tests for premature caching of contextual parameter types

The checker currently handles the following code correctly.

But the existing cases in contextuallyTypedParametersWithInitializers2.ts do not appear to cover the following case: the combination of premature type caching and parameter whose type is widened during contextual checking.

Example:

It is ok:

const fn: (reader: () => number, value: 1) => void =
  (get = () => x, x = 0) => {
    x.toFixed();
  };

Why this needs regression coverage:

when caching the symbol type, we need to check if it is context sensitive, as the author intentionly said:

func (c *Checker) getTypeOfVariableOrParameterOrProperty(symbol *ast.Symbol) *Type {
	links := c.valueSymbolLinks.Get(symbol)
	if links.resolvedType == nil {
		t := c.getTypeOfVariableOrParameterOrPropertyWorker(symbol)
		if t == nil {
			panic("Unexpected nil type")
		}
		// For a contextually typed parameter it is possible that a type has already
		// been assigned (in assignTypeToParameterAndFixTypeParameters), and we want
		// to preserve this type. In fact, we need to _prefer_ that type, but it won't
		// be assigned until contextual typing is complete, so we need to defer in
		// cases where contextual typing may take place.
		if links.resolvedType == nil && !c.isParameterOfContextSensitiveSignature(symbol) {
			links.resolvedType = t
		}
		return t
	}
	return links.resolvedType
}

Checking get = () => x queries x before its contextual parameter checking is complete:

  1. The initial query get the contextual type 1.
  2. Later, checking the default value 0 and widens the parameter's type to number.
  3. If the initial result 1 is cached, assignParameterType returns early and cannot assign the final type.

This would incorrectly produce the error:

Type '0' is not assignable to type '1'.

The existing guard in getTypeOfVariableOrParameterOrProperty prevents this premature caching:

!c.isParameterOfContextSensitiveSignature(symbol)

The example passes with the current implementation. Removing the guard will produce the incorrect TS2322 diagnostic.

Proposed change:

Just add this case to contextuallyTypedParametersWithInitializers2.ts

const test4: (reader: () => number, value: 1) => void =
  (get = () => x, x = 0) => {
    x.toFixed();
  };

Add this case to the contextual parameter initializer tests, together with type and symbol baselines, to protect the existing behavior against regressions.

Existing tests cover the caching guard’s effect on implicit any diagnostics. This test adds coverage of type widening for parameters with default initializers, ensuring that prematurely cached literal types do not cause spurious errors.

Dominant language
Go
Stars
111k
Forks
14.4k
Avg merge
1d 15h
Merged PRs (30d)
106

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/TypeScript

All issues in microsoft/TypeScript

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.