Fix latest tslint errors #13006

saschanaz · 2016-12-18T06:51:10Z

Except this type of errors:

export function getNavigationTree(sourceFile: SourceFile): NavigationTree {
    curSourceFile = sourceFile;
    const result = convertToTree(rootNavigationBarNode(sourceFile));
    curSourceFile = undefined;
    return result;
}
let curSourceFile: SourceFile;
// Identifier 'curSourceFile' is never reassigned; use 'const' instead of 'let'.

let response: protocol.Response;
try {
    ({ response } = this.executeCommand(msg));
}
//  Identifier 'response' is never reassigned; use 'const' instead of 'let'.

I think they are tslint bugs rather than actual code problem. (palantir/tslint#1898)

BTW I feel fairly painful to create a new constant for every for-loop, I hope there is a better way to keep high performance...

saschanaz · 2016-12-18T13:34:05Z

A benchmark shows for-of loop is much slower on Chrome (and thus node.js), does this mean we shouldn't use for-of?

ghost · 2016-12-18T19:23:17Z

for-of gets emitted as a for loop, since we use an ES5 target. (So maybe some of those manually-written for loops could be upgraded.)
Also, this would be a good time to delete our own preferConstRule.ts since tslint has that now.

saschanaz · 2016-12-19T04:09:29Z

Already done the upgrade for obvious cases but many of them are using indices so I couldn't do it for them.

… on #13006 to upgrade.

mhegazy

I would inline the collection.length check instead of adding a new variable for it. There is not much perf difference on modern engines anyways.

mhegazy · 2016-12-19T17:34:50Z

src/compiler/checker.ts

@@ -5118,7 +5119,8 @@ namespace ts {
        function getSignaturesOfSymbol(symbol: Symbol): Signature[] {
            if (!symbol) return emptyArray;
            const result: Signature[] = [];
-            for (let i = 0, len = symbol.declarations.length; i < len; i++) {
+            const len = symbol.declarations.length;
+            for (let i = 0; i < len; i++) {


switch to for..of?

Inner lines of code use i for some purpose. It seems I can still switch to for-of in this case but should I do it?

mhegazy · 2016-12-19T17:35:33Z

src/compiler/checker.ts

@@ -5019,7 +5019,8 @@ namespace ts {
                // If this is a JSDoc construct signature, then skip the first parameter in the
                // parameter list.  The first parameter represents the return type of the construct
                // signature.
-                for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
+                const n = declaration.parameters.length;
+                for (let i = isJSConstructSignature ? 1 : 0; i < n; i++) {
                    const param = declaration.parameters[i];


for (let i = isJSConstructSignature ? 1 : 0; i < declaration.parameters.length; i++) {

mhegazy · 2016-12-19T18:36:27Z

@saschanaz i have merged a change to move tslint back to 4.0.0-dev.3 (#13028). can you update it with your PR to next again.

… lintfix

saschanaz · 2016-12-19T18:50:16Z

It still won't pass because of some bugs, which should be fixed on tslint side.

mhegazy · 2016-12-19T19:10:29Z

It still won't pass because of some bugs, which should be fixed on tslint side.

do you have a list of the issues?

ghost · 2016-12-19T19:12:04Z

@mhegazy See first post and palantir/tslint#1898. I actually fixed one of them yesterday. The other one is also a legitimate issue, but could be fixed on our end by moving the declaration ( let curSourceFile: SourceFile;) to before the function.

saschanaz · 2016-12-19T19:14:28Z

@Andy-MS I'm still seeing destructuring issue on Travis, should I do something to get the merged fix?

ghost · 2016-12-19T19:17:53Z

tslint hasn't published a new version yet, so there's not much to do but wait.

vladima · 2016-12-19T20:01:14Z

I'd prefer not to change our code to workaround bugs in the toolchain hence palantir/tslint#1908

mhegazy · 2016-12-21T20:54:33Z

@nchen63 any news about palantir/tslint#1908?

nchen63 · 2016-12-23T07:09:05Z

it's merged and included in a new release, v4.2.0

saschanaz · 2016-12-23T15:46:33Z

It seems tslint@next is still giving tslint@4.1.0-dev.0.

nchen63 · 2016-12-23T16:30:33Z

should be fixed now

saschanaz added 3 commits December 10, 2016 17:53

Merge remote-tracking branch 'refs/remotes/Microsoft/master'

3202d59

Merge branch 'master' of https://github.com/Microsoft/TypeScript

603ba89

fix linting errors

27a60e4

msftclas added the cla-already-signed label Dec 18, 2016

This was referenced Dec 18, 2016

Format false branch of ternary operation #5905

Closed

tslint@next does not recognize some reassignation palantir/tslint#1898

Closed

remove preferConstRule.ts

27ed5b8

HerringtonDarkholme mentioned this pull request Dec 19, 2016

Fix #1809, introduce non primitive object type #12501

Merged

saschanaz closed this Dec 19, 2016

saschanaz reopened this Dec 19, 2016

msftclas added the cla-already-signed label Dec 19, 2016

ghost pushed a commit that referenced this pull request Dec 19, 2016

Fix tslint version. 4.1.0-dev.0 introduced some bugs, so we must wait…

ef14c08

… on #13006 to upgrade.

ghost mentioned this pull request Dec 19, 2016

Fix tslint version. 4.1.0-dev.0 introduced some bugs, so we must wait on #13006 to upgrade. #13025

Closed

ghost pushed a commit that referenced this pull request Dec 19, 2016

Fix tslint version. 4.1.0-dev.0 introduced some bugs, so we must wait…

6a578f8

… on #13006 to upgrade.

mhegazy reviewed Dec 19, 2016

View reviewed changes

saschanaz added 2 commits December 20, 2016 03:12

inline length

2a941a7

needless newline

1f79741

mhegazy approved these changes Dec 19, 2016

View reviewed changes

saschanaz added 2 commits December 20, 2016 03:46

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

1a6e43d

… lintfix

next again

67e1fd8

saschanaz closed this Dec 23, 2016

saschanaz reopened this Dec 23, 2016

msftclas added the cla-already-signed label Dec 23, 2016

saschanaz closed this Dec 23, 2016

saschanaz reopened this Dec 23, 2016

msftclas added the cla-already-signed label Dec 23, 2016

saschanaz closed this Dec 23, 2016

saschanaz reopened this Dec 23, 2016

msftclas added the cla-already-signed label Dec 23, 2016

saschanaz closed this Dec 23, 2016

saschanaz reopened this Dec 23, 2016

msftclas added the cla-already-signed label Dec 23, 2016

mhegazy merged commit e4b81d0 into microsoft:master Dec 26, 2016

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix latest tslint errors #13006

Fix latest tslint errors #13006

saschanaz commented Dec 18, 2016 •

edited by DanielRosenwasser

Loading

saschanaz commented Dec 18, 2016 •

edited

Loading

ghost commented Dec 18, 2016

saschanaz commented Dec 19, 2016

mhegazy left a comment

mhegazy Dec 19, 2016

saschanaz Dec 19, 2016 •

edited

Loading

mhegazy Dec 19, 2016

mhegazy commented Dec 19, 2016

saschanaz commented Dec 19, 2016

mhegazy commented Dec 19, 2016

ghost commented Dec 19, 2016 •

edited by ghost

Loading

saschanaz commented Dec 19, 2016

ghost commented Dec 19, 2016

vladima commented Dec 19, 2016

mhegazy commented Dec 21, 2016

nchen63 commented Dec 23, 2016

saschanaz commented Dec 23, 2016

nchen63 commented Dec 23, 2016

Fix latest tslint errors #13006

Fix latest tslint errors #13006

Conversation

saschanaz commented Dec 18, 2016 • edited by DanielRosenwasser Loading

saschanaz commented Dec 18, 2016 • edited Loading

ghost commented Dec 18, 2016

saschanaz commented Dec 19, 2016

mhegazy left a comment

Choose a reason for hiding this comment

mhegazy Dec 19, 2016

Choose a reason for hiding this comment

saschanaz Dec 19, 2016 • edited Loading

Choose a reason for hiding this comment

mhegazy Dec 19, 2016

Choose a reason for hiding this comment

mhegazy commented Dec 19, 2016

saschanaz commented Dec 19, 2016

mhegazy commented Dec 19, 2016

ghost commented Dec 19, 2016 • edited by ghost Loading

saschanaz commented Dec 19, 2016

ghost commented Dec 19, 2016

vladima commented Dec 19, 2016

mhegazy commented Dec 21, 2016

nchen63 commented Dec 23, 2016

saschanaz commented Dec 23, 2016

nchen63 commented Dec 23, 2016

saschanaz commented Dec 18, 2016 •

edited by DanielRosenwasser

Loading

saschanaz commented Dec 18, 2016 •

edited

Loading

saschanaz Dec 19, 2016 •

edited

Loading

ghost commented Dec 19, 2016 •

edited by ghost

Loading