Skip to content
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

TS7022 (circular reference) on simple assignments in a loop #59074

Closed
Ghabriel opened this issue Jun 28, 2024 · 6 comments
Closed

TS7022 (circular reference) on simple assignments in a loop #59074

Ghabriel opened this issue Jun 28, 2024 · 6 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@Ghabriel
Copy link

🔎 Search Terms

TS7022, implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer, circular reference, circular type

🕗 Version & Regression Information

  • This is the behavior in every version (3.3.3+) I tried, and I reviewed the FAQ for entries about circular references

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.2#code/CYUwxgNghgTiAEYD2A7AzgF3hAlikAXPJjHgOYDcAUBCFhiJkSefAD7woCuEE8AvJx4RqVAGZIY8ABS0sOAfAAMFeAoA82PCAB0tFGQwALVTgDUZgJTwA3lXiJUmREdiLc+ANo4AutQc4YjIMzvxhLrDWNgC+9o7oWFAQaEgAwq5SgmAZ-vAhWIJJKemw1LFAA

💻 Code

declare const line: string;
let test: string | null = null;

for (let i = 0; i < line.length; i++) {
  const char = line[i];
  if (test === char) {}
  const alsoChar = char;
  test = alsoChar;
}

🙁 Actual behavior

The following error is triggered:
'alsoChar' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.(7022)

🙂 Expected behavior

No errors. char is clearly a string and alsoChar is just a copy of it, so there shouldn't be any type confusion and/or circularity. test isn't even read from, except in the empty if statement.

Additional information about the issue

  • Removing the empty if on line 6 fixes the error.
  • Replacing the if on line 6 by if (test !== char) {} also produces the error, but if (test! < char) {} doesn't.
  • Replacing line 2 by let test: string | undefined = undefined; also produces the error, but let test: string | undefined; doesn't.

This version also reproduces the issue:

declare const line: string;
let test: boolean | null = null;

for (let i = 0; i < line.length; i++) {
  const char = line[i];
  if (test !== char) {}
  const alsoChar = char === ""; // <- using a simple comparison
  test = alsoChar;
}

Same for this one:

declare const line: string;
let test: string | null = null;

for (let i = 0; i < line.length; i++) {
  const char = line[i];
  if (test !== char) {}
  const alsoChar = char[0]; // <- changing char to char[0]
  test = alsoChar;
}

But this one doesn't:

declare const line: string;
let test: string | null = null;

for (let i = 0; i < line.length; i++) {
  const char = line[i];
  if (test !== char) {}
  const alsoChar = char.charAt(0); // <- char.charAt(0) instead of char[0]
  test = alsoChar;
}

Another interesting point is that despite the error, the tooltip does show const alsoChar: string as expected. This is recent, though: v5.3.3 shows the type as any, whereas v5.4.5 shows it as string.

@JoostK
Copy link
Contributor

JoostK commented Jun 29, 2024

Please see #33191 and #42529. The behavior change in 5.4 is likely attributed to #56753. It is indeed somewhat interesting that the tooltip does show a type other than any.

@Andarist
Copy link
Contributor

I'm fixing the weird inconsistent quick info display here: #59075

@Ghabriel
Copy link
Author

Please see #33191 and #42529. The behavior change in 5.4 is likely attributed to #56753. It is indeed somewhat interesting that the tooltip does show a type other than any.

Those issues look similar but I'm not sure the root cause is the same. The supposed circularity in my repro code is being caused by an empty if statement which is essentially dead code here. I assume it's narrowing something internally but given that it's not affecting the control flow at all (can be removed with zero consequences to the runtime), I don't see a good reason for this narrowing to be occurring. (Well, in the code that sparked the issue the if statement is not exactly dead code, but it's still unrelated to the variable that's getting the error.)

I'm fixing the weird inconsistent quick info display here: #59075

Doesn't that make it worse, though? The problem is in the extraneous error here, the tooltip has the correct type...

@Andarist
Copy link
Contributor

The supposed circularity in my repro code is being caused by an empty if statement which is essentially dead code here

It requests type of test which in turn depends on the type of alsoChat. That's how the circularity happens here. I agree that it's weird that some very similar cases work without running into circularity.

Doesn't that make it worse, though? The problem is in the extraneous error here, the tooltip has the correct type...

The tooltip has the correct type somewhat accidentally here. CheckMode.TypeOnly was introduced to suppress comparability errors when dealing with intermediate types created during control flow analysis. Those errors that were suppressed by that PR didn't affect the control flow analysis at all. So it was OK to suppress them.

An encountered circularity returns any type and that very much can propagate through the types computed during control flow analysis. That's why I think it wouldn't be good to suppress this error here in a similar way.

That said, this alternative patch doesn't change anything in the existing test suite and it "fixes" your particular circularity issue:

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index a965c51d22..640a0216c5 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -12055,6 +12055,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             if (isBindingElement(declaration) && checkMode === CheckMode.Contextual) {
                 return errorType;
             }
+            if (checkMode && checkMode & CheckMode.TypeOnly) {
+                return anyType;
+            }
             return reportCircularityError(symbol);
         }
         let type: Type;
@@ -12137,6 +12140,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             if (isBindingElement(declaration) && checkMode === CheckMode.Contextual) {
                 return type;
             }
+            if (checkMode && checkMode & CheckMode.TypeOnly) {
+                return anyType;
+            }
             return reportCircularityError(symbol);
         }
         return type;

But despite that, I think it's rather risky to allow this to propagate freely because the result propagates. Similarly, getTypeOfExpression sets NodeFlags.TypeCached on the node but in this scenario the type wouldn't actually be cached! That's just a different bug waiting to happen somewhere.

I also checked that it wouldn't have a perceivable bad impact on this scenario with an auto type:

declare const line: string;
let test;

for (let i = 0; i < line.length; i++) {
  const char = line[i];
  if (test === char) {}
  const alsoChar = char;
  test = alsoChar;
}

FWIW, I'm not saying that the circularity avoidance couldn't be improved for some of those cases. I feel like the change I proposed is good regardless of that.

@RyanCavanaugh
Copy link
Member

#56753 is being superceded by #59183. This is a true circularity for basically the same reason as #33191 (comment).

See also https://github.com/Microsoft/TypeScript/wiki/FAQ#circularity-errors-may-occur-in-the-presence-of-circularities

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Jul 9, 2024
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

5 participants