From 8c13e3855e0adae812d812aa05e7b3d28d25160b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Sat, 7 Nov 2020 07:11:09 +0100 Subject: [PATCH] ci: Use node-js script for running eslint This is unnecessary hard in shell when compared to a proper programming language. It becomes even easier with a node-js script, as that gives us access to the underlying ESLint module rather than just the CLI interface. Besides that, node-js has the added benefit that we don't need to add more dependencies to the CI image. https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1497 --- .gitlab-ci.yml | 6 +- .gitlab-ci/run-eslint | 128 +++++++++++++++++++++++++++++++++++++++ .gitlab-ci/run-eslint.sh | 116 ----------------------------------- 3 files changed, 132 insertions(+), 118 deletions(-) create mode 100755 .gitlab-ci/run-eslint delete mode 100755 .gitlab-ci/run-eslint.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index adad8f958..52e60764d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -9,6 +9,7 @@ stages: variables: BUNDLE: "extensions-git.flatpak" JS_LOG: "js-report.txt" + LINT_LOG: "eslint-report.txt" .only_default: &only_default only: @@ -42,11 +43,12 @@ eslint: image: registry.gitlab.gnome.org/gnome/gnome-shell/extension-ci:v2 stage: review script: - - ./.gitlab-ci/run-eslint.sh + - export NODE_PATH=$(npm root -g) + - ./.gitlab-ci/run-eslint --output-file ${LINT_LOG} <<: *only_default artifacts: paths: - - reports + - ${LINT_LOG} when: always potfile_check: diff --git a/.gitlab-ci/run-eslint b/.gitlab-ci/run-eslint new file mode 100755 index 000000000..68a664109 --- /dev/null +++ b/.gitlab-ci/run-eslint @@ -0,0 +1,128 @@ +#!/usr/bin/env node + +const { ESLint } = require('eslint'); +const fs = require('fs'); +const path = require('path'); +const { spawn } = require('child_process'); + +function createConfig(config) { + const options = { + cache: true, + cacheLocation: `.eslintcache-${config}`, + }; + + if (config === 'legacy') + options.overrideConfigFile='lint/eslintrc-legacy.yml'; + + return new ESLint(options); +} + +function git(...args) { + const git = spawn('git', args, { stdio: ['ignore', null, 'ignore'] }); + git.stdout.setEncoding('utf8'); + + return new Promise(resolve => { + let out = ''; + git.stdout.on('data', chunk => out += chunk); + git.stdout.on('end', () => resolve(out.trim())); + }); +} + +function createCommon(report1, report2, ignoreColumn=false) { + return report1.map(result => { + const { filePath, messages } = result; + const match = + report2.find(r => r.filePath === filePath) || { messages: [] }; + + const filteredMessages = messages.filter( + msg => match.messages.some( + m => m.line === msg.line && (ignoreColumn || m.column === msg.column))); + + const [errorCount, warningCount] = filteredMessages.reduce( + ([e, w], msg) => { + return [ + e + Number(msg.severity === 2), + w + Number(msg.severity === 1)]; + }, [0, 0]); + + return { + filePath, + messages: filteredMessages, + errorCount, + warningCount, + }; + }); +} + +async function getMergeRequestChanges(remote, branch) { + await git('fetch', remote, branch); + const branchPoint = await git('merge-base', 'HEAD', 'FETCH_HEAD'); + const diff = await git('diff', '-U0', `${branchPoint}...HEAD`); + + const report = []; + let messages = null; + for (const line of diff.split('\n')) { + if (line.startsWith('+++ b/')) { + const filePath = path.resolve(line.substring(6)); + messages = filePath.endsWith('.js') ? [] : null; + if (messages) + report.push({ filePath, messages }); + } else if (messages && line.startsWith('@@ ')) { + [, , changes] = line.split(' '); + [start, count] = `${changes},1`.split(',').map(i => parseInt(i)); + for (let i = start; i < start + count; i++) + messages.push({ line: i }); + } + } + + return report; +} + +function getOption(...names) { + const optIndex = + process.argv.findIndex(arg => names.includes(arg)) + 1; + + if (optIndex === 0) + return undefined; + + return process.argv[optIndex]; +} + +(async function main() { + const outputOption = getOption('--output-file', '-o'); + const outputPath = outputOption ? path.resolve(outputOption) : null; + + const sourceDir = path.dirname(process.argv[1]); + process.chdir(path.resolve(sourceDir, '..')); + + const remote = getOption('--remote') || 'origin'; + const branch = getOption('--branch', '-b'); + + const sources = ['js', 'subprojects/extensions-app/js']; + const regular = createConfig('regular'); + + const ops = []; + ops.push(regular.lintFiles(sources)); + if (branch) + ops.push(getMergeRequestChanges(remote, branch)); + else + ops.push(createConfig('legacy').lintFiles(sources)); + + const results = await Promise.all(ops); + const commonResults = createCommon(...results, branch !== undefined); + + const formatter = await regular.loadFormatter(); + const resultText = formatter.format(commonResults); + + if (outputPath) { + fs.mkdirSync(path.dirname(outputPath), { recursive: true }); + fs.writeFileSync(outputPath, resultText); + } else { + console.log(resultText); + } + + process.exitCode = commonResults.some(r => r.errorCount > 0) ? 1 : 0; +})().catch((error) => { + process.exitCode = 1; + console.error(error); +}); diff --git a/.gitlab-ci/run-eslint.sh b/.gitlab-ci/run-eslint.sh deleted file mode 100755 index 411c9d45b..000000000 --- a/.gitlab-ci/run-eslint.sh +++ /dev/null @@ -1,116 +0,0 @@ -#!/usr/bin/env bash - -OUTPUT_REGULAR=reports/lint-regular-report.txt -OUTPUT_LEGACY=reports/lint-legacy-report.txt -OUTPUT_FINAL=reports/lint-common-report.txt - -OUTPUT_MR=reports/lint-mr-report.txt - -LINE_CHANGES=changed-lines.txt - -is_empty() { - (! grep -q . $1) -} - -run_eslint() { - ARGS_LEGACY='--config lint/eslintrc-legacy.yml' - - local extra_args=ARGS_$1 - local output_var=OUTPUT_$1 - local output=${!output_var} - local cache=.eslintcache-${1,,} - - # ensure output exists even if eslint doesn't report any errors - mkdir -p $(dirname $output) - touch $output - - eslint -f unix --cache --cache-location $cache ${!extra_args} -o $output \ - js subprojects/extensions-app/js -} - -list_commit_range_additions() { - # Turn raw context-less git-diff into a list of - # filename:lineno pairs of new (+) lines - git diff -U0 "$@" -- js | - awk ' - BEGIN { file=""; } - /^+++ b/ { file=substr($0,7); } - /^@@ / { - len = split($3,a,",") - start=a[1] - count=(len > 1) ? a[2] : 1 - - for (line=start; line $target - for l in $(<$matches); do - grep $l $source >> $target - done -} - -create_common() { - # comm requires sorted input; - # we also strip the error message to make the following a "common" error: - # regular: - # file.js:42:23 Indentation of 55, expected 42 - # legacy: - # file.js:42:23 Indentation of 55, extected 24 - prepare() { - sed 's: .*::' $1 | sort - } - - comm -12 <(prepare $OUTPUT_REGULAR) <(prepare $OUTPUT_LEGACY) >$OUTPUT_FINAL.tmp - - # Now add back the stripped error messages - copy_matched_lines $OUTPUT_REGULAR $OUTPUT_FINAL.tmp $OUTPUT_FINAL - rm $OUTPUT_FINAL.tmp -} - -# Disable MR handling for now. We aren't ready to enforce -# non-legacy style just yet ... -unset CI_MERGE_REQUEST_TARGET_BRANCH_NAME - -REMOTE=${1:-$CI_MERGE_REQUEST_PROJECT_URL.git} -BRANCH_NAME=${2:-$CI_MERGE_REQUEST_TARGET_BRANCH_NAME} - -if [ "$BRANCH_NAME" ]; then - git fetch $REMOTE $BRANCH_NAME - branch_point=$(git merge-base HEAD FETCH_HEAD) - commit_range=$branch_point...HEAD - - list_commit_range_additions $commit_range > $LINE_CHANGES - - # Don't bother with running lint when no JS changed - if is_empty $LINE_CHANGES; then - exit 0 - fi -fi - -echo Generating lint report using regular configuration -run_eslint REGULAR -echo Generating lint report using legacy configuration -run_eslint LEGACY -echo Done. -create_common - -if ! is_empty $OUTPUT_FINAL; then - cat $OUTPUT_FINAL - exit 1 -fi - -# Just show the report and succeed when not testing a MR -if [ -z "$BRANCH_NAME" ]; then - exit 0 -fi - -copy_matched_lines $OUTPUT_REGULAR $LINE_CHANGES $OUTPUT_MR -cat $OUTPUT_MR -is_empty $OUTPUT_MR