Add optional type information to rules (#1323) #1363

ScottSWu · 2016-06-30T18:00:45Z

Changed Linter to accept an optional program object in the constructor
- Added helper functions for creating a program using a tsconfig and getting relevant files
Created TypedRule class which receives the program object
- Rules extending this class must implement applyWithProgram
- Linter passes program to TypedRules if available
- Calling apply on a TypedRule throws an error
Added requiresTypeInfo boolean to metadata
Created ProgramAwareRuleWalker which walks with the program / typechecker
Added cli options --project and --type-check
- --project takes a tsconfig.json file
- --type-check enables type checking and TypedRules, requires --project
Added an example restrictPlusOperands rule and tests that uses types

alexeagle · 2016-06-30T21:24:43Z

src/language/rule/typedRule.ts

+
+export abstract class TypedRule extends AbstractRule {
+    public apply(sourceFile: ts.SourceFile): RuleFailure[] {
+        return [];


Add a comment about the behavior here, like "Silently ignore this rule if tslint is executed without a program, as the rule requires type-check information"

alexeagle · 2016-07-06T19:07:29Z

src/language/walker/programAwareRuleWalker.ts

@@ -20,8 +20,8 @@ import {IOptions} from "../../lint";
 import {RuleWalker} from "./ruleWalker";

 export class ProgramAwareRuleWalker extends RuleWalker {
-    protected program: ts.Program;
-    protected typeChecker: ts.TypeChecker;
+    private program: ts.Program;


@jkillian is there a reason your style preference is for field declaration rather than a private modifier on the constructor params?

historically we've tended towards that code style so it's easy to locate all the fields in one place. we've loosened up the guidelines a bit; I don't really mind using constructor param properties.

Let's go ahead and use parameter properties where possible 👍
In fact, just merged #1373 which does this in a few locations

* --project specifies the tsconfig.json file * --type-check enables the type checker and TypedRules * If files are specified as arguments with a tsconfig, only files will be linted * Added exit code checks for new cli options

* Calling apply on a TypedRule throws an Error * restrictPlusOperands * Added additional tests * Removed extraneous constructor * Updated failure messages * Fixed nits, descriptions, newlines, template strings * Added jsdoc comments to Linter methods * Changed cli tests to accommodate changes

ScottSWu · 2016-07-11T17:56:59Z

Hi, sorry for the large PR. Could I get another review of the changes, and should I begin squashing commits?

adidahiya · 2016-07-11T19:35:42Z

Sweet, this looks pretty good @ScottSWu! I think I'll just squash + merge at the end into one commit and take your PR description as the commit message body.

One last thing -- can you add documentation to the README about how to use these new APIs? This includes the new CLI options and static methods on the Linter class.

alexeagle · 2016-07-11T21:33:29Z

README.md

@@ -115,6 +115,8 @@ Options:
 -e, --exclude         exclude globs from path expansion
 -t, --format          output format (prose, json, verbose, pmd, msbuild, checkstyle)  [default: "prose"]
 --test                test that tslint produces the correct output for the specified directory
+--project             tsconfig.json file


path to tsconfig.json file

adidahiya · 2016-07-11T23:27:53Z

README.md

@@ -214,6 +224,21 @@ const linter = new Linter(fileName, fileContents, options);
 const result = linter.lint();
 ```

+#### Type Checking
+
+To enable rules that work with the type checker, a TypeScript program object may be passed to the linter. Helper functions are provided to create a program from a `tsconfig.json` file. A project directory can be specified if project files do not lie in the same directory as the `tsconfig.json` file.


This should probably be edited to "... a TypesScript program [must] be passed to the linter [when using the programmatic API]." (my additions in brackets)

Also it would be good to clarify that a program is automatically created for you when using the appropriate CLI flags.

Fixed, and added a line below the code sample regarding the CLI.

adidahiya · 2016-07-11T23:50:27Z

🎉

HamletDRC · 2016-07-12T05:08:47Z

Will the grunt-tslint plugin need to be updated before grunt users can use this?

Thank you @ScottSWu . There are many users of microsoft-tslint-contrib that really have been wanting this.

myitcv · 2016-07-12T06:30:32Z

@ScottSWu @alexeagle et al - this is great work, thank you! Look forward to trying it out in the @next release

Fank · 2016-07-12T09:02:47Z

@adidahiya Possible to release a new version, which contains these change, soon?

adidahiya · 2016-07-12T21:13:32Z

@Fank @ScottSWu I'm attempting to cut a release here (#1395) but running into some compilation issues. I don't have time to dig into these right now but I'll try to get a release out for you guys ASAP (hopefully tonight or tomorrow)!

adidahiya · 2016-07-14T00:53:19Z

Released v3.14.0-dev.0

glen-84 · 2016-08-02T08:24:35Z

Should it not silently fail if you don't enable type checking? This would allow you to see all the other errors/warnings in an editor, even when you haven't enabled type checking (which won't always be possible or easy to do).

alexeagle reviewed Jun 30, 2016
View reviewed changes

adidahiya added the PR: waiting for reviewer label Jun 30, 2016

ScottSWu force-pushed the typechecking-main branch 2 times, most recently from 8d2ece4 to ab31fc5 Compare July 1, 2016 18:53

alexeagle reviewed Jul 6, 2016
View reviewed changes

adidahiya added PR: waiting for author and removed PR: waiting for reviewer labels Jul 6, 2016

jkillian added PR: waiting for reviewer and removed PR: waiting for author labels Jul 7, 2016

ScottSWu added 4 commits July 7, 2016 10:29

Add cli options for tsconfig and typechecking

a156d11

* --project specifies the tsconfig.json file * --type-check enables the type checker and TypedRules * If files are specified as arguments with a tsconfig, only files will be linted * Added exit code checks for new cli options

Fix import order for build

946c20c

Copyright year and constructor parameter properties

5c17efc

ScottSWu force-pushed the typechecking-main branch from 8ee37c9 to 5c17efc Compare July 7, 2016 17:41

adidahiya mentioned this pull request Jul 7, 2016

Add a rule to disallow "for … in" on array types #1380

Closed

adidahiya added PR: waiting for author and removed PR: waiting for reviewer labels Jul 11, 2016

Added new options, API and rules to the README

137678f

alexeagle reviewed Jul 11, 2016
View reviewed changes

Fix README changes

79df24e

adidahiya reviewed Jul 11, 2016
View reviewed changes

Fix additional README changes

2afe912

adidahiya removed the PR: waiting for author label Jul 11, 2016

adidahiya merged commit 9248e05 into palantir:master Jul 11, 2016

adidahiya mentioned this pull request Jul 12, 2016

Lint Programs as a Whole #680

Closed

adidahiya mentioned this pull request Jul 14, 2016

Running tests is much slower now that type checking is always enabled #1402

Closed

adidahiya mentioned this pull request Aug 13, 2016

Expose an API that accepts ts.SourceFile or ts.Program directly #1096

Closed

AdamCmiel mentioned this pull request Nov 7, 2016

Switch should exhaust enum #696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional type information to rules (#1323) #1363

Add optional type information to rules (#1323) #1363

ScottSWu commented Jun 30, 2016 •

edited by adidahiya

Loading

alexeagle Jun 30, 2016

alexeagle Jul 6, 2016 •

edited

Loading

adidahiya Jul 6, 2016

jkillian Jul 7, 2016

ScottSWu commented Jul 11, 2016

adidahiya commented Jul 11, 2016

alexeagle Jul 11, 2016

adidahiya Jul 11, 2016

ScottSWu Jul 11, 2016

adidahiya commented Jul 11, 2016

HamletDRC commented Jul 12, 2016 •

edited

Loading

myitcv commented Jul 12, 2016

Fank commented Jul 12, 2016

adidahiya commented Jul 12, 2016

adidahiya commented Jul 14, 2016

glen-84 commented Aug 2, 2016

Add optional type information to rules (#1323) #1363

Add optional type information to rules (#1323) #1363

Conversation

ScottSWu commented Jun 30, 2016 • edited by adidahiya Loading

alexeagle Jun 30, 2016

Choose a reason for hiding this comment

alexeagle Jul 6, 2016 • edited Loading

Choose a reason for hiding this comment

adidahiya Jul 6, 2016

Choose a reason for hiding this comment

jkillian Jul 7, 2016

Choose a reason for hiding this comment

ScottSWu commented Jul 11, 2016

adidahiya commented Jul 11, 2016

alexeagle Jul 11, 2016

Choose a reason for hiding this comment

adidahiya Jul 11, 2016

Choose a reason for hiding this comment

ScottSWu Jul 11, 2016

Choose a reason for hiding this comment

adidahiya commented Jul 11, 2016

HamletDRC commented Jul 12, 2016 • edited Loading

myitcv commented Jul 12, 2016

Fank commented Jul 12, 2016

adidahiya commented Jul 12, 2016

adidahiya commented Jul 14, 2016

glen-84 commented Aug 2, 2016

ScottSWu commented Jun 30, 2016 •

edited by adidahiya

Loading

alexeagle Jul 6, 2016 •

edited

Loading

HamletDRC commented Jul 12, 2016 •

edited

Loading