From 8556019d203785f75795b1973a0ed380235cd954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 24 Sep 2018 15:26:52 +0200 Subject: [build] post clang-format suggestions to PRs --- .clang-format | 20 ++++++- circle.yml | 7 ++- package.json | 3 +- scripts/environment.js | 55 ++++++++++++-------- scripts/nitpick/format.js | 129 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 190 insertions(+), 24 deletions(-) create mode 100755 scripts/nitpick/format.js diff --git a/.clang-format b/.clang-format index ff0f39e73b..cd96b6cd27 100644 --- a/.clang-format +++ b/.clang-format @@ -2,13 +2,15 @@ Standard: Cpp11 IndentWidth: 4 AccessModifierOffset: -4 UseTab: Never +AlignAfterOpenBracket: Align BinPackParameters: false +BinPackArguments: false AllowShortIfStatementsOnASingleLine: false AllowShortLoopsOnASingleLine: false AllowShortBlocksOnASingleLine: false AllowShortFunctionsOnASingleLine: false ConstructorInitializerAllOnOneLineOrOnePerLine: true -AlwaysBreakTemplateDeclarations: true +AlwaysBreakTemplateDeclarations: false NamespaceIndentation: None PointerBindsToType: true SpacesInParentheses: false @@ -16,3 +18,19 @@ BreakBeforeBraces: Attach ColumnLimit: 100 Cpp11BracedListStyle: false SpacesBeforeTrailingComments: 1 +FixNamespaceComments: true +ReflowComments: true +BreakStringLiterals: true +SpaceAfterTemplateKeyword: true +SpaceBeforeAssignmentOperators: true +SpaceBeforeParens: ControlStatements +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 1 +SpacesInAngles: false +SpacesInCStyleCastParentheses: false +SpacesInContainerLiterals: false +SpacesInParentheses: false +SpacesInSquareBrackets: false +BreakBeforeBinaryOperators: None +PointerAlignment: Left +SortIncludes: false diff --git a/circle.yml b/circle.yml index 30634619ba..4fc4600322 100644 --- a/circle.yml +++ b/circle.yml @@ -337,12 +337,13 @@ step-library: jobs: nitpick: docker: - - image: mbgl/7d2403f42e:linux-clang-4 + - image: mbgl/clang-6.0:2018-09-24-UfwJCwhu working_directory: /src environment: LIBSYSCONFCPUS: 4 JOBS: 4 BUILDTYPE: Debug + CLANG_FORMAT: clang-format-6.0 steps: - checkout - *restore-node_modules-cache @@ -355,6 +356,10 @@ jobs: name: Verify submodule pin command: scripts/nitpick/submodule-pin.js when: always + - run: + name: Code formatting + command: scripts/nitpick/format.js + when: always - run: name: CMake file list generation command: scripts/nitpick/generated-code.js cmake diff --git a/package.json b/package.json index 7ddbad1997..35ee0eea6c 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "npm-run-all": "^4.0.2" }, "devDependencies": { - "@octokit/rest": "^15.9.2", + "@octokit/rest": "^15.12.0", "aws-sdk": "^2.285.1", "csscolorparser": "^1.0.2", "ejs": "^2.4.1", @@ -29,6 +29,7 @@ "jsonwebtoken": "^8.3.0", "lodash": "^4.16.4", "mapbox-gl-styles": "2.0.2", + "parse-diff": "^0.4.2", "pixelmatch": "^4.0.2", "pngjs": "^3.0.0", "pretty-bytes": "^5.1.0", diff --git a/scripts/environment.js b/scripts/environment.js index cb533a74a8..785fc6221c 100755 --- a/scripts/environment.js +++ b/scripts/environment.js @@ -6,29 +6,42 @@ const github = require('@octokit/rest')(); const {execSync} = require('child_process'); -const pr = process.env['CIRCLE_PULL_REQUEST']; -const head = process.env['CIRCLE_SHA1'] || 'HEAD'; -if (pr) { - const number = +pr.match(/\/(\d+)\/?$/)[1]; - return github.pullRequests.get({ - owner: 'mapbox', - repo: 'mapbox-gl-native', - number - }).then(({data}) => { +let environment = module.exports = async function() { + const head = process.env['CIRCLE_SHA1'] || 'HEAD'; + + if (process.env['CIRCLE_TARGET_BRANCH'] && process.env['CIRCLE_MERGE_BASE']) { + return { + head, + base: process.env['CIRCLE_TARGET_BRANCH'], + mergeBase: process.env['CIRCLE_MERGE_BASE'] + }; + } + + const pr = process.env['CIRCLE_PULL_REQUEST']; + if (pr) { + const number = +pr.match(/\/(\d+)\/?$/)[1]; + const data = (await github.pullRequests.get({ owner: 'mapbox', repo: 'mapbox-gl-native', number })).data; const base = data.base.ref; const mergeBase = execSync(`git merge-base origin/${base} ${head}`).toString().trim(); - - console.log(`export CIRCLE_TARGET_BRANCH=${base}`); - console.log(`export CIRCLE_MERGE_BASE=${mergeBase}`); - }); -} else { - for (const sha of execSync(`git rev-list --max-count=10 ${head}`).toString().trim().split('\n')) { - const base = execSync(`git branch -r --contains ${sha} origin/master origin/release-*`).toString().split('\n')[0].trim().replace(/^origin\//, ''); - if (base.match(/^(master|release-[a-z]+)$/)) { - const mergeBase = execSync(`git merge-base origin/${base} ${head}`).toString().trim(); - console.log(`export CIRCLE_TARGET_BRANCH=${base}`); - console.log(`export CIRCLE_MERGE_BASE=${mergeBase}`); - break; + return { head, base, mergeBase }; + } else { + for (const sha of execSync(`git rev-list --max-count=10 ${head}`).toString().trim().split('\n')) { + const base = execSync(`git branch -r --contains ${sha} origin/master origin/release-*`).toString().split('\n')[0].trim().replace(/^origin\//, ''); + if (base.match(/^(master|release-[a-z]+)$/)) { + const mergeBase = execSync(`git merge-base origin/${base} ${head}`).toString().trim(); + return { head, base, mergeBase }; + } } + return { head, base: null, mergeBase: null }; } } + +async function main() { + const env = await environment(); + console.log(`export CIRCLE_TARGET_BRANCH=${env.base}`); + console.log(`export CIRCLE_MERGE_BASE=${env.mergeBase}`); +} + +if (require.main === module) { + main(); +} diff --git a/scripts/nitpick/format.js b/scripts/nitpick/format.js new file mode 100755 index 0000000000..1b79537ba6 --- /dev/null +++ b/scripts/nitpick/format.js @@ -0,0 +1,129 @@ +#!/usr/bin/env node + +const jwt = require('jsonwebtoken'); +const github = require('@octokit/rest')(); +const fs = require('fs'); + +const {execSync} = require('child_process'); +const parseDiff = require('parse-diff'); + +const SIZE_CHECK_APP_ID = 14028; +const SIZE_CHECK_APP_INSTALLATION_ID = 229425; + +process.on('unhandledRejection', error => { + console.log(error); + process.exit(1) +}); + +async function getAnnotations() { + const env = await require('../environment')(); + if (!env.mergeBase) { + console.log('No merge base available.'); + return; + } + + let annotations = []; + parseDiff(execSync(`git diff ${env.mergeBase} ${env.head} --unified=0 --dst-prefix='' *.cpp *.hpp`).toString()).forEach(file => { + // Can't use git diff's exclude syntax just yet because Circle CI's git is too old. + if (/^vendor\//.test(file.to)) { + return; + } + + const chunks = file.chunks.filter(chunk => chunk.newLines || !chunk.oldLines); + if (!chunks.length) { + return; + } + + const lines = chunks.map(chunk => { + const start = chunk.newStart; + const end = chunk.newStart + (chunk.newLines ? chunk.newLines - 1 : 0); + return `-lines=${start}:${end}`; + }); + + const formatted = execSync([ '${CLANG_FORMAT:-clang-format}', lines.join(' '), file.to].join(' ')).toString(); + + // diff exits with code 1 if the files differ, and code 2 if something went wrong. + parseDiff(execSync(`diff --unified=0 <(git show ${env.head}:${file.to}) - || [ $? -eq 1 ]`, { input: formatted, shell: '/bin/bash' }).toString()).forEach(diff => { + diff.chunks.forEach(function (chunk) { + const start = chunk.oldStart; + const end = chunk.oldStart + (chunk.oldLines ? chunk.oldLines - 1 : 0); + annotations.push({ + path: file.to, + start_line: start, + end_line: end, + annotation_level: 'notice', + title: `consider adjusting the code formatting:`, + message: chunk.changes.map(change => change.content).join('\n') + }); + }); + }); + }); + + return annotations; +} + +async function postToGithub() { + const annotations = await getAnnotations(); + + const key = Buffer.from(pk, 'base64').toString('binary'); + const payload = { + exp: Math.floor(Date.now() / 1000) + 60, + iat: Math.floor(Date.now() / 1000), + iss: SIZE_CHECK_APP_ID + }; + + const token = jwt.sign(payload, key, {algorithm: 'RS256'}); + github.authenticate({type: 'app', token}); + + github.apps.createInstallationToken({installation_id: SIZE_CHECK_APP_INSTALLATION_ID}) + .then(({data}) => { + github.authenticate({type: 'token', token: data.token}); + return github.checks.create({ + owner: 'mapbox', + repo: 'mapbox-gl-native', + name: 'Code Formatting', + head_branch: process.env['CIRCLE_BRANCH'], + head_sha: process.env['CIRCLE_SHA1'], + status: 'completed', + conclusion: 'neutral', + completed_at: new Date().toISOString(), + output: { + title: 'Code Formatting', + summary: 'Proposed code formatting changes', + annotations: annotations + } + }); + }); + +} + +const colors = { + file: (text) => `\x1b[1m\x1b[39m${text}\x1b[0m`, + removal: (text) => `\x1b[31m${text}\x1b[0m`, + addition: (text) => `\x1b[32m${text}\x1b[0m` +}; + +async function printToCommandLine() { + const annotations = await getAnnotations(); + + for (const annotation of annotations) { + console.log(colors.file(`${annotation.path}:${annotation.start_line}`)); + for (const line of annotation.message.split('\n')) { + if (line[0] === '-') { + console.log(colors.removal(line.replace(/\s+$/, '\x1b[97;41m$&'))); + } else if (line[0] === '+') { + console.log(colors.addition(line.replace(/\s+$/, '\x1b[97;42m$&'))); + } else { + console.log(line); + } + } + console.log(''); + } +} + +const pk = process.env['SIZE_CHECK_APP_PRIVATE_KEY']; +if (pk) { + postToGithub(); +} else { + printToCommandLine(); +} -- cgit v1.2.1