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

Lint extensions #10162

Closed
wants to merge 3 commits into from
Closed

Lint extensions #10162

wants to merge 3 commits into from

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Aug 5, 2016

Split from #9038. This PR will likely need to be reopened against master once #10159 is merged, but it exists to track the progress of lint plugins.

@ahejlsberg @mhegazy @rbuckton @vladima
Lint plugins exist to provide a simple way to efficiently add style and sanity checks to a project. In-house, we've been a little dissatisfied with tslint's performance, and it's obvious that to conserve as many resources as possible and achieve the maximum performance possible, lints aught to be built into the compiler, so they can share the work the compiler is already doing, report errors in a uniform way, and benefit from a condensed single-pass architecture. The design (namely the separation between syntactic and semantic lint passes) was inspired by Rust's lint extension API.

Lint plugins as they currently stand take the form of class-like functions (though a factory function will work, too) with a visit method and an optional afterVisit method. visit gets called with a node as the walker descends through the tree, along with an error function which can be used for error reporting. The error function has a wealth of overloads which allow setting the error level, a shortcode to associate with the error message, the location of the error message, and the message itself. A walker may return true in its visit function to indicate that it no longer needs to recur down into the tree at a given point. If all lints do not needs to visit a portion of the tree, then that portion of the walk is skipped.

This is mostly a straight cut from the original PR, things to do from our internal discussion yesterday:

  • Move lint diagnostics into their own methods and out of getSyntacticDiagnostics and getSemanticDiagnostics, so getting semantic/syntactic diagnostics no longer triggers a lint. (This way they can be queried less frequently than standard diagnostics.)
  • Collect lint diagnostics in tsc.js, and add endpoints to tsserver to query for lint diagnostics. (This means editors will need to opt-in to showing lint errors, and decide for themselves how frequently to query for them.)
  • Pass a cancellation token into the lint plugin so that a well-behaved plugin can cancel its operation when the editor requests it (assuming the editor has requested lint diagnostics and follows a model which supports cancellation tokens).

Additionally, lint extensions will need to be updated to have quick fix support once #9304 is merged.

@basarat
Copy link
Contributor

basarat commented Aug 5, 2016

Nice :) Another thing I don't have to think about forking. I recently added tslint support to alm and added the project support : https://basarat.gitbooks.io/alm/content/features/lint.html. This would make the story much more better 🌹

@weswigham weswigham force-pushed the extension-loading branch 2 times, most recently from 8ee57f3 to 5ad76bd Compare August 9, 2016 01:38
@weswigham weswigham force-pushed the extension-loading branch 2 times, most recently from 445d7f7 to d8aec99 Compare August 18, 2016 00:34
@angelozerr
Copy link

@weswigham I 'm really interested with your work. Do you think your work could b emerged to master? If yes, which version of TypeScript could host your work. Thanks!

@angelozerr
Copy link

angelozerr commented Dec 15, 2016

And since TypeScript provides code fix support, it should really fantastic to register too tslint code fixes.

@mhegazy
Copy link
Contributor

mhegazy commented May 22, 2017

@mhegazy mhegazy closed this May 22, 2017
@mhegazy mhegazy deleted the lint-extensions branch May 22, 2017 23:31
@jez9999
Copy link

jez9999 commented Oct 16, 2017

Why was this closed? Language Service Plugins aren't really the same as a linting architecture. They're individual things and none of them are built-in lint functionality for TypeScript. They also only apply at design time whereas linting should probably be runnable on the command line as a pre-built task.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants