Skip to content

ESlint 2.0 #92

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

Closed
wants to merge 36 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
06b6503
Monkey patch eslint, instead of modifying it
Feb 24, 2016
1f111fc
Use eslint-2.4.0
pbrisbin May 26, 2016
5ca5f43
Comment loadPackage override
pbrisbin May 26, 2016
483a24d
Update babel-eslint to v6
pbrisbin May 26, 2016
d1ff631
Update dependencies
dblandin May 27, 2016
70efbc9
Merge pull request #93 from codeclimate/devon/eslint-2
dblandin May 27, 2016
20b0ede
Add standard style plugin and shared config
dblandin Jun 2, 2016
5979e0f
COPY npm-shrinkwrap.json before npm install
dblandin Jun 2, 2016
95911b8
Merge pull request #96 from codeclimate/devon/eslint-2-add-standard-p…
dblandin Jun 2, 2016
88215de
Update CircleCI config to use patrick
dblandin Jun 2, 2016
bfaa029
Cleanup Dockerfile
dblandin Jun 2, 2016
e2dcac7
Merge pull request #98 from codeclimate/devon/pb-eslint-2-update-circ…
dblandin Jun 2, 2016
1b4f231
Vendor additional Standard Style config packages (#102)
dblandin Jun 20, 2016
beefd30
Add eslint-config-google as available plugin
pbrisbin Jul 15, 2016
df907bc
Merge pull request #104 from codeclimate/pb-google-config
pbrisbin Jul 15, 2016
833aa19
Added supporto for ESLint plugin Flowtype
leonardfactory Jul 21, 2016
31e828e
Merge pull request #106 from favoloso/eslint-2-flowtype-plugin
pbrisbin Jul 21, 2016
fe5dfac
Add task to update npm-shrinkwrap.json
pbrisbin Jul 21, 2016
9f2293d
Add support for eslint-config-angular
leftees Jul 21, 2016
2f5cabf
Merge pull request #107 from codeclimate/js-eslint
pbrisbin Jul 21, 2016
4952a9b
Update package.json (#112)
leftees Aug 2, 2016
870d031
Heuristically filter out minified files
Aug 11, 2016
187fb76
Merge pull request #114 from codeclimate/abh-wf-skip-minifieds
Aug 11, 2016
13dc1a5
add eslint-config-ember package
Aug 11, 2016
e18647f
Merge pull request #118 from aaroncraigongithub/eslint2/eslint-config…
pbrisbin Aug 12, 2016
7f3d77b
Switch base image to avoid Segfault
gdiggs Aug 15, 2016
3cb2fa8
Merge pull request #119 from codeclimate/gd-segfault
gdiggs Aug 15, 2016
5a524a5
Update babel-eslint & eslint-flowtype
Aug 16, 2016
0ffdd03
Merge pull request #121 from codeclimate/will/bump-babel-and-flowtype
wfleming Aug 16, 2016
7b4c106
Code cleanup
GarthDB Aug 1, 2016
a66ec7e
Support for ignore_path in engine config
GarthDB Jul 29, 2016
9da846b
Add --gecos "" to avoid adduser prompt
pbrisbin Aug 17, 2016
54e0d4e
add hapi config support
idoshamun Aug 21, 2016
71f21cf
Merge pull request #124 from elegantmonkeys/pb-eslint-2
Aug 22, 2016
49a2b84
Add eslint-config-airbnb-base as a top-level dep
chadxz Aug 30, 2016
5018915
Merge pull request #126 from chadxz/eslint-config-airbnb-base
Aug 30, 2016
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .codeclimate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ engines:
ratings:
paths:
- bin/eslint.js
- lib/checks.js
- lib/*.js
exclude_paths:
- "node_modules/**"
- "test/**"
4 changes: 3 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"env": {
"node": true
"node": true,
"mocha": true
},
"rules": {
"block-spacing": 2,
"brace-style": [2, "1tbs", { "allowSingleLine": true }],
"comma-dangle": [2, "never"],
"comma-style": [2, "first", { exceptions: {ArrayExpression: true, ObjectExpression: true} }],
Expand Down
21 changes: 17 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,17 +1,30 @@
FROM mhart/alpine-node:5.4
FROM node:6.3.1-slim
MAINTAINER Code Climate <hello@codeclimate.com>

WORKDIR /usr/src/app
COPY npm-shrinkwrap.json /usr/src/app/
COPY package.json /usr/src/app/

RUN apk --update add git && \
RUN apt-get update && \
apt-get install -y git jq && \
npm install && \
apk del --purge git
git clone https://github.com/eslint/eslint.git && \
ESLINT_DOCS_VERSION=`npm -j ls eslint | jq -r .dependencies.eslint.version` && \
cd eslint && \
git checkout v$ESLINT_DOCS_VERSION && \
cd .. && \
mkdir -p /usr/src/app/lib/docs/rules/ && \
cp ./eslint/docs/rules/* /usr/src/app/lib/docs/rules/ && \
rm -rf eslint && \
apt-get purge -y git jq && \
apt-get autoremove -y

RUN adduser -u 9000 --gecos "" --disabled-password app
COPY . /usr/src/app
RUN chown -R app:app /usr/src/app

RUN adduser -u 9000 -D app
USER app

VOLUME /code
WORKDIR /code

Expand Down
11 changes: 9 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
.PHONY: image test
.PHONY: image test citest shrinkwrap

IMAGE_NAME ?= codeclimate/codeclimate-eslint

image:
docker build --rm -t $(IMAGE_NAME) .

test: image
docker run --rm --workdir="/usr/src/app" $(IMAGE_NAME) npm run test
docker run --rm $(IMAGE_NAME) sh -c "cd /usr/src/app && npm run test"

citest:
docker run --rm $(IMAGE_NAME) sh -c "cd /usr/src/app && npm run test"

shrinkwrap: image
docker run --rm --workdir /usr/src/app $(IMAGE_NAME) sh -c \
'npm shrinkwrap >/dev/null && cat npm-shrinkwrap.json' > npm-shrinkwrap.json
27 changes: 19 additions & 8 deletions bin/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ process.chdir(CODE_DIR);
var stdout = console.log;
console.log = console.error;

var CLIEngine = require("eslint").CLIEngine;
var docs = require("eslint").docs;
var eslint = require('../lib/eslint-patch')(require('eslint'));

var BatchSanitizer = require("../lib/batch_sanitizer");
var CLIEngine = eslint.CLIEngine;
var docs = eslint.docs;
var fs = require("fs");
var glob = require("glob");
var options = { extensions: [".js"], ignore: true, reset: false, useEslintrc: true };
Expand Down Expand Up @@ -43,6 +46,8 @@ function buildIssueJson(message, path) {
if(message.fatal) {
checkName = "fatal";
}
var line = message.line || 1;
var column = message.column || 1;

var issue = {
type: "issue",
Expand All @@ -56,12 +61,12 @@ function buildIssueJson(message, path) {
path: path,
positions: {
begin: {
line: message.line || 1,
column: message.column || 1
line: line,
column: column
},
end: {
line: message.line || 1,
column: message.column || 1
line: line,
column: column
}
}
},
Expand Down Expand Up @@ -165,6 +170,10 @@ runWithTiming("engineConfig", function () {
options.extensions = userConfig.extensions;
}

if (userConfig.ignore_path) {
options.ignorePath = userConfig.ignore_path;
}

if (userConfig.debug) {
debug = true;
}
Expand All @@ -181,17 +190,19 @@ function analyzeFiles() {
var batchNum = 0
, batchSize = 10
, batchFiles
, batchReport;
, batchReport
, sanitizedBatchFiles;

while(analysisFiles.length > 0) {
batchFiles = analysisFiles.splice(0, batchSize);
sanitizedBatchFiles = (new BatchSanitizer(batchFiles)).sanitizedFiles();

if (debug) {
process.stderr.write("Analyzing: " + batchFiles + "\n");
}

runWithTiming("analyze-batch-" + batchNum, function() {
batchReport = cli.executeOnFiles(batchFiles);
batchReport = cli.executeOnFiles(sanitizedBatchFiles);
});
runWithTiming("report-batch" + batchNum, function() {
batchReport.results.forEach(function(result) {
Expand Down
31 changes: 20 additions & 11 deletions circle.yml
Original file line number Diff line number Diff line change
@@ -1,25 +1,34 @@
machine:
services:
- docker
environment:
CLOUDSDK_CORE_DISABLE_PROMPTS: 1
PRIVATE_REGISTRY: us.gcr.io/code_climate
image_name: codeclimate-eslint

dependencies:
override:
- >
docker run
--env CIRCLE_BRANCH
--env CIRCLE_PROJECT_REPONAME
--env CIRCLE_TOKEN
--env GCR_JSON_KEY
--volume /var/run/docker.sock:/var/run/docker.sock
codeclimate/patrick pull || true
- make image

test:
override:
- IMAGE_NAME="$PRIVATE_REGISTRY/$CIRCLE_PROJECT_REPONAME:b$CIRCLE_BUILD_NUM" make test
- make citest

deployment:
registry:
branch: master
owner: codeclimate
commands:
- echo $gcloud_json_key_base64 | sed 's/ //g' | base64 -d > /tmp/gcloud_key.json
- curl https://sdk.cloud.google.com | bash
- gcloud auth activate-service-account $gcloud_account_email --key-file /tmp/gcloud_key.json
- gcloud docker -a
- docker push $PRIVATE_REGISTRY/$CIRCLE_PROJECT_REPONAME:b$CIRCLE_BUILD_NUM
- >
docker run
--env CIRCLE_BUILD_NUM
--env CIRCLE_PROJECT_REPONAME
--env GCR_JSON_KEY
--volume /var/run/docker.sock:/var/run/docker.sock
codeclimate/patrick push gcr

notify:
webhooks:
Expand Down
41 changes: 41 additions & 0 deletions lib/batch_sanitizer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
var fs = require("fs");

var MINIFIED_AVG_LINE_LENGTH_CUTOFF = 100;

function BatchSanitizer(files, stderr) {
this.files = files;
this.stderr = stderr || process.stderr;
}

BatchSanitizer.prototype.sanitizedFiles = function() {
var sanitizedFiles = [];

for(var i = 0; i < this.files.length; i++) {
if (this.isMinified(this.files[i])) {
this.stderr.write("WARN: Skipping " + this.files[i] + ": it appears to be minified\n");
} else {
sanitizedFiles.push(this.files[i]);
}
}

return sanitizedFiles;
};

BatchSanitizer.prototype.isMinified = function(path) {
var buf = fs.readFileSync(path)
, newline = "\n".charCodeAt(0)
, lineCount = 0
, charsSeen = 0;

for(var i = 0; i < buf.length; i++) {
if (buf[i] === newline) {
lineCount++;
} else {
charsSeen++;
}
}

return (charsSeen / lineCount) >= MINIFIED_AVG_LINE_LENGTH_CUTOFF;
};

module.exports = BatchSanitizer;
40 changes: 40 additions & 0 deletions lib/docs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* @fileoverview Providing easy access to rule documentation
*/

'use strict';

var fs = require('fs')
, path = require('path');

//------------------------------------------------------------------------------
// Privates
//------------------------------------------------------------------------------

var docs = Object.create(null);

function load() {
var docsDir = path.join(__dirname, '/docs/rules');

fs.readdirSync(docsDir).forEach(function(file) {
var content = fs.readFileSync(docsDir + '/' + file, 'utf8');

// Remove the .md extension from the filename
docs[file.slice(0, -3)] = content;
});
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

exports.get = function(ruleId) {
return docs[ruleId];
};

//------------------------------------------------------------------------------
// Initialization
//------------------------------------------------------------------------------

// loads existing docs
load();
33 changes: 33 additions & 0 deletions lib/eslint-patch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';
var meld = require('meld');
var docs = require('./docs');

var supportedPlugins = ['react', 'babel'];

module.exports = function patcher(eslint) {

meld.around(eslint.CLIEngine, 'loadPlugins', function(joinPoint) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this patch working as intended? I arrived here because I'm seeing Failed to load plugin errors for an unsupported plugin, but I think this patch is intended to avoid loading unsupported plugins?

I'm questioning whether this does anything because it appears to me that eslint.CLIEngine has not had a loadPlugins method since this commit, and that commit has been around since ESLint 2.0.0?

Copy link
Contributor Author

@pbrisbin pbrisbin Aug 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is not working as intended. It was at the time it was written, but (as you noted) that's no longer the case given the current internals of ESLint.

We're evaluating if we want to accept unsupported plugins causing errors like that, or try to bring back the logic intended to replace that with ignoring them. Unfortunately, this meld approach doesn't appear viable on the new internals, so it would require moving back to our own fork of ESLint itself.

In the meantime though, we should probably remove this.

EDIT: It may not have even worked when it was written. The history is uncertain here.

var pluginNames = joinPoint.args[0];
var filteredPluginNames = pluginNames.filter(function(pluginName) {
return supportedPlugins.indexOf(pluginName) >= 0;
});
return joinPoint.proceed(filteredPluginNames);
});

meld.around(eslint.CLIEngine, 'addPlugin', function() {
return;
});

// meld.around(eslint.CLIEngine.Config, 'loadPackage', function(joinPoint) {
// var filePath = joinPoint.args[0];
// if (filePath.match(/^eslint-config-airbnb.*/)) {
// return joinPoint.proceed();
// } else {
// return {};
// }
// });

eslint.docs = docs;

return eslint;
};
Loading