From e86026c26a7bddad3f879104d329e4d8ad3ef73c Mon Sep 17 00:00:00 2001 From: Matt Mazzola Date: Mon, 22 Aug 2016 09:52:58 -0700 Subject: [PATCH] Setup tslint on build and test to enforce good practices and consistent style. (#9) * Setup tslint on build and test to enforce good practices and consistent style. * Auto-format all files to use consistent tab size of 2. --- .vscode/settings.json | 6 ++ gulpfile.js | 212 +++++++++++++++++++++---------------- karma.conf.js | 50 ++++----- package.json | 2 + src/models.ts | 63 +++++------ test/models.spec.ts | 3 +- tsconfig.json | 28 ++--- tslint.json | 25 +++++ webpack.test.tsconfig.json | 24 ++--- 9 files changed, 235 insertions(+), 178 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 tslint.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..e6eb634 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,6 @@ +// Place your settings in this file to overwrite default and user settings. +{ + "editor.tabSize": 2, + "editor.detectIndentation": false, + "editor.renderIndentGuides": true +} \ No newline at end of file diff --git a/gulpfile.js b/gulpfile.js index e805bdd..f55dfbf 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -1,142 +1,166 @@ var gulp = require('gulp-help')(require('gulp')); var del = require('del'), - ghPages = require('gulp-gh-pages'), - header = require('gulp-header'), - fs = require('fs'), - moment = require('moment'), - rename = require('gulp-rename'), - replace = require('gulp-replace'), - typedoc = require("gulp-typedoc"), - uglify = require('gulp-uglify'), - karma = require('karma'), - webpack = require('webpack'), - webpackStream = require('webpack-stream'), - webpackConfig = require('./webpack.config'), - webpackTestConfig = require('./webpack.test.config'), - runSequence = require('run-sequence'), - argv = require('yargs').argv; - ; + ghPages = require('gulp-gh-pages'), + header = require('gulp-header'), + fs = require('fs'), + moment = require('moment'), + rename = require('gulp-rename'), + replace = require('gulp-replace'), + tslint = require('gulp-tslint'), + typedoc = require("gulp-typedoc"), + uglify = require('gulp-uglify'), + karma = require('karma'), + webpack = require('webpack'), + webpackStream = require('webpack-stream'), + webpackConfig = require('./webpack.config'), + webpackTestConfig = require('./webpack.test.config'), + runSequence = require('run-sequence'), + argv = require('yargs').argv; +; var package = require('./package.json'); var webpackBanner = package.name + " v" + package.version + " | (c) 2016 Microsoft Corporation " + package.license; var gulpBanner = "/*! " + webpackBanner + " */\n"; gulp.task('build', 'Build for release', function (done) { - return runSequence( - 'clean:dist', - 'compile:ts', - 'min', - 'generatecustomdts', - 'header', - done - ); + return runSequence( + 'tslint:build', + 'clean:dist', + 'compile:ts', + 'min', + 'generatecustomdts', + 'header', + done + ); }); gulp.task('test', 'Runs all tests', function (done) { - return runSequence( - 'clean:tmp', - 'compile:spec', - 'test:js', - done - ); + return runSequence( + 'tslint:test', + 'clean:tmp', + 'compile:spec', + 'test:js', + done + ); }); gulp.task('ghpages', 'Deploy documentation to gh-pages', ['nojekyll'], function () { - return gulp.src(['./docs/**/*'], { - dot: true - }) - .pipe(ghPages({ - force: true, - message: 'Update ' + moment().format('LLL') - })); + return gulp.src(['./docs/**/*'], { + dot: true + }) + .pipe(ghPages({ + force: true, + message: 'Update ' + moment().format('LLL') + })); }); gulp.task("docs", 'Compile documentation from src code', function () { - return gulp - .src(["src/**/*.ts"]) - .pipe(typedoc({ - mode: 'modules', - includeDeclarations: true, + return gulp + .src(["src/**/*.ts"]) + .pipe(typedoc({ + mode: 'modules', + includeDeclarations: true, - // Output options (see typedoc docs) - out: "./docs", - json: "./docs/json/" + package.name + ".json", + // Output options (see typedoc docs) + out: "./docs", + json: "./docs/json/" + package.name + ".json", - // TypeDoc options (see typedoc docs) - ignoreCompilerErrors: true, - version: true - })) - ; + // TypeDoc options (see typedoc docs) + ignoreCompilerErrors: true, + version: true + })) + ; }); gulp.task('nojekyll', 'Add .nojekyll file to docs directory', function (done) { - fs.writeFile('./docs/.nojekyll', '', function (error) { - if (error) { - throw error; - } + fs.writeFile('./docs/.nojekyll', '', function (error) { + if (error) { + throw error; + } - done(); - }); + done(); + }); }); -gulp.task('compile:ts', 'Compile source files', function() { - webpackConfig.plugins = [ - new webpack.BannerPlugin(webpackBanner) - ]; +gulp.task('compile:ts', 'Compile source files', function () { + webpackConfig.plugins = [ + new webpack.BannerPlugin(webpackBanner) + ]; - return gulp.src(['typings/**/*.d.ts', './src/**/*.ts']) - .pipe(webpackStream(webpackConfig)) - .pipe(gulp.dest('./dist')); + return gulp.src(['typings/**/*.d.ts', './src/**/*.ts']) + .pipe(webpackStream(webpackConfig)) + .pipe(gulp.dest('./dist')); }); gulp.task('header', 'Add header to distributed files', function () { - return gulp.src(['./dist/*.d.ts']) - .pipe(header(gulpBanner)) - .pipe(gulp.dest('./dist')); + return gulp.src(['./dist/*.d.ts']) + .pipe(header(gulpBanner)) + .pipe(gulp.dest('./dist')); }); gulp.task('min', 'Minify build files', function () { - return gulp.src(['!./dist/*.min.js', './dist/models.js']) - .pipe(uglify({ - preserveComments: 'license' - })) - .pipe(rename({ - suffix: '.min' - })) - .pipe(gulp.dest('./dist/')); + return gulp.src(['!./dist/*.min.js', './dist/models.js']) + .pipe(uglify({ + preserveComments: 'license' + })) + .pipe(rename({ + suffix: '.min' + })) + .pipe(gulp.dest('./dist/')); }); gulp.task('clean:dist', 'Clean dist folder', function () { - return del([ - './dist/**/*' - ]); + return del([ + './dist/**/*' + ]); }); gulp.task('clean:tmp', 'Clean tmp folder', function () { - return del([ - './tmp/**/*' - ]); + return del([ + './tmp/**/*' + ]); }); gulp.task('compile:spec', 'Compile spec tests', function () { - return gulp.src(['./test/test.spec.ts']) - .pipe(webpackStream(webpackTestConfig)) - .pipe(gulp.dest('./tmp')); + return gulp.src(['./test/test.spec.ts']) + .pipe(webpackStream(webpackTestConfig)) + .pipe(gulp.dest('./tmp')); }); gulp.task('generatecustomdts', 'Generate dts with no exports', function (done) { - return gulp.src(['./dist/*.d.ts']) - .pipe(replace(/export\s/g, '')) - .pipe(rename(function (path) { - path.basename = "models-noexports.d"; - })) - .pipe(gulp.dest('dist/')); + return gulp.src(['./dist/*.d.ts']) + .pipe(replace(/export\s/g, '')) + .pipe(rename(function (path) { + path.basename = "models-noexports.d"; + })) + .pipe(gulp.dest('dist/')); }); gulp.task('test:js', 'Run spec tests', function (done) { - new karma.Server.start({ - configFile: __dirname + '/karma.conf.js', - singleRun: argv.debug ? false : true, - captureTimeout: argv.timeout || 60000 - }, done); + new karma.Server.start({ + configFile: __dirname + '/karma.conf.js', + singleRun: argv.debug ? false : true, + captureTimeout: argv.timeout || 60000 + }, done); +}); + +gulp.task('tslint:build', 'Run TSLint on src', function () { + return gulp.src(["src/**/*.ts"]) + .pipe(tslint({ + formatter: "verbose" + })) + .pipe(tslint.report()); +}); + +gulp.task('tslint:test', 'Run TSLint on src and tests', function () { + return gulp.src(["src/**/*.ts", "test/**/*.ts"]) + .pipe(tslint({ + formatter: "verbose", + configuration: { + rules: { + "no-console": false + } + } + })) + .pipe(tslint.report()); }); \ No newline at end of file diff --git a/karma.conf.js b/karma.conf.js index 3bfc748..f5f7315 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -1,29 +1,29 @@ var argv = require('yargs').argv; module.exports = function (config) { - config.set({ - frameworks: ['jasmine'], - files: [ - './tmp/**/*.js' - ], - exclude: [], - reporters: argv.debug ? ['spec'] : ['spec', 'coverage'], - autoWatch: true, - browsers: [argv.chrome ? 'Chrome' : 'PhantomJS'], - plugins: [ - 'karma-chrome-launcher', - 'karma-jasmine', - 'karma-spec-reporter', - 'karma-phantomjs-launcher', - 'karma-coverage' - ], - preprocessors: { './tmp/**/*.js': ['coverage'] }, - coverageReporter: { - reporters: [ - { type: 'html' }, - { type: 'text-summary' } - ] - }, - logLevel: argv.debug ? config.LOG_DEBUG : config.LOG_INFO - }); + config.set({ + frameworks: ['jasmine'], + files: [ + './tmp/**/*.js' + ], + exclude: [], + reporters: argv.debug ? ['spec'] : ['spec', 'coverage'], + autoWatch: true, + browsers: [argv.chrome ? 'Chrome' : 'PhantomJS'], + plugins: [ + 'karma-chrome-launcher', + 'karma-jasmine', + 'karma-spec-reporter', + 'karma-phantomjs-launcher', + 'karma-coverage' + ], + preprocessors: { './tmp/**/*.js': ['coverage'] }, + coverageReporter: { + reporters: [ + { type: 'html' }, + { type: 'text-summary' } + ] + }, + logLevel: argv.debug ? config.LOG_DEBUG : config.LOG_INFO + }); }; \ No newline at end of file diff --git a/package.json b/package.json index 9f7531b..12fe4c2 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "gulp-help": "^1.6.1", "gulp-rename": "^1.2.2", "gulp-replace": "^0.5.4", + "gulp-tslint": "^6.0.2", "gulp-typedoc": "^2.0.0", "gulp-uglify": "^1.5.4", "jasmine-core": "^2.4.1", @@ -52,6 +53,7 @@ "phantomjs-prebuilt": "^2.1.7", "run-sequence": "^1.2.1", "ts-loader": "^0.8.2", + "tslint": "^3.14.0", "typedoc": "^0.4.4", "typescript": "^1.8.10", "typings": "^1.3.2", diff --git a/src/models.ts b/src/models.ts index a790dc9..90a472c 100644 --- a/src/models.ts +++ b/src/models.ts @@ -1,11 +1,13 @@ declare var require: Function; +/* tslint:disable:no-var-requires */ export const advancedFilterSchema = require('./schemas/advancedFilter.json'); export const filterSchema = require('./schemas/filter.json'); export const loadSchema = require('./schemas/load.json'); export const pageSchema = require('./schemas/page.json'); export const settingsSchema = require('./schemas/settings.json'); export const basicFilterSchema = require('./schemas/basicFilter.json'); +/* tslint:enable:no-var-requires */ import * as jsen from 'jsen'; @@ -20,7 +22,7 @@ export interface IError { } function normalizeError(error: IValidationError): IError { - if(!error.message) { + if (!error.message) { error.message = `${error.path} is invalid. Not meeting ${error.keyword} constraint`; } @@ -38,14 +40,14 @@ function validate(schema: any, options?: any) { const validate = jsen(schema, options); const isValid = validate(x); - if(isValid) { + if (isValid) { return undefined; } else { return validate.errors .map(normalizeError); } - } + }; } export interface ISettings { @@ -178,22 +180,22 @@ export function isHierarchy(arg: any): arg is IFilterHierarchyTarget { } export abstract class Filter { - protected static schemaUrl: string; - protected schemaUrl: string; static schema: string; + protected static schemaUrl: string; target: IFilterTarget; - + protected schemaUrl: string; + constructor( target: IFilterTarget ) { this.target = target; } - + toJSON(): IFilter { return { $schema: this.schemaUrl, target: this.target - } + }; }; } @@ -201,7 +203,7 @@ export class BasicFilter extends Filter { static schemaUrl: string = "http://powerbi.com/product/schema#basic"; operator: BasicFilterOperators; values: (string | number | boolean)[]; - + constructor( target: IFilterTarget, operator: BasicFilterOperators, @@ -210,40 +212,40 @@ export class BasicFilter extends Filter { super(target); this.operator = operator; this.schemaUrl = BasicFilter.schemaUrl; - - if(values.length === 0) { + + if (values.length === 0) { throw new Error(`values must be a non-empty array. You passed: ${values}`); } - + /** * Accept values as array instead of as individual arguments * new BasicFilter('a', 'b', 1, 2); * new BasicFilter('a', 'b', [1,2]); */ - if(Array.isArray(values[0])) { + if (Array.isArray(values[0])) { this.values = <(string | number | boolean)[]>values[0]; } else { this.values = <(string | number | boolean)[]>values; } } - + toJSON(): IBasicFilter { const filter = super.toJSON(); - + filter.operator = this.operator; filter.values = this.values; - + return filter; } } export class AdvancedFilter extends Filter { static schemaUrl: string = "http://powerbi.com/product/schema#advanced"; - + logicalOperator: AdvancedFilterLogicalOperators; conditions: IAdvancedFilterCondition[]; - + constructor( target: IFilterTarget, logicalOperator: AdvancedFilterLogicalOperators, @@ -251,48 +253,47 @@ export class AdvancedFilter extends Filter { ) { super(target); this.schemaUrl = AdvancedFilter.schemaUrl; - + // Guard statements - if(typeof logicalOperator !== "string" || logicalOperator.length === 0) { + if (typeof logicalOperator !== "string" || logicalOperator.length === 0) { // TODO: It would be nicer to list out the possible logical operators. throw new Error(`logicalOperator must be a valid operator, You passed: ${logicalOperator}`); } - + this.logicalOperator = logicalOperator; - - + let extractedConditions: IAdvancedFilterCondition[]; /** * Accept conditions as array instead of as individual arguments * new AdvancedFilter('a', 'b', "And", { value: 1, operator: "Equals" }, { value: 2, operator: "IsGreaterThan" }); * new AdvancedFilter('a', 'b', "And", [{ value: 1, operator: "Equals" }, { value: 2, operator: "IsGreaterThan" }]); */ - if(Array.isArray(conditions[0])) { + if (Array.isArray(conditions[0])) { extractedConditions = conditions[0]; } else { extractedConditions = conditions; } - if(extractedConditions.length === 0) { + if (extractedConditions.length === 0) { throw new Error(`conditions must be a non-empty array. You passed: ${conditions}`); } - if(extractedConditions.length > 2) { + if (extractedConditions.length > 2) { throw new Error(`AdvancedFilters may not have more than two conditions. You passed: ${conditions.length}`); } - if(extractedConditions.length === 1 && logicalOperator !== "And") { + if (extractedConditions.length === 1 && logicalOperator !== "And") { throw new Error(`Logical Operator must be "And" when there is only one condition provided`); } this.conditions = extractedConditions; } - + toJSON(): IAdvancedFilter { const filter = super.toJSON(); - + filter.logicalOperator = this.logicalOperator; filter.conditions = this.conditions; - + return filter; } -} \ No newline at end of file +} diff --git a/test/models.spec.ts b/test/models.spec.ts index 5d28b99..c17519b 100644 --- a/test/models.spec.ts +++ b/test/models.spec.ts @@ -396,7 +396,6 @@ describe("Unit | Filters", function () { } ]; - // Act const attemptToCreateFilter = () => { return new models.AdvancedFilter({ table: "Table", column: "c" }, "And", ...conditions); @@ -499,4 +498,4 @@ describe("Unit | Filters", function () { expect(models.getFilterType(testData.nonFilter)).toBe(models.FilterType.Unknown); }); }); -}); \ No newline at end of file +}); diff --git a/tsconfig.json b/tsconfig.json index 36d9554..06d4656 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,16 +1,16 @@ { - "compilerOptions": { - "target": "es5", - "declaration": true, - "sourceMap": true, - "noImplicitUseStrict": true, - "outDir": "dist" - }, - "exclude": [ - "node_modules", - "typings/index.d.ts", - "dist", - "test", - "tmp" - ] + "compilerOptions": { + "target": "es5", + "declaration": true, + "sourceMap": true, + "noImplicitUseStrict": true, + "outDir": "dist" + }, + "exclude": [ + "node_modules", + "typings/index.d.ts", + "dist", + "test", + "tmp" + ] } \ No newline at end of file diff --git a/tslint.json b/tslint.json new file mode 100644 index 0000000..6bc5a55 --- /dev/null +++ b/tslint.json @@ -0,0 +1,25 @@ +{ + "extends": "tslint:recommended", + "rules": { + "max-line-length": false, + "member-access": false, + "one-line": [ + "check-whitespace", + "check-open-brace" + ], + "quotemark": [ + "single", + "avoid-escape" + ], + "trailing-comma": false, + "whitespace": [ + "check-branch", + "check-decl", + "check-operator", + "check-module", + "check-seperator", + "check-type" + ], + "object-literal-sort-keys": false + } +} \ No newline at end of file diff --git a/webpack.test.tsconfig.json b/webpack.test.tsconfig.json index ab705ff..1d4b04c 100644 --- a/webpack.test.tsconfig.json +++ b/webpack.test.tsconfig.json @@ -1,14 +1,14 @@ { - "compilerOptions": { - "target": "es5", - "noImplicitAny": true, - "sourceMap": true - }, - "exclude": [ - "node_modules", - "typings/index.d.ts", - "dist", - "test", - "tmp" - ] + "compilerOptions": { + "target": "es5", + "noImplicitAny": true, + "sourceMap": true + }, + "exclude": [ + "node_modules", + "typings/index.d.ts", + "dist", + "test", + "tmp" + ] } \ No newline at end of file