Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

[no-var][semi] behaviour for interfaces and declare var #266

Closed
ficristo opened this issue Dec 24, 2018 · 5 comments
Closed

[no-var][semi] behaviour for interfaces and declare var #266

ficristo opened this issue Dec 24, 2018 · 5 comments
Labels
bug/incomplete rule parser bug bugs with typescript-eslint-parser / typescript-estree

Comments

@ficristo
Copy link

Repro
See below.

{
  "rules": {
    "no-var": "error",
    "semi": "error"
  }
}
interface Object {
    foo(): string,
}
declare var Object: {
    new (value?: any): Object,
    foo(): string
}
declare namespace CodeMirror {
    export var Doc : CodeMirror.DocConstructor;
    var version: string;
}

Expected Result
No no-var errors.
semi errors for the three lines inside the curly braces. (I'm not sure if commas are valid, it seems typescript is quite lenient about them). No errors after the curly braces.

Actual Result
no-var errors.
no semi errors but after the last curly brace.

Additional Info

Versions

package version
eslint-plugin-typescript 1.0.0-rc.0
typescript-eslint-parser eslint-plugin-typescript/parser
typescript 3.1.6
@ficristo ficristo changed the title [no-var][semi] behaviour for interfaces and declar var [no-var][semi] behaviour for interfaces and declare var Dec 24, 2018
@bradzacher bradzacher added bug/incomplete rule requires investigation bug that require more investigation labels Jan 2, 2019
@bradzacher
Copy link
Owner

bradzacher commented Jan 2, 2019

commented on the wrong issue - comment moved to #268

@bradzacher bradzacher added parser bug bugs with typescript-eslint-parser / typescript-estree and removed requires investigation bug that require more investigation labels Jan 2, 2019
@bradzacher
Copy link
Owner

As for your semi desired errors;

interface Object {
    foo(): string,
}
declare var Object: {
    new (value?: any): Object,
    foo(): string
} // error: Missing semicolon.

use typescript/member-delimiter-style. This will enforce styling for your type/interface definitions.

semi should (and does) only error on the missing semicolon for the variable declaration.


declare namespace CodeMirror {
    export var Doc : CodeMirror.DocConstructor;
    var version: string;
}
declare namespace CodeMirror {
    export var Doc : CodeMirror.DocConstructor // error: Missing semicolon.
    var version: string // error: Missing semicolon.
}

semi correctly handles variable definitions within a namespace.

@armano2
Copy link
Contributor

armano2 commented Jan 2, 2019

some time ago i added support for declare: JamesHenry/typescript-estree#42
but we are still waiting for eslint/typescript-eslint-parser#584 to be merged before we can use it here

there are going to be a lot of changes / fixes to existing rules after typescript-eslint-parser upgrades typescript-estree from 5.3.0 to 9.0.0 xd

@bradzacher
Copy link
Owner

My earlier comment was about no-unused-vars, when this issue was about no-var

I would say that the errors are correct.

declare namespace CodeMirror {
    export var Doc : CodeMirror.DocConstructor; // error: Unexpected var, use let or const instead.
    var version: string; // error: Unexpected var, use let or const instead.
}

You should instead use let/const in your namespace declarations (playground).


interface Object {
    foo(): string,
}
declare var Object: { // error: Unexpected var, use let or const instead.
    new (value?: any): Object,
    foo(): string
}

It's hard to say without knowing your use case; but I think that this is probably incorrect use of declare var.
Extending the base Object definition is a weird thing to do, and would have unintended side-effects.
A better solution would be to use a .d.ts file and extend the definition.

Though we could add support for skipping declared vars.
It's a bit hard for us though, as we'd have to redefine the entire rule to make it work.

@ficristo
Copy link
Author

ficristo commented Jan 2, 2019

My mistake: the semi errors were about only the first snippet, the second one, as you noted, is correct.
I'll take a look to typescript/member-delimiter-style.
For now I'm fine to know I should use let \ const and avoid a declare var.
Thank you.

@ficristo ficristo closed this as completed Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug/incomplete rule parser bug bugs with typescript-eslint-parser / typescript-estree
Projects
None yet
Development

No branches or pull requests

3 participants