panuhorsmalahti / gulp-tslint

TypeScript linter plugin for Gulp
MIT License
118 stars 44 forks source link

Make use of TSLint formatters, rather than custom gulp-tslint reporters #66

Closed cgwrench closed 8 years ago

cgwrench commented 8 years ago

This pull request removes all gulp-tslint reporters. Instead, formatters included with TSLint, or formatters compatible with TSLint, are used.

This change allows gulp-tslint to use any formatter that is added to TSLint, for example the new VSO formatter recently added to TSLint, without requiring any changes to gulp-tslint. This will close https://github.com/panuhorsmalahti/gulp-tslint/issues/64 and close https://github.com/panuhorsmalahti/gulp-tslint/issues/27.

Please let me know if the approach adopted looks acceptable, and whether there are any changes that you would like to see. Overall this change has simplified the code for gulp-tslint and made it more flexible. This has caused some breaking changes, discussed below, so I am more than happy to incorporate feedback into these changes if there are more appropriate ways to introduce this feature.

Overview of changes

This is quite a significant change, and there may be some pain for users. Specifically:

  1. The options passed to tslintPlugin have changed. The PluginOptions interface is now:

    export interface PluginOptions {
       configuration?: any;
       formatter?: string;
       formattersDirectory?: string;
       rulesDirectory?: string;
       tslint?: any;
    }
  2. The signature for tslintPlugin.report has changed.

    This method no longer takes a reporter name as the first argument (instead a formatter is specified in the options to tslintPlugin), and the Reporter interface has been removed. The signature of tslintPlugin.report has therefore changed from

    tslintPlugin.report(reporter: string | Reporter, options?: ReportOptions)

    to

    tslintPlugin.report(options?: ReportOptions)

    This is arguably the most significant change. All users will have to change existing code that looks like

    gulp.task("tslint", () =>
       return gulp.src("source.ts")
           .pipe(tslint())
           .pipe(tslint.report("verbose"))
    );

    to

    gulp.task("tslint", () =>
       return gulp.src("source.ts")
           .pipe(tslint({
               formatter: "verbose"
           }))
           .pipe(tslint.report())
    );

    if upgrading to a release that contains these changes.

  3. Custom gulp-tslint reporters will no longer work; instead users will have to make use of the TSLint equivalents.

    For example, Instead of gulp-tslint-teamcity, tslint-teamcity-reporter should be used, and instead of gulp-tslint-stylish, tslint-stylish should be used. Note that gulp-tslint-stylish already appears to be marked as deprecated.

  4. tslintPlugin.ProseErrorFormat is no longer exported.

    Rather than defining a custom Failure interface (and the associated Position interface), I've made use of the TSLint RuleFailure interface directly. This, together with the removal of the Reporter interface (see point 2, above) led me to remove the proseErrorFormat method from the public API.

    Outstanding concerns

I believe that this pull request contains a complete implementation of this feature. However, I haven't attempted to update the README or CHANGELOG. I am happy to update these in another commit or pull request if these changes are suitable for merging.

panuhorsmalahti commented 8 years ago

Looks great! If you could add documentation / breaking changes I'll merge this. Bump the major version while you're at it.

cgwrench commented 8 years ago

I've had a go at updating the README to describe the use of TSLint formatters and removed any references to reporters. I have also added an entry and the CHANGELOG and bumped the version to 6.0.0. Please let me know if you'd like anything further changing or adding.

panuhorsmalahti commented 8 years ago

Thanks 👍. I'll probably add a "deprecated" warning if the user is trying to supply a string to the tslint.report function to make the transition smoother.

panuhorsmalahti commented 8 years ago

Released now.