Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[Breaking change] Remove all use of the language service. Use only the Program. #2235

Merged
merged 7 commits into from Mar 14, 2017
Merged

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2017

PR checklist

What changes did you make?

Tslint now only provides the Program, not the LanguageService. LanguageService is for editors; tslint should use the Program and TypeChecker API.
This would be break any custom typed rules, so should be in a major version change. So long as the custom typed rules do not use the language service, conversion is simple. See await-promise for an example.

Only no-use-before-declare and no-unused-variable used the language service.
Rewrote no-use-before-declare. It now gets the symbol at each use and checks that it comes after the declaration.
I think no-unused-variable was going to be deprecated anyway, so removed it. It would be possible to rewrite that too if you want to keep it.

As a side-effect, fixes #1969 since we don't have wrapProgram, which creates a new language service, which then has its own program. This probably helps performance too, but I haven't tested.

Also changed tslint's own linting to use the program. Using typed rules in this repo could be done in another PR.

Will rebase after #2234 is in.

@@ -31,5 +31,5 @@ export abstract class TypedRule extends AbstractRule {
throw new Error(`The '${this.ruleName}' rule requires type checking`);
}

public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.Program): RuleFailure[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also rename the parameter

@nchen63
Copy link
Contributor

nchen63 commented Mar 6, 2017

actually, #1481 is no longer being deprecated due to community response. Can you put it back?

@nchen63
Copy link
Contributor

nchen63 commented Mar 6, 2017

The parameter originally used program, and was changed to use language service in #1608

Need to verify that the original issue is doesn't break

@ghost
Copy link
Author

ghost commented Mar 14, 2017

Rewrote no-unused-variable to not need the language service. Done by creating a duplicate program with the unused-checking compiler options set. Added workaround for microsoft/TypeScript#9944 (which #1608 fixed) that doesn't require checkEdit.
This rule would presumably be unnecessary if we fixed that bug and implemented microsoft/TypeScript#11051.

Copy link
Contributor

@nchen63 nchen63 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good

@nchen63 nchen63 merged commit f11f309 into palantir:master Mar 14, 2017
@ghost ghost deleted the no_language_service branch March 14, 2017 18:58
@ghost
Copy link
Author

ghost commented Mar 14, 2017

Note that this makes no-unused-variable and no-use-before-declare typed rules, while before they worked without a type checker.

@nchen63
Copy link
Contributor

nchen63 commented Mar 14, 2017

That might be a problem for people who use plug-ins like the VSCode-TSLint since typed rules are not supported. People seem to use this rule to see the warnings instead of using the compiler flag so it doesn't block compilation; see #1481 (comment).

@ajafff
Copy link
Contributor

ajafff commented Mar 14, 2017

I'm already working on a solution to use both rules without type checking.
no-use-before-declare is no problem.
no-unused-variable would only find unused private class members with type checking enabled. The problem with implicit required imports may also still be there - haven't looked at the code yet - so we would exclude them from the check without type checking. This way we would at least get most of the failures and find the rest with type checking.

@ghost
Copy link
Author

ghost commented Mar 14, 2017

It may be possible to create a program consisting of a single file and run the rest as usual. no-unused-variable has a lot of corner cases so I wouldn't want to duplicate the work done in the TS compiler.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants