From 6ce09af64034877c156901b8d2455d3d2068dafa Mon Sep 17 00:00:00 2001 From: Alexander Mankuta Date: Fri, 15 Dec 2017 16:11:50 +0200 Subject: [PATCH 1/2] Rerwrite in JavaScript We-re maintaining our own fork of CSSLink because we need to make some changes to make the output compatible with the Spec. This makes CSSLint updates hard and as a result the engine lags behind the upstream. CSSLint provides APIs that we can use directly to augment output but they're only exposed in JavaScript. This rewrite is targeted towards using the upstream CSSLint and only maintaining the custom parts that in the engine. This should make CSSLint updates trivial as long as the API stays stable. As a bonus it drops Ruby from dependencies making the overal image smaller. --- .codeclimate.yml | 9 +- .dockerignore | 1 + .eslintrc | 32 +++ .gitignore | 1 + .rubocop.yml | 41 --- .ruby-version | 1 - Dockerfile | 17 +- Gemfile | 10 - Gemfile.lock | 44 --- Makefile | 2 +- Rakefile | 5 - bin/csslint | 18 +- bin/csslint.rb | 14 + lib/cc/engine/csslint.rb | 92 ------- lib/cc/engine/csslint/check_details.rb | 54 ---- lib/check-details.js | 44 +++ lib/codeclimate-formatter.js | 78 ++++++ lib/csslint.js | 120 ++++++++ package.json | 25 ++ spec/cc/engine/csslint/check_details_spec.rb | 21 -- spec/cc/engine/csslint_spec.rb | 106 -------- spec/spec_helper.rb | 2 - test/check-details-test.js | 18 ++ test/codeclimate-formatter_test.js | 54 ++++ test/csslint-test.js | 127 +++++++++ yarn.lock | 272 +++++++++++++++++++ 26 files changed, 806 insertions(+), 402 deletions(-) create mode 100644 .eslintrc create mode 100644 .gitignore delete mode 100644 .rubocop.yml delete mode 100644 .ruby-version delete mode 100644 Gemfile delete mode 100644 Gemfile.lock delete mode 100644 Rakefile create mode 100755 bin/csslint.rb delete mode 100644 lib/cc/engine/csslint.rb delete mode 100644 lib/cc/engine/csslint/check_details.rb create mode 100644 lib/check-details.js create mode 100644 lib/codeclimate-formatter.js create mode 100644 lib/csslint.js create mode 100644 package.json delete mode 100644 spec/cc/engine/csslint/check_details_spec.rb delete mode 100644 spec/cc/engine/csslint_spec.rb delete mode 100644 spec/spec_helper.rb create mode 100644 test/check-details-test.js create mode 100644 test/codeclimate-formatter_test.js create mode 100644 test/csslint-test.js create mode 100644 yarn.lock diff --git a/.codeclimate.yml b/.codeclimate.yml index 1b449f2..42e43fe 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,8 +1,7 @@ engines: - rubocop: + eslint: enabled: true -ratings: - paths: - - "**.rb" + channel: eslint-4 exclude_paths: - - spec/**/* + - test/**/* + - node_modules/**/* diff --git a/.dockerignore b/.dockerignore index cf6ce25..01d2a07 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,2 +1,3 @@ .git Makefile +node_modules diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 0000000..63664eb --- /dev/null +++ b/.eslintrc @@ -0,0 +1,32 @@ +{ + "env": { + "node": true, + "mocha": true, + "es6": true + }, + "rules": { + "block-spacing": 2, + "brace-style": [2, "1tbs", { "allowSingleLine": true }], + "comma-dangle": [2, "never"], + "comma-style": [2, "first", { exceptions: {ArrayExpression: true, ObjectExpression: true} }], + "complexity": [2, 6], + "curly": 2, + "eqeqeq": [2, "allow-null"], + "max-statements": [2, 30], + "no-shadow-restricted-names": 2, + "no-undef": 2, + "no-use-before-define": 2, + "radix": 2, + "semi": 2, + "space-infix-ops": 2, + "strict": 0 + }, + "globals": { + "AnalysisView": true, + "PollingView": true, + "Prism": true, + "Spinner": true, + "Timer": true, + "moment": true + } +} diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..c2658d7 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +node_modules/ diff --git a/.rubocop.yml b/.rubocop.yml deleted file mode 100644 index 6583489..0000000 --- a/.rubocop.yml +++ /dev/null @@ -1,41 +0,0 @@ -Style/StringLiterals: - Enabled: false - -Style/Documentation: - Enabled: false - -Metrics/LineLength: - Enabled: false - -Style/TrailingComma: - Enabled: false - -Style/FileName: - Exclude: - - 'bin/**/*' - -Style/ClassAndModuleChildren: - Exclude: - - 'spec/**/*' - -Metrics/ModuleLength: - Exclude: - - 'spec/**/*' - -Style/GuardClause: - Enabled: false - -Style/IfUnlessModifier: - Enabled: false - -Style/DotPosition: - Enabled: false - -Style/SignalException: - Enabled: false - -Metrics/AbcSize: - Enabled: false - -Rails/TimeZone: - Enabled: false diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 5859406..0000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -2.2.3 diff --git a/Dockerfile b/Dockerfile index 950153c..b3c90a9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,17 +1,16 @@ -FROM codeclimate/alpine-ruby:b38 +FROM node:alpine + +RUN adduser -u 9000 -D app WORKDIR /usr/src/app -COPY Gemfile /usr/src/app/ -COPY Gemfile.lock /usr/src/app/ -RUN apk --update add nodejs git zlib zlib-dev ruby ruby-dev ruby-bundler less build-base && \ - bundle install -j 4 && \ - apk del --purge build-base zlib zlib-dev && rm -fr /usr/share/ri +COPY package.json yarn.lock ./ -ENV CSSLINT_SHA=87aa604a4cbc5125db979576f1b09b35980fcf08 -RUN npm install -g codeclimate/csslint.git#$CSSLINT_SHA +RUN yarn install && \ + chown -R app:app ./ + +COPY . ./ -RUN adduser -u 9000 -D app USER app COPY . /usr/src/app diff --git a/Gemfile b/Gemfile deleted file mode 100644 index 76ec3ca..0000000 --- a/Gemfile +++ /dev/null @@ -1,10 +0,0 @@ -source "https://rubygems.org" - -gem 'json' -gem 'nokogiri' -gem "pry" - -group :test do - gem "rake" - gem "rspec" -end diff --git a/Gemfile.lock b/Gemfile.lock deleted file mode 100644 index 99ebd75..0000000 --- a/Gemfile.lock +++ /dev/null @@ -1,44 +0,0 @@ -GEM - remote: https://rubygems.org/ - specs: - coderay (1.1.0) - diff-lcs (1.2.5) - json (1.8.3) - method_source (0.8.2) - mini_portile2 (2.1.0) - nokogiri (1.6.8) - mini_portile2 (~> 2.1.0) - pkg-config (~> 1.1.7) - pkg-config (1.1.7) - pry (0.10.1) - coderay (~> 1.1.0) - method_source (~> 0.8.1) - slop (~> 3.4) - rake (10.4.2) - rspec (3.2.0) - rspec-core (~> 3.2.0) - rspec-expectations (~> 3.2.0) - rspec-mocks (~> 3.2.0) - rspec-core (3.2.3) - rspec-support (~> 3.2.0) - rspec-expectations (3.2.1) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.2.0) - rspec-mocks (3.2.1) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.2.0) - rspec-support (3.2.2) - slop (3.6.0) - -PLATFORMS - ruby - -DEPENDENCIES - json - nokogiri - pry - rake - rspec - -BUNDLED WITH - 1.10.6 diff --git a/Makefile b/Makefile index cb4fb3e..f83f495 100644 --- a/Makefile +++ b/Makefile @@ -4,4 +4,4 @@ image: docker build -t codeclimate/codeclimate-csslint . test: image - docker run --rm codeclimate/codeclimate-csslint rspec $(RSPEC_ARGS) + docker run --rm codeclimate/codeclimate-csslint npm run test diff --git a/Rakefile b/Rakefile deleted file mode 100644 index 8281788..0000000 --- a/Rakefile +++ /dev/null @@ -1,5 +0,0 @@ -require "rspec/core/rake_task" - -RSpec::Core::RakeTask.new(:spec) - -task default: :spec diff --git a/bin/csslint b/bin/csslint index 8b2417d..d9c7377 100755 --- a/bin/csslint +++ b/bin/csslint @@ -1,14 +1,10 @@ -#!/usr/bin/env ruby -$LOAD_PATH.unshift(File.expand_path(File.join(File.dirname(__FILE__), "../lib"))) +#!/usr/local/bin/node --expose-gc -require 'cc/engine/csslint' +const fs = require("fs"); +const Engine = require("../lib/csslint"); -if File.exists?("/config.json") - engine_config = JSON.parse(File.read("/config.json")) -else - engine_config = {} -end +const CONFIG_PATH = "/config.json"; +let config = JSON.parse(fs.readFileSync(CONFIG_PATH)); -CC::Engine::CSSlint.new( - directory: "/code", engine_config: engine_config, io: STDOUT -).run +const CODE_DIR = "/code"; +new Engine(CODE_DIR, console, config).run(); diff --git a/bin/csslint.rb b/bin/csslint.rb new file mode 100755 index 0000000..8b2417d --- /dev/null +++ b/bin/csslint.rb @@ -0,0 +1,14 @@ +#!/usr/bin/env ruby +$LOAD_PATH.unshift(File.expand_path(File.join(File.dirname(__FILE__), "../lib"))) + +require 'cc/engine/csslint' + +if File.exists?("/config.json") + engine_config = JSON.parse(File.read("/config.json")) +else + engine_config = {} +end + +CC::Engine::CSSlint.new( + directory: "/code", engine_config: engine_config, io: STDOUT +).run diff --git a/lib/cc/engine/csslint.rb b/lib/cc/engine/csslint.rb deleted file mode 100644 index fbe3c4d..0000000 --- a/lib/cc/engine/csslint.rb +++ /dev/null @@ -1,92 +0,0 @@ -require "json" -require "nokogiri" -require "ostruct" -require "shellwords" - -module CC - module Engine - MissingAttributesError = Class.new(StandardError) - - DEFAULT_IDENTIFIER = OpenStruct.new(value: "parse-error") - - class CSSlint - DEFAULT_EXTENSIONS = [".css"].freeze - - autoload :CheckDetails, "cc/engine/csslint/check_details" - - def initialize(directory: , io: , engine_config: ) - @directory = directory - @engine_config = engine_config - @io = io - end - - def run - Dir.chdir(@directory) do - results.xpath('//file').each do |file| - path = file['name'].sub(/\A#{@directory}\//, '') - file.children.each do |node| - next unless node.name == "error" - issue = create_issue(node, path) - puts("#{issue.to_json}\0") - end - end - end - end - - private - - attr_reader :engine_config - - # rubocop:disable Metrics/MethodLength - def create_issue(node, path) - check_name = node.attributes.fetch("identifier", DEFAULT_IDENTIFIER).value - check_details = CheckDetails.fetch(check_name) - - { - type: "issue", - check_name: check_name, - description: node.attributes.fetch("message").value, - categories: check_details.categories, - remediation_points: check_details.remediation_points, - location: { - path: path, - positions: { - begin: { - line: node.attributes.fetch("line").value.to_i, - column: node.attributes.fetch("column").value.to_i - }, - end: { - line: node.attributes.fetch("line").value.to_i, - column: node.attributes.fetch("column").value.to_i - } - } - } - } - rescue KeyError => ex - raise MissingAttributesError, "#{ex.message} on XML '#{node}' when analyzing file '#{path}'" - end - # rubocop:enable Metrics/MethodLength - - def results - @results ||= Nokogiri::XML(csslint_xml) - end - - def csslint_xml - `csslint --format=checkstyle-xml #{files_to_inspect.shelljoin}` - end - - def files_to_inspect - include_paths = engine_config.fetch("include_paths", ["./"]) - extensions = engine_config.fetch("config", {}).fetch("extensions", DEFAULT_EXTENSIONS) - extensions_glob = extensions.join(",") - include_paths.flat_map do |path| - if path.end_with?("/") - Dir.glob("#{File.expand_path path}/**/*{#{extensions_glob}}") - elsif path.end_with?(*extensions) - path - end - end.compact - end - end - end -end diff --git a/lib/cc/engine/csslint/check_details.rb b/lib/cc/engine/csslint/check_details.rb deleted file mode 100644 index d268634..0000000 --- a/lib/cc/engine/csslint/check_details.rb +++ /dev/null @@ -1,54 +0,0 @@ -module CC - module Engine - class CSSlint - class CheckDetails - ALL_RULES = { - # https://github.com/CSSLint/csslint/wiki/Rules - "adjoining-classes" => { categories: "Compatibility" }, - "box-model" => { categories: "Bug Risk" }, - "box-sizing" => { categories: "Compatibility" }, - "bulletproof-font-face" => { categories: "Compatibility" }, - "compatible-vendor-prefixes" => { categories: "Compatibility" }, - "display-property-grouping" => { categories: "Bug Risk" }, - "duplicate-background-images" => { categories: "Bug Risk" }, - "duplicate-properties" => { categories: "Bug Risk" }, - "empty-rules" => { categories: "Bug Risk" }, - "fallback-colors" => { categories: "Compatibility" }, - "font-faces" => { categories: "Bug Risk" }, - "gradients" => { categories: "Compatibility" }, - "import" => { categories: "Bug Risk" }, - "known-properties" => { categories: "Bug Risk" }, - "overqualified-elements" => { categories: "Bug Risk" }, - "parse-error" => { categories: "Bug Risk" }, - "regex-selectors" => { categories: "Bug Risk" }, - "shorthand" => { categories: "Bug Risk" }, - "star-property-hack" => { categories: "Compatibility" }, - "text-indent" => { categories: "Compatibility" }, - "underscore-property-hack" => { categories: "Compatibility" }, - "unique-headings" => { categories: "Duplication" }, - "universal-selector" => { categories: "Bug Risk" }, - "unqualified-attributes" => { categories: "Bug Risk" }, - "vendor-prefix" => { categories: "Compatibility" }, - "zero-units" => { categories: "Bug Risk" }, - }.freeze - - DEFAULT_CATEGORY = "Style".freeze - DEFAULT_REMEDIATION_POINTS = 50_000.freeze - - attr_reader :categories, :remediation_points - - def self.fetch(check_name) - new(ALL_RULES.fetch(check_name, {})) - end - - def initialize( - categories: DEFAULT_CATEGORY, - remediation_points: DEFAULT_REMEDIATION_POINTS - ) - @categories = Array(categories) - @remediation_points = remediation_points - end - end - end - end -end diff --git a/lib/check-details.js b/lib/check-details.js new file mode 100644 index 0000000..13f9e47 --- /dev/null +++ b/lib/check-details.js @@ -0,0 +1,44 @@ +// https://github.com/CSSLint/csslint/wiki/Rules +const ALL_RULES = { + "adjoining-classes": "Compatibility", + "box-model": "Bug Risk", + "box-sizing": "Compatibility", + "bulletproof-font-face": "Compatibility", + "compatible-vendor-prefixes": "Compatibility", + "display-property-grouping": "Bug Risk", + "duplicate-background-images": "Performance", + "duplicate-properties": "Bug Risk", + "empty-rules": "Bug Risk", + "fallback-colors": "Compatibility", + "floats": "Clarity", + "font-faces": "Performance", + "font-sizes": "Clarity", + "gradients": "Compatibility", + "ids": "Complexity", + "import": "Performance", + "important": "Complexity", + "known-properties": "Bug Risk", + "overqualified-elements": "Performance", + "parse-error": "Bug Risk", + "regex-selectors": "Performance", + "shorthand": "Performance", + "star-property-hack": "Compatibility", + "text-indent": "Compatibility", + "underscore-property-hack": "Compatibility", + "unique-headings": "Duplication", + "universal-selector": "Performance", + "unqualified-attributes": "Performance", + "vendor-prefix": "Compatibility", + "zero-units": "Performance" +}; + +const DEFAULT_CATEGORY = "Style"; +const DEFAULT_REMEDIATION_POINTS = 50000; + +module.exports = function(check_name) { + let category = ALL_RULES[check_name] || DEFAULT_CATEGORY; + return { + categories: [category], + remediation_points: DEFAULT_REMEDIATION_POINTS + }; +}; diff --git a/lib/codeclimate-formatter.js b/lib/codeclimate-formatter.js new file mode 100644 index 0000000..ed9f03b --- /dev/null +++ b/lib/codeclimate-formatter.js @@ -0,0 +1,78 @@ +"use strict"; + +const checkDetails = require('./check-details'); + +const DEFAULT_IDENTIFIER = "parse-error"; +const forEach = require("csslint/dist/csslint-node").CSSLint.Util.forEach; + +function ruleIdentifier(rule) { + if (!rule || !("id" in rule)) { + return "generic"; + } + return rule.id; +}; + +function reportJSON(filename, report) { + let check_name = report.rule ? ruleIdentifier(report.rule) : DEFAULT_IDENTIFIER; + let details = checkDetails(check_name); + + return JSON.stringify({ + type: "issue", + check_name: check_name, + description: report.message, + categories: details.categories, + remediation_points: details.remediation_points, + location: { + path: filename, + positions: { + begin: { + line: report.line || 1, + column: report.col || 1 + }, + end: { + line: report.line || 1, + column: report.col || 1 + } + } + } + }) + "\x00"; +} + +module.exports = { + // format information + id: "codeclimate", + name: "Code Climate format", + + startFormat: function() { + return ""; + }, + endFormat: function() { + return ""; + }, + + readError: function(filename, message) { + let report = { + type: "error", + line: 1, + col: 1, + message : message + }; + return reportJSON(filename, report); + }, + + formatResults: function(results, filename/*, options*/) { + let reports = results.messages; + let output = []; + + if (reports.length > 0) { + forEach(reports, function (report) { + // ignore rollups for now + if (!report.rollup) { + output.push(reportJSON(filename, report)); + } + }); + } + + return output.join(""); + } +}; diff --git a/lib/csslint.js b/lib/csslint.js new file mode 100644 index 0000000..bf1dffa --- /dev/null +++ b/lib/csslint.js @@ -0,0 +1,120 @@ +"use strict"; + +const fs = require("fs"); +const path = require("path"); +const glob = require("glob"); +const CSSLint = require("csslint/dist/csslint-node").CSSLint; +const CodeClimateFormatter = require("./codeclimate-formatter"); + +const DEFAULT_EXTENSIONS = [".css"]; + +CSSLint.addFormatter(CodeClimateFormatter); + +function readFile(filename) { + try { + return fs.readFileSync(filename, "utf-8"); + } catch (ex) { + return ""; + } +} + + +class Analyzer { + constructor(directory, console, config) { + this.directory = directory; + this.console = console; + this.config = config; + } + + run() { + let files = this.expandPaths(this.config.include_paths || ["./"]); + + this.processFiles(files); + } + + + // private ================================================================= + + + print(message) { + this.console.log(message); + } + + processFile(relativeFilePath) { + let input = readFile(path.join(this.directory, relativeFilePath)); + let formatter = CSSLint.getFormatter("codeclimate"); + + if (!input) { + this.print(formatter.readError(relativeFilePath, "Could not read file data. Is the file empty?")); + } else { + let result = CSSLint.verify(input); + let messages = result.messages || []; + let output = formatter.formatResults(result, relativeFilePath); + if (output) { + this.print(output); + } + } + } + + + processFiles(files) { + for (let file of files) { + this.processFile(file); + } + } + + + expandPaths(paths) { + let files = []; + + for (let path of paths) { + let new_files = this.getFiles(path, this.directory); + files = files.concat(new_files); + } + + return files; + } + + getFiles(pathname) { + var files = []; + let full_pathname = path.normalize(path.join(this.directory, pathname)); + let stat; + let base_name = path.basename(pathname); + + try { + stat = fs.statSync(full_pathname); + } catch (ex) { + return []; + } + + if (stat.isFile() && this.extensionsRegExp.test(full_pathname)) { + return [pathname]; + } else if (stat.isDirectory()) { + for (let file of fs.readdirSync(full_pathname)) { + let new_path = path.join(full_pathname, file); + files = files.concat(this.getFiles(path.relative(this.directory, new_path))); + }; + } + + return files; + } + + get extensionsRegExp() { + return RegExp( + (this.config.extensions || DEFAULT_EXTENSIONS). + map(e => e.replace('.', '\\.')). + join("|") + + "$" + ); + } +} + +module.exports = class { + constructor(directory, console, config) { + this.analyzer = new Analyzer(directory, console, config); + } + + run() { + this.analyzer.run(); + } +}; diff --git a/package.json b/package.json new file mode 100644 index 0000000..2c01c33 --- /dev/null +++ b/package.json @@ -0,0 +1,25 @@ +{ + "name": "codeclimate-csslint", + "version": "1.0.0", + "description": "Code Climate CSSLint Engine", + "repository": { + "type": "git", + "url": "https://github.com/codeclimate/codeclimate-csslint.git" + }, + "author": "Code Climate", + "license": "MIT", + "dependencies": { + "csslint": "^1.0.5", + "glob": "^7.1.2" + }, + "devDependencies": { + "chai": "^4.1.2", + "mkdirp": "^0.5.1", + "mocha": "^4.0.1", + "sinon": "^4.1.3", + "temp": "^0.8.3" + }, + "scripts": { + "test": "mocha -gc test" + } +} diff --git a/spec/cc/engine/csslint/check_details_spec.rb b/spec/cc/engine/csslint/check_details_spec.rb deleted file mode 100644 index 134dbb6..0000000 --- a/spec/cc/engine/csslint/check_details_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require "spec_helper" - -class CC::Engine::CSSlint - describe CheckDetails do - describe ".fetch" do - it "returns details for customized checks" do - details = CheckDetails.fetch("import") - - expect(details.categories).to eq ["Bug Risk"] - expect(details.remediation_points).to eq 50_000 - end - - it "returns defaults for unknown checks" do - details = CheckDetails.fetch("made-up") - - expect(details.categories).to eq ["Style"] - expect(details.remediation_points).to eq 50_000 - end - end - end -end diff --git a/spec/cc/engine/csslint_spec.rb b/spec/cc/engine/csslint_spec.rb deleted file mode 100644 index 6b17dfb..0000000 --- a/spec/cc/engine/csslint_spec.rb +++ /dev/null @@ -1,106 +0,0 @@ -require "spec_helper" - -module CC - module Engine - describe CSSlint do - let(:code) { Dir.mktmpdir } - let(:engine_config) { {} } - let(:lint) do - CSSlint.new(directory: code, io: nil, engine_config: engine_config) - end - let(:id_selector_content) { '#id { color: red; }' } - - describe '#run' do - it 'analyzes *.css files' do - create_source_file('foo.css', id_selector_content) - expect{ lint.run }.to output(/Don't use IDs in selectors./).to_stdout - end - - it 'fails on malformed file' do - create_source_file('foo.css', '�6�') - expect{ lint.run }.to output(/Unexpected token/).to_stdout - end - - it "doesn't analyze *.scss files" do - create_source_file('foo.scss', id_selector_content) - expect{ lint.run }.to_not output.to_stdout - end - - it "only reports issues in the file where they're present" do - create_source_file('bad.css', id_selector_content) - create_source_file('good.css', '.foo { margin: 0 }') - expect{ lint.run }.not_to output(/good\.css/).to_stdout - end - - describe "with include_paths" do - let(:engine_config) { - {"include_paths" => %w(included.css included_dir/ config.yml)} - } - - before do - create_source_file("included.css", id_selector_content) - create_source_file( - "included_dir/file.css", "p { color: blue !important; }" - ) - create_source_file( - "included_dir/sub/sub/subdir/file.css", "img { }" - ) - create_source_file("config.yml", "foo:\n bar: \"baz\"") - create_source_file("not_included.css", "a { outline: none; }") - end - - it "includes all mentioned files" do - expect{ lint.run }.to \ - output(/Don't use IDs in selectors./).to_stdout - end - - it "expands directories" do - expect{ lint.run }.to output(/Use of !important/).to_stdout - expect{ lint.run }.to output(/Rule is empty/).to_stdout - end - - it "excludes any unmentioned files" do - expect{ lint.run }.not_to \ - output(/Outlines should only be modified using :focus/).to_stdout - end - - it "shouldn't call a top-level Dir.glob ever" do - allow(Dir).to receive(:glob).and_call_original - expect(Dir).not_to receive(:glob).with("**/*.css") - expect{ lint.run }.to \ - output(/Don't use IDs in selectors./).to_stdout - end - - it "only includes CSS files, even when a non-CSS file is directly included" do - expect{ lint.run }.not_to output(/config.yml/).to_stdout - end - end - - describe "with custom extensions" do - let(:engine_config) do - { - "config" => { - "extensions" => %w(.fancycss) - } - } - end - - before do - create_source_file("master.fancycss", id_selector_content) - end - - it "takes into account extensions" do - expect{ lint.run }.to \ - output(/Don't use IDs in selectors./).to_stdout - end - end - end - - def create_source_file(path, content) - abs_path = File.join(code, path) - FileUtils.mkdir_p(File.dirname(abs_path)) - File.write(abs_path, content) - end - end - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb deleted file mode 100644 index 2be99e0..0000000 --- a/spec/spec_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -require "cc/engine/csslint" -require "tmpdir" diff --git a/test/check-details-test.js b/test/check-details-test.js new file mode 100644 index 0000000..bb8646b --- /dev/null +++ b/test/check-details-test.js @@ -0,0 +1,18 @@ +const checkDetails = require("../lib/check-details"); +const expect = require("chai").expect; + +describe("Check Details", function() { + it("returns details for customized checks", function() { + let details = checkDetails("import"); + + expect(details.categories).to.deep.equal(["Performance"]); + expect(details.remediation_points).to.eq(50000); + }); + + it("returns defauls for unknown checks", function() { + let details = checkDetails("made-up"); + + expect(details.categories).to.deep.equal(["Style"]); + expect(details.remediation_points).to.eq(50000); + }); +}); diff --git a/test/codeclimate-formatter_test.js b/test/codeclimate-formatter_test.js new file mode 100644 index 0000000..ad7eb10 --- /dev/null +++ b/test/codeclimate-formatter_test.js @@ -0,0 +1,54 @@ +const Formatter = require("../lib/codeclimate-formatter"); +const expect = require("chai").expect; +const CSSLint = require("csslint/dist/csslint-node").CSSLint; + +describe("Code Climate Formatter", function() { + describe(".startFormat", function() { + it("returns a blank string", function() { + expect(Formatter.startFormat()).to.eq(""); + }); + }); + + describe(".endFormat", function() { + it("returns a blank string", function() { + expect(Formatter.endFormat()).to.eq(""); + }); + }); + + describe(".readError", function() { + it("properly serializes a read error", function() { + expect(Formatter.readError("foo.css", "Can not read the file")).to.eq( + '{"type":"issue","check_name":"parse-error","description":"Can not read the file","categories":["Bug Risk"],"remediation_points":50000,"location":{"path":"foo.css","positions":{"begin":{"line":1,"column":1},"end":{"line":1,"column":1}}}}\x00' + ); + }); + }); + + describe(".formatResults", function() { + it("properly serializes reports", function() { + let reports = [ + { + type: "warning", + line: 1, + col: 1, + message: "Don't use adjoining classes.", + evidence: ".im-bad {", + rule: CSSLint.getRules().find( rule => rule.id === "adjoining-classes") + }, + { + type: "warning", + line: 10, + col: 1, + message: "Disallow empty rules", + evidence: ".empty {}", + rule: CSSLint.getRules().find( rule => rule.id === "empty-rules") + } + ]; + + expect(Formatter.formatResults({messages: reports}, "foo.css")).to.eq( + '{"type":"issue","check_name":"adjoining-classes","description":"Don\'t use adjoining classes.","categories":["Compatibility"],"remediation_points":50000,"location":{"path":"foo.css","positions":{"begin":{"line":1,"column":1},"end":{"line":1,"column":1}}}}\x00' + + '{"type":"issue","check_name":"empty-rules","description":"Disallow empty rules","categories":["Bug Risk"],"remediation_points":50000,"location":{"path":"foo.css","positions":{"begin":{"line":10,"column":1},"end":{"line":10,"column":1}}}}\x00' + ); + }); + }); + +}); diff --git a/test/csslint-test.js b/test/csslint-test.js new file mode 100644 index 0000000..8892f37 --- /dev/null +++ b/test/csslint-test.js @@ -0,0 +1,127 @@ +const Engine = require("../lib/csslint"); +const expect = require("chai").expect; +const CSSLint = require("csslint/dist/csslint-node").CSSLint; +const temp = require('temp').track(); +const fs = require("fs"); +const path = require("path"); +const mkdirp = require('mkdirp').sync; + +class FakeConsole { + constructor() { + this.logs = []; + this.warns = []; + } + + get output() { + return this.logs.join("\n"); + } + + + log(str) { + this.logs.push(str); + } + + warn(str) { + console.warn(str); + this.warns.push(str); + } +} + + +function createSourceFile(root, filename, content) { + let dirname = path.dirname(path.join(root, filename)); + if (!fs.existsSync(dirname)) { + mkdirp(dirname); + } + fs.writeFileSync(path.join(root, filename), content); +} + +describe("CSSLint Engine", function() { + beforeEach(function(){ + this.id_selector_content = "#id { color: red; }"; + this.code_dir = temp.mkdirSync("code"); + this.console = new FakeConsole(); + this.lint = new Engine(this.code_dir, this.console, {}); + }); + + it('analyzes *.css files', function() { + createSourceFile(this.code_dir, 'foo.css', this.id_selector_content); + + this.lint.run(); + expect(this.console.output).to.include("Don't use IDs in selectors."); + }); + + it('fails on malformed file', function() { + createSourceFile(this.code_dir, 'foo.css', '�6�'); + + this.lint.run(); + expect(this.console.output).to.include('Unexpected token'); + }); + + it("doesn't analyze *.scss files", function() { + createSourceFile(this.code_dir, 'foo.scss', this.id_selector_content); + + this.lint.run(); + expect(this.console.output).to.eq(''); + }); + + it("only reports issues in the file where they're present", function() { + createSourceFile(this.code_dir, 'bad.css', this.id_selector_content); + createSourceFile(this.code_dir, 'good.css', '.foo { margin: 0 }'); + + this.lint.run(); + expect(this.console.output).to.not.include('good.css'); + }); + + context("with include_paths", function(){ + beforeEach(function() { + let engine_config = { + include_paths: ["included.css", "included_dir/", "config.yml"] + }; + this.lint = new Engine(this.code_dir, this.console, engine_config); + + createSourceFile(this.code_dir, "included.css", this.id_selector_content); + createSourceFile(this.code_dir, "included_dir/file.css", "p { color: blue !important; }"); + createSourceFile(this.code_dir, "included_dir/sub/sub/subdir/file.css", "img { }"); + createSourceFile(this.code_dir, "config.yml", "foo:\n bar: \"baz\""); + createSourceFile(this.code_dir, "not_included.css", "a { outline: none; }"); + }); + + it("includes all mentioned files", function() { + this.lint.run(); + expect(this.console.output).to.include("Don't use IDs in selectors."); + }); + + it("expands directories", function() { + this.lint.run(); + expect(this.console.output).to.include('Use of !important'); + expect(this.console.output).to.include('Rule is empty'); + }); + + it("excludes any unmentioned files", function() { + this.lint.run(); + expect(this.console.output).to.not.include('Outlines should only be modified using :focus'); + }); + + it("only includes CSS files, even when a non-CSS file is directly included", function() { + this.lint.run(); + expect(this.console.output).to.not.include('config.yml'); + }); + }); + + context("with custom extensions", function(){ + beforeEach(function() { + let engine_config = { + extensions: [".fancycss"] + }; + this.lint = new Engine(this.code_dir, this.console, engine_config); + + createSourceFile(this.code_dir, "master.fancycss", this.id_selector_content); + }); + + it("takes into account extensions", function() { + this.lint.run(); + expect(this.console.output).to.include("Don't use IDs in selectors."); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock new file mode 100644 index 0000000..99a6eac --- /dev/null +++ b/yarn.lock @@ -0,0 +1,272 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +assertion-error@^1.0.1: + version "1.0.2" + resolved "https://registry.yarnpkg.com/assertion-error/-/assertion-error-1.0.2.tgz#13ca515d86206da0bac66e834dd397d87581094c" + +balanced-match@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767" + +brace-expansion@^1.1.7: + version "1.1.8" + resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.8.tgz#c07b211c7c952ec1f8efd51a77ef0d1d3990a292" + dependencies: + balanced-match "^1.0.0" + concat-map "0.0.1" + +browser-stdout@1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/browser-stdout/-/browser-stdout-1.3.0.tgz#f351d32969d32fa5d7a5567154263d928ae3bd1f" + +chai@^4.1.2: + version "4.1.2" + resolved "https://registry.yarnpkg.com/chai/-/chai-4.1.2.tgz#0f64584ba642f0f2ace2806279f4f06ca23ad73c" + dependencies: + assertion-error "^1.0.1" + check-error "^1.0.1" + deep-eql "^3.0.0" + get-func-name "^2.0.0" + pathval "^1.0.0" + type-detect "^4.0.0" + +check-error@^1.0.1: + version "1.0.2" + resolved "https://registry.yarnpkg.com/check-error/-/check-error-1.0.2.tgz#574d312edd88bb5dd8912e9286dd6c0aed4aac82" + +clone@~2.1.0: + version "2.1.1" + resolved "https://registry.yarnpkg.com/clone/-/clone-2.1.1.tgz#d217d1e961118e3ac9a4b8bba3285553bf647cdb" + +commander@2.11.0: + version "2.11.0" + resolved "https://registry.yarnpkg.com/commander/-/commander-2.11.0.tgz#157152fd1e7a6c8d98a5b715cf376df928004563" + +concat-map@0.0.1: + version "0.0.1" + resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b" + +csslint@^1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/csslint/-/csslint-1.0.5.tgz#19cc3eda322160fd3f7232af1cb2a360e898a2e9" + dependencies: + clone "~2.1.0" + parserlib "~1.1.1" + +debug@3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/debug/-/debug-3.1.0.tgz#5bb5a0672628b64149566ba16819e61518c67261" + dependencies: + ms "2.0.0" + +deep-eql@^3.0.0: + version "3.0.1" + resolved "https://registry.yarnpkg.com/deep-eql/-/deep-eql-3.0.1.tgz#dfc9404400ad1c8fe023e7da1df1c147c4b444df" + dependencies: + type-detect "^4.0.0" + +diff@3.3.1: + version "3.3.1" + resolved "https://registry.yarnpkg.com/diff/-/diff-3.3.1.tgz#aa8567a6eed03c531fc89d3f711cd0e5259dec75" + +diff@^3.1.0: + version "3.4.0" + resolved "https://registry.yarnpkg.com/diff/-/diff-3.4.0.tgz#b1d85507daf3964828de54b37d0d73ba67dda56c" + +escape-string-regexp@1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4" + +formatio@1.2.0, formatio@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/formatio/-/formatio-1.2.0.tgz#f3b2167d9068c4698a8d51f4f760a39a54d818eb" + dependencies: + samsam "1.x" + +fs.realpath@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/fs.realpath/-/fs.realpath-1.0.0.tgz#1504ad2523158caa40db4a2787cb01411994ea4f" + +get-func-name@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/get-func-name/-/get-func-name-2.0.0.tgz#ead774abee72e20409433a066366023dd6887a41" + +glob@7.1.2, glob@^7.1.2: + version "7.1.2" + resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15" + dependencies: + fs.realpath "^1.0.0" + inflight "^1.0.4" + inherits "2" + minimatch "^3.0.4" + once "^1.3.0" + path-is-absolute "^1.0.0" + +growl@1.10.3: + version "1.10.3" + resolved "https://registry.yarnpkg.com/growl/-/growl-1.10.3.tgz#1926ba90cf3edfe2adb4927f5880bc22c66c790f" + +has-flag@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-2.0.0.tgz#e8207af1cc7b30d446cc70b734b5e8be18f88d51" + +he@1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/he/-/he-1.1.1.tgz#93410fd21b009735151f8868c2f271f3427e23fd" + +inflight@^1.0.4: + version "1.0.6" + resolved "https://registry.yarnpkg.com/inflight/-/inflight-1.0.6.tgz#49bd6331d7d02d0c09bc910a1075ba8165b56df9" + dependencies: + once "^1.3.0" + wrappy "1" + +inherits@2: + version "2.0.3" + resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de" + +isarray@0.0.1: + version "0.0.1" + resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf" + +just-extend@^1.1.26: + version "1.1.27" + resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-1.1.27.tgz#ec6e79410ff914e472652abfa0e603c03d60e905" + +lodash.get@^4.4.2: + version "4.4.2" + resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" + +lolex@^1.6.0: + version "1.6.0" + resolved "https://registry.yarnpkg.com/lolex/-/lolex-1.6.0.tgz#3a9a0283452a47d7439e72731b9e07d7386e49f6" + +lolex@^2.2.0: + version "2.3.1" + resolved "https://registry.yarnpkg.com/lolex/-/lolex-2.3.1.tgz#3d2319894471ea0950ef64692ead2a5318cff362" + +minimatch@^3.0.4: + version "3.0.4" + resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" + dependencies: + brace-expansion "^1.1.7" + +minimist@0.0.8: + version "0.0.8" + resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.8.tgz#857fcabfc3397d2625b8228262e86aa7a011b05d" + +mkdirp@0.5.1, mkdirp@^0.5.1: + version "0.5.1" + resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.1.tgz#30057438eac6cf7f8c4767f38648d6697d75c903" + dependencies: + minimist "0.0.8" + +mocha@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/mocha/-/mocha-4.0.1.tgz#0aee5a95cf69a4618820f5e51fa31717117daf1b" + dependencies: + browser-stdout "1.3.0" + commander "2.11.0" + debug "3.1.0" + diff "3.3.1" + escape-string-regexp "1.0.5" + glob "7.1.2" + growl "1.10.3" + he "1.1.1" + mkdirp "0.5.1" + supports-color "4.4.0" + +ms@2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/ms/-/ms-2.0.0.tgz#5608aeadfc00be6c2901df5f9861788de0d597c8" + +nise@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/nise/-/nise-1.2.0.tgz#079d6cadbbcb12ba30e38f1c999f36ad4d6baa53" + dependencies: + formatio "^1.2.0" + just-extend "^1.1.26" + lolex "^1.6.0" + path-to-regexp "^1.7.0" + text-encoding "^0.6.4" + +once@^1.3.0: + version "1.4.0" + resolved "https://registry.yarnpkg.com/once/-/once-1.4.0.tgz#583b1aa775961d4b113ac17d9c50baef9dd76bd1" + dependencies: + wrappy "1" + +os-tmpdir@^1.0.0: + version "1.0.2" + resolved "https://registry.yarnpkg.com/os-tmpdir/-/os-tmpdir-1.0.2.tgz#bbe67406c79aa85c5cfec766fe5734555dfa1274" + +parserlib@~1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/parserlib/-/parserlib-1.1.1.tgz#a64cfa724062434fdfc351c9a4ec2d92b94c06f4" + +path-is-absolute@^1.0.0: + version "1.0.1" + resolved "https://registry.yarnpkg.com/path-is-absolute/-/path-is-absolute-1.0.1.tgz#174b9268735534ffbc7ace6bf53a5a9e1b5c5f5f" + +path-to-regexp@^1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.7.0.tgz#59fde0f435badacba103a84e9d3bc64e96b9937d" + dependencies: + isarray "0.0.1" + +pathval@^1.0.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/pathval/-/pathval-1.1.0.tgz#b942e6d4bde653005ef6b71361def8727d0645e0" + +rimraf@~2.2.6: + version "2.2.8" + resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.2.8.tgz#e439be2aaee327321952730f99a8929e4fc50582" + +samsam@1.x: + version "1.3.0" + resolved "https://registry.yarnpkg.com/samsam/-/samsam-1.3.0.tgz#8d1d9350e25622da30de3e44ba692b5221ab7c50" + +sinon@^4.1.3: + version "4.1.3" + resolved "https://registry.yarnpkg.com/sinon/-/sinon-4.1.3.tgz#fc599eda47ed9f1a694ce774b94ab44260bd7ac5" + dependencies: + diff "^3.1.0" + formatio "1.2.0" + lodash.get "^4.4.2" + lolex "^2.2.0" + nise "^1.2.0" + supports-color "^4.4.0" + type-detect "^4.0.5" + +supports-color@4.4.0: + version "4.4.0" + resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-4.4.0.tgz#883f7ddabc165142b2a61427f3352ded195d1a3e" + dependencies: + has-flag "^2.0.0" + +supports-color@^4.4.0: + version "4.5.0" + resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-4.5.0.tgz#be7a0de484dec5c5cddf8b3d59125044912f635b" + dependencies: + has-flag "^2.0.0" + +temp@^0.8.3: + version "0.8.3" + resolved "https://registry.yarnpkg.com/temp/-/temp-0.8.3.tgz#e0c6bc4d26b903124410e4fed81103014dfc1f59" + dependencies: + os-tmpdir "^1.0.0" + rimraf "~2.2.6" + +text-encoding@^0.6.4: + version "0.6.4" + resolved "https://registry.yarnpkg.com/text-encoding/-/text-encoding-0.6.4.tgz#e399a982257a276dae428bb92845cb71bdc26d19" + +type-detect@^4.0.0, type-detect@^4.0.5: + version "4.0.5" + resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.5.tgz#d70e5bc81db6de2a381bcaca0c6e0cbdc7635de2" + +wrappy@1: + version "1.0.2" + resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f" From 5899409507bceda9c6144f7c63281733f351018f Mon Sep 17 00:00:00 2001 From: Alexander Mankuta Date: Fri, 15 Dec 2017 16:55:30 +0200 Subject: [PATCH 2/2] Update engine to follow the Spec --- Dockerfile | 14 +++++++++++--- Makefile | 2 +- engine.json | 13 +++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 engine.json diff --git a/Dockerfile b/Dockerfile index b3c90a9..4f271fa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,26 @@ FROM node:alpine +LABEL maintainer="Code Climate " RUN adduser -u 9000 -D app WORKDIR /usr/src/app -COPY package.json yarn.lock ./ +COPY package.json yarn.lock engine.json ./ RUN yarn install && \ - chown -R app:app ./ + chown -R app:app ./ && \ + apk add --no-cache --virtual .dev-deps jq && \ + export csslint_version=$(yarn --json list --pattern csslint 2>/dev/null | jq -r '.data.trees[0].name' | cut -d@ -f2) && \ + cat engine.json | jq '.version = .version + "/" + env.csslint_version' > /engine.json && \ + apk del .dev-deps COPY . ./ +COPY . /usr/src/app + USER app -COPY . /usr/src/app +VOLUME /code +WORKDIR /code CMD ["/usr/src/app/bin/csslint"] diff --git a/Makefile b/Makefile index f83f495..43b56ef 100644 --- a/Makefile +++ b/Makefile @@ -4,4 +4,4 @@ image: docker build -t codeclimate/codeclimate-csslint . test: image - docker run --rm codeclimate/codeclimate-csslint npm run test + docker run --rm codeclimate/codeclimate-csslint sh -c "cd /usr/src/app && npm run test" diff --git a/engine.json b/engine.json new file mode 100644 index 0000000..6909387 --- /dev/null +++ b/engine.json @@ -0,0 +1,13 @@ +{ + "name": "CSSLint", + "description": "CSSLint is a tool to help point out problems with your CSS code.", + "maintainer": { + "name": "Code Climate", + "email": "hello@codeclimate.com" + }, + "languages": [ + "CSS" + ], + "version": "2.0.0", + "spec_version": "0.3.1" +}