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

False positive on `no-undef` with mixed JS & TS codebase #416

Closed
tchakabam opened this Issue Dec 19, 2017 · 20 comments

Comments

Projects
None yet
@tchakabam
Copy link

tchakabam commented Dec 19, 2017

What version of TypeScript are you using?

2.6.1

What version of typescript-eslint-parser are you using?

11.0.0

What code were you trying to parse?

export type SomeThing = {
    id: string;
}

What did you expect to happen?

No lint error

What happened?

False positive on no-undef eslint core rule.

It basically means we can not use type definitions without disabling that rule (globablly or locally)

This has already been discussed in #77

It really becomes tricky when the project mixes JS and TS code.

But we haven't found any viable solution yet, apart applying different configuration to the various file types.

It would be nice if the eslint core rules would be overridable by the Typescript parser usage somehow I guess.

BUT this is really only an issue for mixed JS/TS codebases, because:

let's keep in mind this is easily fixable if your code is Typescript only,
since you can boldly disable this core rule as the TS compiler should complain anyway about a
missing definition of something :)
It really only becomes an issue when you want to take care of JS code using the same lint config.

@eslint eslint bot added the triage label Dec 19, 2017

@tchakabam tchakabam changed the title False positive on `no-undef` with Mixed JS & TS codebase False positive on `no-undef` with mixed JS & TS codebase Dec 19, 2017

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Dec 19, 2017

problem also appears on enumarations like here:

export enum Events {
    FOO: 'bar'
}

Error: 'FOO' is not defined

and Error: 'Events' is not defined

@StoneCypher

This comment has been minimized.

Copy link

StoneCypher commented Feb 4, 2018

Also happening here. Basically everything in this gets flagged no-undef:

type item = string;
type row  = item[];
type doc  = row[];



enum header_mode {

  none   = 'none',
  index  = 'index',
  number = 'number',
  first  = 'first'

}



enum quoting_circumstance {

  minimal        = 'minimal',
  always         = 'always',
  except_numbers = 'except_numbers',
  strict_nl      = 'strict_nl'

}



interface stringify_options {

  headers?                : row | false,
  quoter?                 : (s: string) => string,
  field_separator?        : string,
  row_separator?          : string,
  trailing_row_separator? : boolean

// , consumer: function
}



export { item, row, doc, header_mode, quoting_circumstance, stringify_options };

As so:


C:\Users\john\projects\csv_4180\src\ts\csv_types.ts
8:6 error 'header_mode' is not defined no-undef
10:3 error 'None' is not defined no-undef
11:3 error 'Index' is not defined no-undef
13:3 error 'First' is not defined no-undef
19:6 error 'quoting_circumstance' is not defined no-undef
21:3 error 'minimal' is not defined no-undef
22:3 error 'always' is not defined no-undef
23:3 error 'except_numbers' is not defined no-undef
24:3 error 'strict_nl' is not defined no-undef
30:11 error 'stringify_options' is not defined no-undef
32:3 error 'headers' is not defined no-undef
33:3 error 'quoter' is not defined no-undef
33:30 error 's' is not defined no-undef
34:3 error 'field_separator' is not defined no-undef
35:3 error 'row_separator' is not defined no-undef
36:3 error 'trailing_row_separator' is not defined no-undef
43:26 error 'header_mode' is not defined no-undef
43:39 error 'quoting_circumstance' is not defined no-undef
43:61 error 'stringify_options' is not defined no-undef

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Feb 4, 2018

Let's keep in mind this is easily fixable if your code is Typescript only, since you can boldly disable this core rule as the TS compiler should complain anyway about a missing definition of something :)

It really only becomes an issue when you want to take care of JS code using the same lint config.

@StoneCypher

This comment has been minimized.

Copy link

StoneCypher commented Feb 5, 2018

unless you want the actual protection afforded by the rule, which is significant

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Feb 5, 2018

hm, i must be missing something then :) what missing protection do you mean? when compiling actual typescript, the compiler will fail on accessing any symbol hat is "not defined" in the ecmascript sense :) so that protection from undefined symbols is basically ensured for any typescript code. the problem only occurs for code that is plain JavaScript (which the ts compiler also can process, but it will not apply the typescript compilation rules to it).

for example, in a proper typescript file, if you try to assign a variable like this: foobar = 4 and the symbol doesn't exist in any way in the respective scope (and which would also lead eslint to flag it with a no-undef error), then the Typescript compiler will also fail anyway. It will just look like this in your output:

TS2304: Cannot find name 'foobar'.

... but maybe i am just completely missing what you're trying to say or confused about something?

@corbinu

This comment has been minimized.

Copy link

corbinu commented Feb 5, 2018

@StoneCypher You can run eslint once against all .js files in your project and then again with the TS parser against all .ts files

Also the TS compiler also catches undefined in JS files not just TS files

@gpoitch

This comment has been minimized.

Copy link

gpoitch commented Feb 5, 2018

Running this eslint config against a mixed codebase with success:

module.exports = {
  rules: {
    // ... your js rules ...
  },

  overrides: {
    files: ['**/*.ts'],
    parser: 'typescript-eslint-parser',
    rules: {
      'no-undef': 'off'
    }
  }
}
@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Feb 5, 2018

that's a very nice workaround 👍

@davidfurlong

This comment has been minimized.

Copy link

davidfurlong commented Jul 20, 2018

overrides: { files: ['**/*.ts'], parser: 'typescript-eslint-parser', rules: { 'no-undef': 'off' } }

Worth mentioning that overrides expects an array now

@StoneCypher

This comment has been minimized.

Copy link

StoneCypher commented Jul 20, 2018

for the record that "workaround" just means you can't inherit any inferred type knowledge from js into ts

@lvivski

This comment has been minimized.

Copy link

lvivski commented Aug 2, 2018

We're running eslint with typescript-eslint-parser on a mixed codebase (JS/TS) and get false positives on class properties in JS classes. Not sure if that's the fault of this parser or maybe typescript compiler gives some weird AST for javascript files it analyzes. Disabling the rule is not an option, since it will stop throwing errors when you actually trying to use something that is not defined

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Aug 20, 2018

@lvivski Yes. But Typescript compiler will already complain if you use something that is not defined, so the lint rule shouldn't be needed anyway :)

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Aug 20, 2018

@StoneCypher Can you give an example of what you mean?

@christophehurpeau

This comment has been minimized.

Copy link

christophehurpeau commented Aug 20, 2018

@tchakabam When we use babel as a compiler, it doesn't complain like typescript compiler would :/ So this rule is still usefull

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Aug 20, 2018

@christophehurpeau Ok. What would be the reason for using this plugin but compiling with Babel?

EDIT: It was completely new to me that Babel has a preset for Typescript. If it doesn't "complain" like the Typescript compiler does, what does it actually do? :)

@j-f1

This comment has been minimized.

Copy link
Contributor

j-f1 commented Aug 20, 2018

It just deletes the TS syntax.

@christophehurpeau

This comment has been minimized.

Copy link

christophehurpeau commented Aug 20, 2018

I use a lot of babel plugins like some of babel-minify or babel-plugin-discard-module-references, so I can't build with typescript.
Howerver I should probably add another linting step and call typescript without emitting anything

@tchakabam

This comment has been minimized.

Copy link

tchakabam commented Aug 20, 2018

Sure, I have seen how one can get very integrated with Babel :) A lot of the stuff those plugins do can be done by bare webpack plugins btw, which are not tied to one or another compiler. Just in case you seek to break out.

You probably should at least use it as a linter, like you propose. Writing typescript code without actually running the compiler is ... ? :) Stays that with an editor like VS Code you still get the whole Typescript-compile messages/warnings and intellisense. Is that what your current use-case is?

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Aug 21, 2018

It looks like this issue has been resolved and the discussion has gotten off topic. Can we close this now or is there something else being proposed?

@amcdnl

This comment has been minimized.

Copy link

amcdnl commented Aug 29, 2018

@kaicataldo - I'm running Babel7 and TypeScript plugin and getting this :(.

    "@babel/core": "^7.0.0",
    "@babel/plugin-proposal-class-properties": "^7.0.0",
    "@babel/plugin-proposal-decorators": "^7.0.0",
    "@babel/plugin-proposal-object-rest-spread": "^7.0.0",
    "@babel/plugin-syntax-dynamic-import": "^7.0.0",
    "@babel/plugin-transform-runtime": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/preset-react": "^7.0.0",
    "@babel/preset-typescript": "^7.0.0",
    "babel-core": "^7.0.0-0",
    "babel-jest": "^23.4.2",
    "babel-loader": "^8.0.0",
    "typescript-eslint-parser": "^18.0.0",
    "eslint": "^5.4.0",
    "eslint-config-prettier": "^3.0.1",
    "eslint-config-standard": "^12.0.0",
    "eslint-loader": "^2.1.0",
    "eslint-plugin-graphql": "^2.1.1",
    "eslint-plugin-import": "^2.14.0",
    "eslint-plugin-jest": "^21.22.0",
    "eslint-plugin-node": "^7.0.1",
    "eslint-plugin-prettier": "^2.6.2",
    "eslint-plugin-promise": "^4.0.0",
    "eslint-plugin-react": "^7.11.1",
    "eslint-plugin-standard": "^4.0.0",
    "eslint-plugin-typescript": "^0.12.0",

which reports:

./App.tsx
Module Error (from ../node_modules/thread-loader/dist/cjs.js):

/ui/src/App.tsx
  45:11  error  'AppProps' is not defined          no-undef

@SiAdcock SiAdcock referenced this issue Sep 4, 2018

Closed

eslint #160

mysticatea added a commit that referenced this issue Nov 8, 2018

mysticatea added a commit that referenced this issue Nov 13, 2018

Update: add proper scope analysis (fixes #535) (#540)
* Update: add proper scope analysis (fixes #535)

* add computed-properties-in-type fixture

* add computed-properties-in-interface fixture

* add function-overload fixture

* add method-overload fixture

* add class-properties fixture

* add decorators fixture

* update visitor-keys

* add declare-global fixture

* fix typo

* add test for typeof in array destructuring

* add namespace fixture

* add declare-module fixture

* fix crash

* add declare-function.ts fixture

* add abstract-class fixture

* add typeof-in-call-signature fixture

* add test for #416

* add test for #435

* add test for #437

* add test for #443

* add test for #459

* add test for #466

* add test for #471

* add test for #487

* add test for #535

* add test for #536

* add test for #476

* fix test to use `expect()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment