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

[no-unused-vars] False positive when declaring modules #2523

Closed
troch opened this issue Sep 8, 2020 · 11 comments
Closed

[no-unused-vars] False positive when declaring modules #2523

troch opened this issue Sep 8, 2020 · 11 comments

Comments

@troch
Copy link

@troch troch commented Sep 8, 2020

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

I also tried canary versions.

Repro

{
  "rules": {
    "@typescript-eslint/no-unused-vars": "error"
  }
}
declare global {
  interface Window {
     /* ... */
  }
}

declare module 'external-module' {
  export default (arg: any) => any
}

Expected Result

I didn't expect any error.

Actual Result

  • Error 'global' is defined but never used.
  • Error 'arg' is defined but never used.

Versions

package version
@typescript-eslint/eslint-plugin 4.1.0
@typescript-eslint/parser 4.1.0
TypeScript 4.0.2
ESLint 7.8.1
node 12.18.1
@bradzacher
Copy link
Member

@bradzacher bradzacher commented Sep 8, 2020

Where is your declare global?
Is it in a .d.ts file?

@bradzacher bradzacher added awaiting response and removed triage labels Sep 8, 2020
@tblue1994
Copy link

@tblue1994 tblue1994 commented Sep 8, 2020

We are running into a very similar situation, and yes, it is happening inside a .d.ts file

@bradzacher
Copy link
Member

@bradzacher bradzacher commented Sep 8, 2020

If you're running the latest version, then there should be no way that it's reporting in a .d.ts file.

Could you provide more information please? A comment with no context is counter productive to helping me investigate.

A repro repo is always a great way to help the volunteer maintainers investigate.

@troch
Copy link
Author

@troch troch commented Sep 8, 2020

In my case declare global is not in a d.ts file but collocated with code.

@wKovacs64
Copy link

@wKovacs64 wKovacs64 commented Sep 9, 2020

image

Not sure if this is the same or slightly different - in my case, I'm using declaration merging with a namespace from node_modules. Same versions as @troch reported. And I guess it's right, I'm not using Subject, but that's the signature of the declaration I'm merging into, so...?

I can try to put together a minimal reproduction soon.

Reproduction here: https://github.com/wKovacs64/typescript-eslint-issue-2523 (edit: deleted, solved below)

@zouhangwithsweet

This comment was marked as off-topic.

@ldantonov
Copy link

@ldantonov ldantonov commented Sep 10, 2020

I see the same issue upgrading to v4.1.0 from 4.0.1, in a .ts file:

declare global {
  interface HTMLElement {
    _fitSpaceOnResize?: FitSpaceResizeListener
  }
}

error 'global' is defined but never used @typescript-eslint/no-unused-vars

@davidje13
Copy link

@davidje13 davidje13 commented Sep 12, 2020

I see the same problem when defining custom Jest matchers:

declare global { // <-- 'global' is defined but never used (@typescript-eslint/no-unused-vars)
  namespace jest {
    interface Matchers<R> { // <-- 'Matchers' is defined but never used (@typescript-eslint/no-unused-vars)
      toBeSeven: () => R;
    }
  }
}

expect.extend({
  toBeSeven(value): jest.CustomMatcherResult {
    if (value !== 7) {
      return { pass: false, message: (): string => `Expected seven, but got ${value}` };
    }
    return { pass: true, message: (): string => 'Expected non-seven' };
  },
});

it('is seven', () => {
  expect(7).toBeSeven();
});

As others have noted, this is in the main .ts file, not in a .d.ts file. AFAIK this is a pretty standard pattern when defining custom matchers for use within a project.

@bradzacher bradzacher added bug and removed awaiting response labels Sep 12, 2020
@bradzacher bradzacher self-assigned this Sep 13, 2020
bradzacher added a commit that referenced this issue Sep 13, 2020
…ules

Fixes #2523

- `declare global` should never be marked as unused.
- namespaces within declared namespaces all ambiently export their children
@bradzacher
Copy link
Member

@bradzacher bradzacher commented Sep 13, 2020

@wKovacs64

declare namespace Cypr_something_I_cant_see {
  interface Chainable<Subject = any> {
    toggleDarkMode(): void;
    clearLocalStorageForReal(): void;
  }
}

This case is not a false-positive - the generic is unused.
If the generic is required for some reason, you must use a disable comment.


The other cases of:

  • using a declare global in a .ts file being marked as unused
  • children of a namespace that is a child of a declared namespace being marked as unused

will be fixed in the next version.

@bradzacher bradzacher added the has pr label Sep 13, 2020
bradzacher added a commit that referenced this issue Sep 13, 2020
…ules (#2553)

Fixes #2523

- `declare global` should never be marked as unused.
- namespaces within declared namespaces all ambiently export their children
@davidje13
Copy link

@davidje13 davidje13 commented Sep 13, 2020

If the generic is required for some reason, you must use a disable comment.

It isn't required, so the proper fix is to change the code (in this case, just interface Chainable { with no generics).

TypeScript will still correctly merge the definitions if one has no generics (or only some of the generics). I confirmed that when I changed Matchers<R, T> to just Matchers<R> in my Jest example, because the rule is correctly catching unused generic arguments.


Thanks for the fix!

@wKovacs64
Copy link

@wKovacs64 wKovacs64 commented Sep 13, 2020

Ah, learn something new every day - thanks, @davidje13 and @bradzacher!

phaux added a commit to phaux/typescript-eslint that referenced this issue Sep 28, 2020
…ules (typescript-eslint#2553)

Fixes typescript-eslint#2523

- `declare global` should never be marked as unused.
- namespaces within declared namespaces all ambiently export their children
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants