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

--noImplicitAny error not reported for variable declaration #30899

Closed
mjbvz opened this issue Apr 12, 2019 · 11 comments
Closed

--noImplicitAny error not reported for variable declaration #30899

mjbvz opened this issue Apr 12, 2019 · 11 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Apr 12, 2019

TypeScript Version: 3.5.0-dev.20190412

Search Terms:

Code
Compile the following ts with --noImplicitAny

let x;
if (Date.now()) {
    x = 1;
} else {
    x = { obj: true };
}

foo(x);

function foo(y) { }

Expected behavior:
The declaration let x should produce an implicit any error

Actual behavior:
No error reported. After the assignment, the type of x is number | { obj: boolean }

Playground Link:
https://www.typescriptlang.org/play/#src=let%20x%3B%0D%0Aif%20(Date.now())%20%7B%0D%0A%20%20%20%20x%20%3D%201%3B%0D%0A%7D%20else%20%7B%0D%0A%20%20%20%20x%20%3D%20%7B%20obj%3A%20true%20%7D%3B%0D%0A%7D%0D%0A%0D%0Afoo(x)%3B%0D%0A%0D%0Afunction%20foo(y)%20%7B%20%7D

Related Issues:
From microsoft/vscode#72193

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 12, 2019
@RyanCavanaugh
Copy link
Member

There is no any behavior displayed here. Writes to x are tracked, and if the read type doesn't match its use, an error is issued.

Intuitively, this is entirely consistent with you writing

let x = Date.now() ? 1 : { obj: true };

which is not an error either.

@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 12, 2019

That does not seem correct to me. The programmer can assign x to just about anything, which makes it the same as if they had written: let x: any:

let x;

for (const a of []) {
    x = 1;
}
if (Date.now()) {
    x = new Date();
}
switch (Date.now()) {
    case 1: x = ''; break
   case 2: x = () => {} break
}

foo(x);

function foo(y) { }

Type inference will make sure x has the correctly inferred type when it is used

Screen Shot 2019-04-12 at 3 21 35 PM

but the assignments themselves are the problem we are trying to catch

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 12, 2019

Why should that code have an error, but not this code? The problem here is foo accepting an any.

for (const a of []) {
    foo(1);
}
if (Date.now()) {
    foo(new Date());
}
switch (Date.now()) {
    case 1: foo(''); break
   case 2: foo(() => {}) break
}

function foo(y) { }

This is an introduced feature (#11263) we did on purpose to reduce spurious noImplicitAnys in cases where a variable was always introduced with a correct type. Add a lint rule if you want lets with no initializers to always have a type annotation - that is a style rule, not a correctness rule.

@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 12, 2019

Thanks for the link to the issue. I can better understand why this decision was made but still I'm not convinced from a language user perspective that this let x; should not be treated as an implicit any in cases where x is assigned to different types.

For example, thy should be an implicit any error:

function foo(x?) {
    if (Date.now()) {
        x = 'str';
    }
    return bar(x);
}

function bar(x: any) { }

while this is not:

let x;
if (Date.now()) {
    x = 'str';
}
bar(x)

function bar(x: any) { }

@RyanCavanaugh
Copy link
Member

Because one of them creates a usable any target site where no any was specified (foo({ asdf: 10 })), and the other doesn't.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 12, 2019

You should think of it in these terms: Was I allowed to access a property of an any-typed variable without writing : any ? If you didn't do that, you didn't observe an implicit any.

function foo(x?) {
    x.asdf; // yes
}

vs

let x;
x.asdf; // no
if (Date.now()) {
    x = 'str';
    x.asdf; // no
}
bar(x)
x.asdf; // no

function bar(x: any) {
  x.asdf; // I *did* write :any
}

@lallenlowe
Copy link

lallenlowe commented Nov 30, 2022

Consider an example like:

const parseError = (err: Error) => {
  let errData;
  try {
    errData = JSON.parse(err.message);
    // log errData
  } catch (e) {
    // log failure
  }
  if (errData['some-key'].includes('403')) {
    throw new Error('some error message');
  }
  throw err;
};

When we go to access errData['some-key'] when errData wasn't set, Node will throw the Error Cannot read properties of undefined (reading 'some-key')
errData is implicitly of type any here, which is why TS won't protect me from accessing properties on it even though it can clearly be undefined. I try really hard to watch out for these issues, but on a team with 10s of engineers they just keep making it into the codebase. Is there no way to get protection from this kind of "implicit any"?

@RyanCavanaugh
Copy link
Member

@lallenlowe you have a different problem than the one described here. errData isn't implicitly any, it's explicitly any due to the inference from JSON.parse's return type. You should write:

    errData = JSON.parse(err.message) as { [k: string]: string };

or similar

@lallenlowe
Copy link

lallenlowe commented Dec 1, 2022

@RyanCavanaugh OK, yeah, I see how it is different now. Of course I can cast that type, I just wish TS could enforce this because it is all over our repo now. I think the correct solution is probably that JSON.parse and the like should return type unknown instead of type any.

@tjx666
Copy link

tjx666 commented Jun 8, 2023

I also think it is strange, because the options noImplicitAny should work like it named: help find the any which is not explict set. I will try to to find or self implemtnt a eslint rule can report error for this

@tjx666
Copy link

tjx666 commented Jun 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants