TypeScript Language Server False Positive Error Reporting

Does this issue occur when all extensions are disabled?: Yes

VS Code Version: 1.87.0
OS Version: Ubuntu 22.04

Steps to Reproduce:

Clone the project git clone https://github.com/DScheglov/res-test.git
Open the VS Code: code res-test
Install dependencies (in terminal): npm i
Open file ./src/index.ts

The following errors are shown:

These errors are false positive:

project is configured to use the same typescript compiler as a project. See .vscode/settings.json
project is compiled successfully via terminal: npx tsc (no errors)
errors are gone after TS Language Server is restared (Ctrl + Shift + P , TypeScript: Restart TS Server)
errors go back after any touch of the file (just add a space).

There is not such problem in WebStorm

(Sorry, I have to check)

Errors are not displayed in the TS Playground as well:
Playground

Minimized repro

export interface Result<T, E> {
  mapErr<F>(fn: (error: E) => F): Result<T, F>;
  [Symbol.iterator](): Generator<E, T>;
}

declare const okIfObject: (value: unknown) => Result<Record<string, unknown>, 'ERR_NOT_AN_OBJECT'>;
declare const okIfInt: (value: unknown) => Result<number, 'ERR_NOT_AN_INT'>;
export declare function Do2<T, E>(job: () => Generator<E, T>): void;

declare let value: unknown;
Do2(function* () {
  const object = yield* okIfObject(value).mapErr(
    error => 0
  );

  const age = yield* okIfInt(object.age).mapErr(
    error => 0
  );

  return { age };
});

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh)

I've met similar error from eslint:

Shoud I register the bug in @typescript-eslint? Or there it will be fixed after this one?

I'm not sure how their rule works. If it depends on type information (looks like it does) then that error should go away after we fix this one.

Analyzing Ryan's simplified repro the problem comes from encodedSemanticClassifications-full that is called before semantic diagnostics. It breaks the regular order of type checking since while collecting tokens it calls reclassifyByType on object's declaration. That reads its type and to do that it has to resolve the type of the yield expression. That is a function call that has to be inferred (speaking about .mapErr here) and that depends on the contextual type of the yield operand, that in turn requires the return type of the containing function to be computed which leads to computing type of the return expression and that depends on... object's type. That's how the circularity arises.

Failing test case for this situation:

/// <reference path='fourslash.ts'/>

// @strict: true
// @target: esnext
// @lib: esnext

//// export interface Result<T, E> {
////   mapErr<F>(fn: (error: E) => F): Result<T, F>;
////   [Symbol.iterator](): Generator<E, T>;
//// }
////
//// declare const okIfObject: (
////   value: unknown,
//// ) => Result<Record<string, unknown>, "ERR_NOT_AN_OBJECT">;
////
//// declare const okIfInt: (value: unknown) => Result<number, "ERR_NOT_AN_INT">;
////
//// export declare function Do2<T, E>(job: () => Generator<E, T>): void;
////
//// declare let value: unknown;
////
//// Do2(function* () {
////   const object = yield* okIfObject(value).mapErr((error) => 0);
////   const age = yield* okIfInt(object.age).mapErr((error) => 0);
////   return { age };
//// });

verify.encodedSemanticClassificationsLength('2020', 132);
verify.getSemanticDiagnostics([]);

I spent some time on this to learn what the proper fix could be. I concluded that the only proper fix for this has to be about making the cycle resolution logic smarter - or for the "out of order" type resolution to alter its behavior somehow.

No extra flags passed down by the caller can fix this because the compiler doesn't always control the caller. For instance, I managed to somewhat improve the situation locally for the test case above but that didn't fix quick info problem at the very same location:

/// <reference path="fourslash.ts" />

// @strict: true
// @target: esnext
// @lib: esnext

//// export interface Result<T, E> {
////   mapErr<F>(fn: (error: E) => F): Result<T, F>;
////   [Symbol.iterator](): Generator<E, T>;
//// }
////
//// declare const okIfObject: (
////   value: unknown,
//// ) => Result<Record<string, unknown>, "ERR_NOT_AN_OBJECT">;
////
//// declare const okIfInt: (value: unknown) => Result<number, "ERR_NOT_AN_INT">;
////
//// export declare function Do2<T, E>(job: () => Generator<E, T>): void;
////
//// declare let value: unknown;
////
//// Do2(function* () {
////   const object/*1*/ = yield* okIfObject(value).mapErr((error) => 0);
////   const age = yield* okIfInt(object.age).mapErr((error) => 0);
////   return { age };
//// });

verify.quickInfoAt("1", "const object: Record<string, unknown>");

One of those calls checker.getTypeAtLocation while the other one calls checker.getTypeOfSymbolAtLocation. The problem with any attempt to fix this by passing extra information through callers/links/whatever is that it won't fix raw API calls like this: checker.getTypeOfSymbol(checker.getSymbolAtLocation(node)).

Perhaps there is a way to fix this by overriding resolutionStart at some moment conditionally but I don't know what exact moment that would be or what the condition guarding this should be 😉 A part of the idea could be that outer inference should always start before type resolution for a node nested in it under regular circumstances - maybe this fact can be leveraged to manipulate the resolution arrays or the resolutionStart variable

Additional repro noted at #57429 (comment)

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh)
I've found one more bug in TypeScript, but I'm not sure if it is related to this one. Could you please check:
#57903

Unfortunately it seems like it doesn't fix the issue, rather it does but it makes it appear in a different place, the return of the generator

It does appear to be the case only if there is no specified return, adding a return keyword makes the issue disappear

Interestingly, a modified #57585 (comment) to omit the return like the above is also fixed by my hack, which implies that the error being produced above is a cycle caused by some other query than semantic tokens. tsserver trace logs ("typescript.tsserver.log": "verbose") would be interesting here to figure out what path happened to be turned into a test case.

the full code to repro this is:

import { Effect } from "effect"

Effect.gen(function* () {
  const a = yield* Effect.succeed(0)
  const b = yield* Effect.succeed(1)
})

the bug presents while you edit, like adding a new line, making changes in general

Not sure if helpful but you can find the log from tsserver here: https://gist.github.com/mikearnaldi/fc2c4f15135b034d62f0625de82557f4

Yeah, that log's really long (the error doesn't happen until message 1020), so would need something shorter. That and you're using a language service plugin, which we generally treat as "voiding all warranties" when it comes to us trying to figure out what's wrong. But I don't think it matters in this case (but do try without it).

I can't get that example to reproduce myself, though, only with the return present...

Mateusz Burzyński (@Andarist)

Much shorter log without the plugin: https://gist.github.com/mikearnaldi/70037e879777f1f3243d0496559c2a99

To reproduce delete the second line:

const b = yield* Effect.succeed(1)

and re-type it manually, error should appear.

I wonder if I can use replay.io to record a session here cc Mateusz Burzyński (@Andarist)

That log looks truncated; the main reason for the log is to find the steps to get to the bad error, so we need steps from the first message all the way to the error to be able to write an equivalent test which touches everything in the right way. (Hence why a full log and a short one is helpful.)

I still wasn't able to make it reproduce when typing it out by hand, though... It's possible my editor did not make the same sequence of calls, of course.

Try a few times to delete and retype, it's sporadic, doesn't happen always. Will provide a full log tomorrow

Is there a strong need to provide an extra repro for this if we already have 3 compiler tests written for this here? ;p

I figured that would be helpful given I fixed the cases that looked effect-y, but it didn't help in the real world? That to me implied that there are still more cases missing, but sorry for the noise.

I working on a fix for this, I'll put something up later this afternoon.

Navigation Menu

Repository navigation

TypeScript Language Server False Positive Error Reporting #57585

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

TypeScript Language Server False Positive Error Reporting #57585

Description

Activity

RyanCavanaugh commented on Feb 29, 2024

DScheglov commented on Mar 1, 2024

RyanCavanaugh commented on Mar 1, 2024

Andarist commented on Mar 3, 2024

Andarist commented on Mar 3, 2024

RyanCavanaugh commented on Mar 6, 2024

DScheglov commented on Mar 22, 2024

7 remaining items

mikearnaldi commented on Apr 26, 2024

jakebailey commented on Apr 26, 2024

mikearnaldi commented on Apr 26, 2024

mikearnaldi commented on Apr 26, 2024

jakebailey commented on Apr 26, 2024

mikearnaldi commented on Apr 26, 2024

jakebailey commented on Apr 26, 2024

mikearnaldi commented on Apr 26, 2024

Andarist commented on Apr 26, 2024

jakebailey commented on Apr 26, 2024

ahejlsberg commented on Apr 26, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions