Skip to content

Commit

Permalink
Use linter exit code for GitHub check status
Browse files Browse the repository at this point in the history 
Closes #10
  • Loading branch information
@samuelmeuli
samuelmeuli committed Jan 8, 2020
1 parent 41c7427 commit 73e9574
Show file tree
Hide file tree
Showing 28 changed files with 607 additions and 530 deletions.
17 changes: 7 additions & 10 deletions src/github.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const { name: actionName } = require("../package");
const { getEnv, getInput, log } = require("./utils/action");
const request = require("./utils/request");

const ANNOTATION_LEVELS = ["notice", "warning", "failure"];

/**
* Returns information about the GitHub repository and action trigger event
*
Expand Down Expand Up @@ -51,25 +49,24 @@ function getContext() {

/**
* Creates a new check on GitHub which annotates the relevant commit with linting errors
*
* @param checkName {string}: Name which will be displayed in the check list
* @param sha {string}: SHA of the commit which should be annotated
* @param context {{actor: string, branch: string, event: object, eventName: string, repository:
* string, token: string, username: string, workspace: string}}: Object information about the GitHub
* repository and action trigger event
* @param results {object[]}: Results from the linter execution
* @param lintResult {{isSuccess: boolean, warning: [], error: []}}: Parsed lint result
* @param summary {string}: Summary for the GitHub check
*/
async function createCheck(checkName, sha, context, results, summary) {
async function createCheck(checkName, sha, context, lintResult, summary) {
let annotations = [];
for (let level = 0; level < 3; level += 1) {
for (const level of ["warning", "error"]) {
annotations = [
...annotations,
...results[level].map(result => ({
...lintResult[level].map(result => ({
path: result.path,
start_line: result.firstLine,
end_line: result.lastLine,
annotation_level: ANNOTATION_LEVELS[level],
annotation_level: level === "warning" ? "warning" : "failure",
message: result.message,
})),
];
Expand All @@ -78,15 +75,15 @@ async function createCheck(checkName, sha, context, results, summary) {
// Only use the first 50 annotations (limit for a single API request)
if (annotations.length > 50) {
log(
`There are more than 50 errors/warnings from ${checkName}. Annotations are created for the first 50 results only.`,
`There are more than 50 errors/warnings from ${checkName}. Annotations are created for the first 50 violations only.`,
);
annotations = annotations.slice(0, 50);
}

const body = {
name: checkName,
head_sha: sha,
conclusion: results[2].length === 0 ? "success" : "failure",
conclusion: lintResult.isSuccess ? "success" : "failure",
output: {
title: checkName,
summary,
Expand Down
29 changes: 18 additions & 11 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,24 @@ async function runAction() {
log(
`Linting ${autoFix ? "and auto-fixing " : ""}files in ${lintDirAbs} with ${linter.name}…`,
);
const results = linter.lint(lintDirAbs, fileExtList, autoFix);
const resultsParsed = linter.parseResults(context.workspace, results);
const lintOutput = linter.lint(lintDirAbs, fileExtList, autoFix);

// Parse output of linting command
const lintResult = linter.parseOutput(context.workspace, lintOutput);
log(
`Linting result of ${linter.name} is considered a ${
lintResult.isSuccess ? "success" : "failure"
}`,
);

// Build and log a summary of linting errors/warnings
let summary;
if (resultsParsed[1].length > 0 && resultsParsed[2].length > 0) {
summary = `Found ${resultsParsed[2].length} errors and ${resultsParsed[1].length} warnings with ${linter.name}`;
} else if (resultsParsed[2].length > 0) {
summary = `Found ${resultsParsed[2].length} errors with ${linter.name}`;
} else if (resultsParsed[1].length > 0) {
summary = `Found ${resultsParsed[1].length} warnings with ${linter.name}`;
if (lintResult.warning.length > 0 && lintResult.error.length > 0) {
summary = `Found ${lintResult.error.length} errors and ${lintResult.warning.length} warnings with ${linter.name}`;
} else if (lintResult.error.length > 0) {
summary = `Found ${lintResult.error.length} errors with ${linter.name}`;
} else if (lintResult.warning.length > 0) {
summary = `Found ${lintResult.warning.length} warnings with ${linter.name}`;
} else {
summary = `No code style issues found with ${linter.name}`;
}
Expand All @@ -79,7 +86,7 @@ async function runAction() {
git.pushChanges(context);
}

checks.push({ checkName: linter.name, resultsParsed, summary });
checks.push({ checkName: linter.name, lintResult, summary });
}
}

Expand All @@ -89,8 +96,8 @@ async function runAction() {
log(""); // Create empty line in logs
const headSha = git.getHeadSha();
await Promise.all(
checks.map(({ checkName, resultsParsed, summary }) =>
github.createCheck(checkName, headSha, context, resultsParsed, summary),
checks.map(({ checkName, lintResult, summary }) =>
github.createCheck(checkName, headSha, context, lintResult, summary),
),
);
}
Expand Down
42 changes: 23 additions & 19 deletions src/linters/black.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const commandExists = require("../../vendor/command-exists");
const { run } = require("../utils/action");
const { diffToParsedResults } = require("../utils/diff");
const { parseErrorsFromDiff } = require("../utils/diff");
const { initLintResult } = require("../utils/lint-result");

/**
* https://black.readthedocs.io
Expand All @@ -11,9 +12,7 @@ class Black {
}

/**
* Verifies that all required programs are installed. Exits the GitHub action if one of the
* programs is missing
*
* Verifies that all required programs are installed. Throws an error if programs are missing
* @param {string} dir: Directory to run the linting program in
*/
static async verifySetup(dir) {
Expand All @@ -30,28 +29,33 @@ class Black {

/**
* Runs the linting program and returns the command output
*
* @param {string} dir: Directory to run the linting program in
* @param {string[]} extensions: Array of file extensions which should be linted
* @param {string} dir: Directory to run the linter in
* @param {string[]} extensions: File extensions which should be linted
* @param {boolean} fix: Whether the linter should attempt to fix code style issues automatically
* @returns {string}: Results of the linting process
* @returns {{status: number, stdout: string, stderr: string}}: Output of the lint command
*/
static lint(dir, extensions, fix = false) {
return run(`black ${fix ? "" : "--diff"} --include "^.*\\.(${extensions.join("|")})$" "."`, {
dir,
ignoreErrors: true,
}).stdout;
return run(
`black ${fix ? "" : "--check --diff"} --include "^.*\\.(${extensions.join("|")})$" "."`,
{
dir,
ignoreErrors: true,
},
);
}

/**
* Parses the results of the linting process and returns it as a processable array
*
* @param {string} dir: Directory in which the linting program has been run
* @param {string} results: Results of the linting process
* @returns {object[]}: Parsed results
* Parses the output of the lint command. Determines the success of the lint process and the
* severity of the identified code style violations
* @param {string} dir: Directory in which the linter has been run
* @param {{status: number, stdout: string, stderr: string}} output: Output of the lint command
* @returns {{isSuccess: boolean, warning: [], error: []}}: Parsed lint result
*/
static parseResults(dir, results) {
return diffToParsedResults(results);
static parseOutput(dir, output) {
const lintResult = initLintResult();
lintResult.error = parseErrorsFromDiff(output.stdout);
lintResult.isSuccess = output.status === 0;
return lintResult;
}
}

Expand Down
49 changes: 26 additions & 23 deletions src/linters/eslint.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const commandExists = require("../../vendor/command-exists");
const { run } = require("../utils/action");
const { initLintResult } = require("../utils/lint-result");
const { removeTrailingPeriod } = require("../utils/string");

/**
Expand All @@ -11,9 +12,7 @@ class ESLint {
}

/**
* Verifies that all required programs are installed. Exits the GitHub action if one of the
* programs is missing
*
* Verifies that all required programs are installed. Throws an error if programs are missing
* @param {string} dir: Directory to run the linting program in
*/
static async verifySetup(dir) {
Expand All @@ -32,11 +31,10 @@ class ESLint {

/**
* Runs the linting program and returns the command output
*
* @param {string} dir: Directory to run the linting program in
* @param {string[]} extensions: Array of file extensions which should be linted
* @param {string} dir: Directory to run the linter in
* @param {string[]} extensions: File extensions which should be linted
* @param {boolean} fix: Whether the linter should attempt to fix code style issues automatically
* @returns {string}: Results of the linting process
* @returns {{status: number, stdout: string, stderr: string}}: Output of the lint command
*/
static lint(dir, extensions, fix = false) {
return run(
Expand All @@ -47,37 +45,42 @@ class ESLint {
dir,
ignoreErrors: true,
},
).stdout;
);
}

/**
* Parses the results of the linting process and returns it as a processable array
*
* @param {string} dir: Directory in which the linting program has been run
* @param {string} results: Results of the linting process
* @returns {object[]}: Parsed results
* Parses the output of the lint command. Determines the success of the lint process and the
* severity of the identified code style violations
* @param {string} dir: Directory in which the linter has been run
* @param {{status: number, stdout: string, stderr: string}} output: Output of the lint command
* @returns {{isSuccess: boolean, warning: [], error: []}}: Parsed lint result
*/
static parseResults(dir, results) {
const resultsJson = JSON.parse(results);

// Parsed results: [notices, warnings, failures]
const resultsParsed = [[], [], []];
static parseOutput(dir, output) {
const lintResult = initLintResult();
lintResult.isSuccess = output.status === 0;

for (const result of resultsJson) {
const { filePath, messages } = result;
const outputJson = JSON.parse(output.stdout);
for (const violation of outputJson) {
const { filePath, messages } = violation;
const path = filePath.substring(dir.length + 1);
for (const msg of messages) {
const { line, message, ruleId, severity } = msg;
resultsParsed[severity].push({
const entry = {
path,
firstLine: line,
lastLine: line,
message: `${removeTrailingPeriod(message)} (${ruleId})`,
});
};
if (severity === 1) {
lintResult.warning.push(entry);
}
if (severity === 2) {
lintResult.error.push(entry);
}
}
}

return resultsParsed;
return lintResult;
}
}

Expand Down
37 changes: 17 additions & 20 deletions src/linters/flake8.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const commandExists = require("../../vendor/command-exists");
const { sep } = require("path");
const { log, run } = require("../utils/action");
const { initLintResult } = require("../utils/lint-result");
const { capitalizeFirstLetter } = require("../utils/string");

const PARSE_REGEX = /^(.*):([0-9]+):[0-9]+: (\w*) (.*)$/gm;
Expand All @@ -14,9 +15,7 @@ class Flake8 {
}

/**
* Verifies that all required programs are installed. Exits the GitHub action if one of the
* programs is missing
*
* Verifies that all required programs are installed. Throws an error if programs are missing
* @param {string} dir: Directory to run the linting program in
*/
static async verifySetup(dir) {
Expand All @@ -33,11 +32,10 @@ class Flake8 {

/**
* Runs the linting program and returns the command output
*
* @param {string} dir: Directory to run the linting program in
* @param {string[]} extensions: Array of file extensions which should be linted
* @param {string} dir: Directory to run the linter in
* @param {string[]} extensions: File extensions which should be linted
* @param {boolean} fix: Whether the linter should attempt to fix code style issues automatically
* @returns {string}: Results of the linting process
* @returns {{status: number, stdout: string, stderr: string}}: Output of the lint command
*/
static lint(dir, extensions, fix = false) {
if (fix) {
Expand All @@ -46,35 +44,34 @@ class Flake8 {
return run(`flake8 --filename ${extensions.map(ext => `"**${sep}*.${ext}"`).join(",")}`, {
dir,
ignoreErrors: true,
}).stdout;
});
}

/**
* Parses the results of the linting process and returns it as a processable array
*
* @param {string} dir: Directory in which the linting program has been run
* @param {string} results: Results of the linting process
* @returns {object[]}: Parsed results
* Parses the output of the lint command. Determines the success of the lint process and the
* severity of the identified code style violations
* @param {string} dir: Directory in which the linter has been run
* @param {{status: number, stdout: string, stderr: string}} output: Output of the lint command
* @returns {{isSuccess: boolean, warning: [], error: []}}: Parsed lint result
*/
static parseResults(dir, results) {
const matches = results.matchAll(PARSE_REGEX);

// Parsed results: [notices, warnings, failures]
const resultsParsed = [[], [], []];
static parseOutput(dir, output) {
const lintResult = initLintResult();
lintResult.isSuccess = output.status === 0;

const matches = output.stdout.matchAll(PARSE_REGEX);
for (const match of matches) {
const [_, pathFull, line, rule, text] = match;
const path = pathFull.substring(2); // Remove "./" or ".\" from start of path
const lineNr = parseInt(line, 10);
resultsParsed[2].push({
lintResult.error.push({
path,
firstLine: lineNr,
lastLine: lineNr,
message: `${capitalizeFirstLetter(text)} (${rule})`,
});
}

return resultsParsed;
return lintResult;
}
}

Expand Down
Loading

0 comments on commit 73e9574

Please sign in to comment.