From b0fa7ee2d1f9d8b8605c74d01aee10f0d0c3bb7c Mon Sep 17 00:00:00 2001 From: Alex Kocharin Date: Mon, 23 Dec 2013 04:14:57 +0400 Subject: [PATCH] using eslint to check the code --- .eslint.js | 264 +++++++++++++++++++++++++++++++++++++++++++ lib/cli.js | 32 +++--- lib/local-storage.js | 4 +- lib/logger.js | 22 ++-- lib/storage.js | 4 +- lib/streams.js | 13 --- lib/up-storage.js | 23 ++-- lib/utils.js | 8 +- package.yaml | 7 ++ 9 files changed, 317 insertions(+), 60 deletions(-) create mode 100644 .eslint.js diff --git a/.eslint.js b/.eslint.js new file mode 100644 index 000000000..cb109e333 --- /dev/null +++ b/.eslint.js @@ -0,0 +1,264 @@ +module.exports = { + env: { + node: true, + }, + + /* + * 0 - disable + * Rules that more harmful than useful, or just buggy. + * + * 1 - warning + * Rules that we didn't encounter yet. You can safely ignore them, + * but I'd like to know any interesting use-cases they forbid. + * + * 2 - error + * Rules that have proven to be useful, please follow them. + */ + + rules: { + // didn't understand what it does, but it fails a good code + 'block-scoped-var': 0, + + // fails where newlines are used to format pretty big "if": + // if ( + // name.charAt(0) === "." || + // name.match(/[\/@\s\+%:]/) || + // name !== encodeURIComponent(name) || + // name.toLowerCase() === "node_modules" + // ) { + 'brace-style': 0, + + // snake_case is more readable, what's up with you guys? + 'camelcase': 0, + + // if some functions are complex, they are for a good reason, + // ain't worth it + 'complexity': [0, 10], + + // never saw it, but self is preferred + 'consistent-this': [1, 'self'], + + // fails good code + 'curly': 0, + + // fails good code, where this notation is used for consistency: + // something['foo-bar'] = 123 + // something['blahblah'] = 234 + 'dot-notation': 0, + + // pointless in many cases (like indexOf() == -1), harmful in a few + // cases (when you do want to ignore types), fails good code + 'eqeqeq': 0, + + // if someone is changing prototype and makes properties enumerable, + // it's their own fault + 'guard-for-in': 0, + + // if some functions are complex, they are for a good reason, + // ain't worth it + 'max-depth': [0, 4], + + // should it really throw for every long URL? + 'max-len': [0, 80, 4], + + // that's obvious by just looking at the code, you don't need lint for that + 'max-params': [0, 3], + + // if some functions are complex, they are for a good reason, + // ain't worth it + 'max-statements': [0, 10], + + // that one makes sense + 'new-cap': 2, + + 'new-parens': 1, + 'no-alert': 1, + + // I'm writing javascript, not some weird reduced version of it + 'no-bitwise': 0, + + 'no-caller': 1, + + // not working around IE bugs, sorry + 'no-catch-shadow': 0, + + // see above, IE is useful for downloading other browsers only + 'no-comma-dangle': 0, + + 'no-cond-assign': 1, + + // good for removing debugging code + 'no-console': 2, + + 'no-control-regex': 1, + + // good for removing debugging code + 'no-debugger': 2, + + 'no-delete-var': 1, + 'no-div-regex': 1, + 'no-dupe-keys': 1, + + // why would anyone need to check against that? + 'no-else-return': 0, + + // sometimes empty statement contains useful comment + 'no-empty': 0, + + 'no-empty-class': 1, + 'no-empty-label': 1, + + // stupid rule + // "x == null" is "x === null || x === undefined" + 'no-eq-null': 0, + + 'no-eval': 1, + 'no-ex-assign': 1, + 'no-extend-native': 1, + + // fails good code, when parens are used for grouping: + // (req && req.headers['via']) ? req.headers['via'] + ', ' : '' + // not everyone remembers priority tables, you know + 'no-extra-parens': 0, + + // fails defensive semicolons: + // ;['foo', 'bar'].forEach(function(x) {}) + 'no-extra-semi': 0, + + 'no-fallthrough': 1, + 'no-floating-decimal': 1, + 'no-func-assign': 1, + 'no-global-strict': 1, + 'no-implied-eval': 1, + 'no-invalid-regexp': 1, + 'no-iterator': 1, + 'no-label-var': 1, + 'no-loop-func': 1, + + // fails good code: + // var fs = require('fs'), + // , open = fs.open + 'no-mixed-requires': [0, false], + + 'no-multi-str': 1, + 'no-native-reassign': 1, + 'no-negated-in-lhs': 1, + + // XXX: not released yet + //'no-nested-ternary': 1, + + 'no-new': 1, + + // new Array(12) is used to pre-allocate arrays + 'no-new-array': 0, + + 'no-new-func': 1, + 'no-new-object': 1, + 'no-new-wrappers': 1, + 'no-obj-calls': 1, + + // fails good code: + // fs.open('/file', 0666, function(){}) + 'no-octal': 0, + + // fails good code: + // console.log('\033[31m' + str + '\033[39m') + // also fails \0 which is not octal escape + 'no-octal-escape': 0, + + // I'm writing javascript, not some weird reduced version of it + 'no-plusplus': 0, + + 'no-proto': 1, + + // fails good code: + // if (a) { + // var x = 'foo' + // } else { + // var x = bar + // } + 'no-redeclare': 0, + + 'no-return-assign': 1, + 'no-script-url': 1, + 'no-self-compare': 1, + + // sometimes useful, often isn't + // probably worth enforcing + 'no-shadow': 2, + + 'no-shadow-restricted-names': 1, + 'no-spaced-func': 1, + + // can't agree more, but it's a task for code review, not for lint + 'no-sync': 0, + + // I'm writing javascript, not some weird reduced version of it + 'no-ternary': 0, + + // the single most important rule in the entire ruleset + 'no-undef': 2, + + 'no-undef-init': 1, + + // it is failing our own underscores + 'no-underscore-dangle': 0, + + // fails function hoisting + 'no-unreachable': 0, + + // fails npm-style code, it's good once you get used to it: + // if (typeof(options) === 'function') callback = options, options = {} + 'no-unused-expressions': 0, + + // fails (function(_err) {}) where named argument is used to show what + // nth function argument means + 'no-unused-vars': 0, + + // fails function hoisting + 'no-use-before-define': 0, + + 'no-with': 1, + + // fails foobar( (function(){}).bind(this) ) + // parens are added for readability + 'no-wrap-func': 0, + + // fails good code: + // var x + // if (something) { + // var y + 'one-var': 0, + + // the most stupid rule I ever saw + 'quote-props': 0, + + // fails situation when different quotes are used to avoid escaping + 'quotes': [0, 'single'], + + 'radix': 1, + 'regex-spaces': 1, + + // http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding + 'semi': 0, + + // fails good code where spaces are used for grouping: + // (x+y * y+z) + 'space-infix-ops': 0, + + 'space-return-throw-case': 1, + + // typeof(something) should have braces to look like a function + // a matter of taste I suppose + 'space-unary-word-ops': 0, + + // strict mode is just harmful, + // can I have a check to enforce not using it? + 'strict': 0, + + 'unnecessary-strict': 1, + 'use-isnan': 1, + 'wrap-iife': 1, + 'wrap-regex': 1, + }, +} diff --git a/lib/cli.js b/lib/cli.js index 2d1626717..408678c7b 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -1,7 +1,7 @@ #!/usr/bin/env node if (process.getuid() === 0) { - console.error("Sinopia doesn't need superuser privileges. Don't run it under root.") + global.console.error("Sinopia doesn't need superuser privileges. Don't run it under root.") } var logger = require('./logger') @@ -43,7 +43,7 @@ try { var readline = require('readline') var rl = readline.createInterface(process.stdin, process.stdout) var timeout = setTimeout(function() { - console.log('I got tired waiting for an answer. Exitting...') + global.console.log('I got tired waiting for an answer. Exitting...') process.exit(1) }, 20000) @@ -61,7 +61,7 @@ try { afterConfigLoad() } else if (x[0] == 'N' || x[0] == 'n') { rl.close() - console.log('So, you just accidentally run me in a wrong folder. Exitting...') + global.console.log('So, you just accidentally run me in a wrong folder. Exitting...') process.exit(1) } else { askUser() @@ -115,18 +115,20 @@ function afterConfigLoad() { function write_config_banner(def, config) { var hostport = get_hostport() - console.log('===========================================================') - console.log(' Creating a new configuration file: "%s"', config_path) - console.log(' ') - console.log(' If you want to setup npm to work with this registry,') - console.log(' run following commands:') - console.log(' ') - console.log(' $ npm set registry http://%s:%s/', hostport[0], hostport[1]) - console.log(' $ npm set always-auth true') - console.log(' $ npm adduser') - console.log(' Username: %s', def.user) - console.log(' Password: %s', def.pass) - console.log('===========================================================') + var log = global.console.log + + log('===========================================================') + log(' Creating a new configuration file: "%s"', config_path) + log(' ') + log(' If you want to setup npm to work with this registry,') + log(' run following commands:') + log(' ') + log(' $ npm set registry http://%s:%s/', hostport[0], hostport[1]) + log(' $ npm set always-auth true') + log(' $ npm adduser') + log(' Username: %s', def.user) + log(' Password: %s', def.pass) + log('===========================================================') } process.on('uncaughtException', function(err) { diff --git a/lib/local-storage.js b/lib/local-storage.js index d5c59a1bf..b922802b0 100644 --- a/lib/local-storage.js +++ b/lib/local-storage.js @@ -2,7 +2,7 @@ var fs = require('fs') , Path = require('path') , crypto = require('crypto') , assert = require('assert') - , fs_storage = require('./local-fs') + , FS_Storage = require('./local-fs') , UError = require('./error').UserError , utils = require('./utils') , mystreams = require('./streams') @@ -17,7 +17,7 @@ function Storage(config) { if (!(this instanceof Storage)) return new Storage(config) this.config = config var path = Path.resolve(Path.dirname(this.config.self_path), this.config.storage) - this.storage = new fs_storage(path) + this.storage = new FS_Storage(path) this.logger = Logger.logger.child({sub: 'fs'}) return this } diff --git a/lib/logger.js b/lib/logger.js index ff28846f7..a5984f859 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -3,20 +3,14 @@ var Logger = require('bunyan') , utils = require('./utils') function getlvl(x) { - if (x < 15) { - return 'trace' - } else if (x < 25) { - return 'debug' - } else if (x < 35) { - return 'info' - } else if (x == 35) { - return 'http' - } else if (x < 45) { - return 'warn' - } else if (x < 55) { - return 'error' - } else { - return 'fatal' + switch(true) { + case x < 15: return 'trace' + case x < 25: return 'debug' + case x < 35: return 'info' + case x == 35: return 'http' + case x < 45: return 'warn' + case x < 55: return 'error' + default: return 'fatal' } } diff --git a/lib/storage.js b/lib/storage.js index 9b0da3eaa..8f376ffed 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -426,14 +426,14 @@ Storage.prototype.get_package = function(name, options, callback) { self.local.get_package(name, options, function(err, data) { if (err && (!err.status || err.status >= 500)) { // report internal errors right away - return cb(err) + return callback(err) } self._sync_package_with_uplinks(name, data, options, function(err, result, uplink_errors) { if (err) return callback(err) var whitelist = ['_rev', 'name', 'versions', 'dist-tags'] for (var i in result) { - if (!~whitelist.indexOf(i)) delete result[i] + if (whitelist.indexOf(i) === -1) delete result[i] } result['dist-tags'].latest = Storage._semver_sort(Object.keys(result.versions)) diff --git a/lib/streams.js b/lib/streams.js index 5c8d74b66..13f6ab417 100644 --- a/lib/streams.js +++ b/lib/streams.js @@ -51,16 +51,3 @@ function add_abstract_method(self, name) { }) } -function __test() { - var test = new ReadTarball() - test.abort() - setTimeout(function() { - test.abort = function() { - console.log('ok') - } - test.abort = function() { - throw 'fail' - } - }, 100) -} - diff --git a/lib/up-storage.js b/lib/up-storage.js index f3a72fe14..0284330ba 100644 --- a/lib/up-storage.js +++ b/lib/up-storage.js @@ -1,10 +1,11 @@ var URL = require('url') , request = require('request') - , stream = require('stream') + , Stream = require('stream') , UError = require('./error').UserError , mystreams = require('./streams') , Logger = require('./logger') , utils = require('./utils') + , encode = encodeURIComponent // // Implements Storage interface @@ -67,7 +68,8 @@ function _setupProxy(hostname, config, mainconfig, isHTTPS) { if (no_proxy_item[0] !== '.') no_proxy_item = '.' + no_proxy_item if (hostname.lastIndexOf(no_proxy_item) === hostname.length - no_proxy_item.length) { if (this.proxy) { - this.logger.debug({url: this.url.href, rule: no_proxy_item}, 'not using proxy for @{url}, excluded by @{rule} rule') + this.logger.debug({url: this.url.href, rule: no_proxy_item}, + 'not using proxy for @{url}, excluded by @{rule} rule') this.proxy = false } break @@ -79,13 +81,14 @@ function _setupProxy(hostname, config, mainconfig, isHTTPS) { if (typeof(this.proxy) !== 'string') { delete this.proxy } else { - this.logger.debug({url: this.url.href, proxy: this.proxy}, 'using proxy @{proxy} for @{url}') + this.logger.debug({url: this.url.href, proxy: this.proxy}, + 'using proxy @{proxy} for @{url}') } } Storage.prototype.request = function(options, cb) { if (!this.status_check()) { - var req = new stream.Readable() + var req = new Stream.Readable() process.nextTick(function() { if (typeof(cb) === 'function') cb(new Error('uplink is offline')) req.emit('error', new Error('uplink is offline')) @@ -177,11 +180,11 @@ Storage.prototype.status_check = function(alive) { if (arguments.length === 0) { return true // hold off this feature until v0.6.0 - if (!this.is_alive && Math.abs(Date.now() - this.is_alive_time) < 2*60*1000) { +/* if (!this.is_alive && Math.abs(Date.now() - this.is_alive_time) < 2*60*1000) { return false } else { return true - } + }*/ } else { if (this.is_alive && !alive) { this.logger.warn({host: this.url.host}, 'host @{host} is now offline') @@ -206,7 +209,7 @@ Storage.prototype.add_package = function(name, metadata, options, callback) { if (typeof(options) === 'function') callback = options, options = {} this.request({ - uri: '/' + escape(name), + uri: '/' + encode(name), method: 'PUT', json: metadata, }, function(err, res, body) { @@ -222,7 +225,7 @@ Storage.prototype.add_version = function(name, version, metadata, tag, options, if (typeof(options) === 'function') callback = options, options = {} this.request({ - uri: '/' + escape(name) + '/' + escape(version) + '/-tag/' + escape(tag), + uri: '/' + encode(name) + '/' + encode(version) + '/-tag/' + encode(tag), method: 'PUT', json: metadata, }, function(err, res, body) { @@ -241,7 +244,7 @@ Storage.prototype.add_tarball = function(name, filename, options) { , self = this var wstream = this.request({ - uri: '/' + escape(name) + '/-/' + escape(filename) + '/whatever', + uri: '/' + encode(name) + '/-/' + encode(filename) + '/whatever', method: 'PUT', headers: { 'Content-Type': 'application/octet-stream' @@ -286,7 +289,7 @@ Storage.prototype.get_package = function(name, options, callback) { this._add_proxy_headers(options.req, headers) this.request({ - uri: '/' + escape(name), + uri: '/' + encode(name), json: true, headers: headers, }, function(err, res, body) { diff --git a/lib/utils.js b/lib/utils.js index 271343b1a..f3e4d52d9 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -79,10 +79,10 @@ module.exports.filter_tarball_urls = function(pkg, req, config) { } for (var ver in pkg.versions) { - if (pkg.versions[ver].dist != null - && pkg.versions[ver].dist.tarball != null) { - pkg.versions[ver].dist.__sinopia_orig_tarball = pkg.versions[ver].dist.tarball - pkg.versions[ver].dist.tarball = filter(pkg.versions[ver].dist.tarball) + var dist = pkg.versions[ver].dist + if (dist != null && dist.tarball != null) { + dist.__sinopia_orig_tarball = dist.tarball + dist.tarball = filter(dist.tarball) } } return pkg diff --git a/package.yaml b/package.yaml index de5bdeb28..ea84ec7f2 100644 --- a/package.yaml +++ b/package.yaml @@ -33,6 +33,10 @@ devDependencies: rimraf: '*' mocha: '*' + # linting tools + eslint: '*' + #eslint-stylish: '*' + keywords: - private - package @@ -44,6 +48,7 @@ keywords: scripts: test: mocha ./test/functional ./test/unit + lint: eslint -c ./.eslint.js ./lib # we depend on streams2 stuff # it can be replaced with isaacs/readable-stream, ask if you need to use 0.8 @@ -51,5 +56,7 @@ engines: node: '>=0.10' preferGlobal: true + +# http://www.wtfpl.net/txt/copying/ license: WTFPL