Skip to content
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

Add ruff #620

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
- name: Install Python dependencies
run: |
cd ./test/linters/projects/
pip install -r ./autopep8/requirements.txt -r ./black/requirements.txt -r ./flake8/requirements.txt -r ./mypy/requirements.txt -r ./oitnb/requirements.txt -r ./pylint/requirements.txt
pip install -r ./autopep8/requirements.txt -r ./black/requirements.txt -r ./flake8/requirements.txt -r ./mypy/requirements.txt -r ./oitnb/requirements.txt -r ./pylint/requirements.txt -r ./ruff/requirements.txt

# Ruby

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ _**Note:** The behavior of actions like this one is currently limited in the con
- [Mypy](https://mypy.readthedocs.io/)
- [oitnb](https://pypi.org/project/oitnb/)
- [Pylint](https://pylint.pycqa.org)
- [Ruff](https://beta.ruff.rs)
- **Ruby:**
- [ERB Lint](https://github.com/Shopify/erb-lint)
- [RuboCop](https://rubocop.readthedocs.io)
Expand Down Expand Up @@ -443,6 +444,7 @@ Some options are not available for specific linters:
| prettier | ✅ | ✅ |
| pylint | ❌ | ❌ (py) |
| rubocop | ✅ | ❌ (rb) |
| ruff | ✅ | ❌ (py) |
| rustfmt | ✅ | ❌ (rs) |
| stylelint | ✅ | ✅ |
| swift_format_official | ✅ | ✅ |
Expand Down
24 changes: 24 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,30 @@ inputs:
required: false
default: "false"

ruff:
description: Enable or disable ruff checks
required: false
default: "false"
ruff_args:
description: Additional arguments to pass to the linter
required: false
default: ""
ruff_dir:
description: Directory where the ruff command should be run
required: false
ruff_extensions:
description: Extensions of files to check with ruff
required: false
default: "py"
ruff_command_prefix:
description: Shell command to prepend to the linter command
required: false
default: ""
ruff_auto_fix:
description: Whether this linter should try to fix code style issues automatically. If set to `true`, it will only work if "auto_fix" is set to `true` as well
required: false
default: "true"

# Ruby

rubocop:
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"@actions/core": "^1.10.0",
"command-exists": "^1.2.9",
"glob": "^8.1.0",
"parse-diff": "^0.11.0",
"parse-diff": "^0.11.1",
"shescape": "^1.6.4"
},
"peerDependencies": {},
Expand Down
2 changes: 2 additions & 0 deletions src/linters/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const PHPCodeSniffer = require("./php-codesniffer");
const Prettier = require("./prettier");
const Pylint = require("./pylint");
const RuboCop = require("./rubocop");
const Ruff = require("./ruff");
const RustFmt = require("./rustfmt");
const Stylelint = require("./stylelint");
const SwiftFormatLockwood = require("./swift-format-lockwood");
Expand Down Expand Up @@ -45,6 +46,7 @@ const linters = {
dotnet_format: DotnetFormat,
gofmt: Gofmt,
oitnb: Oitnb,
ruff: Ruff,
rustfmt: RustFmt,
prettier: Prettier,
swift_format_lockwood: SwiftFormatLockwood,
Expand Down
2 changes: 1 addition & 1 deletion src/linters/mypy.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Mypy {
if (!specifiedPath) {
extraArgs = ` ${dir}`;
}
return run(`${prefix} mypy ${args}${extraArgs}`, {
return run(`${prefix} mypy --no-color-output --no-error-summary ${args}${extraArgs}`, {
dir,
ignoreErrors: true,
});
Expand Down
91 changes: 91 additions & 0 deletions src/linters/ruff.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
const { sep } = require("path");

const { run } = require("../utils/action");
const commandExists = require("../utils/command-exists");
const { initLintResult } = require("../utils/lint-result");
const { capitalizeFirstLetter } = require("../utils/string");

const PARSE_REGEX = /^(.*):([0-9]+):[0-9]+: (\w*) (.*)$/gm;

/** @typedef {import('../utils/lint-result').LintResult} LintResult */

/**
* https://beta.ruff.rs
*/
class Ruff {
static get name() {
return "Ruff";
}

/**
* Verifies that all required programs are installed. Throws an error if programs are missing
* @param {string} dir - Directory to run the linting program in
* @param {string} prefix - Prefix to the lint command
*/
static async verifySetup(dir, prefix = "") {
// Verify that Python is installed (required to execute Ruff)
if (!(await commandExists("python"))) {
throw new Error("Python is not installed");
}

// Verify that Ruff is installed
try {
run(`${prefix} ruff --version`, { dir });
} catch (err) {
throw new Error(`${this.name} is not installed`);
}
}

/**
* Runs the linting program and returns the command output
* @param {string} dir - Directory to run the linter in
* @param {string[]} extensions - File extensions which should be linted
* @param {string} args - Additional arguments to pass to the linter
* @param {boolean} fix - Whether the linter should attempt to fix code style issues automatically
* @param {string} prefix - Prefix to the lint command
* @returns {{status: number, stdout: string, stderr: string}} - Output of the lint command
*/
static lint(dir, extensions, args = "", fix = false, prefix = "") {
if (extensions.length !== 1 || extensions[0] !== "py") {
throw new Error(`${this.name} error: File extensions are not configurable`);
}
const fixArg = fix ? "--fix-only --exit-non-zero-on-fix" : "";
return run(`${prefix} ruff check --quiet ${fixArg} ${args} .`, {
dir,
ignoreErrors: true,
});
}

/**
* 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 {LintResult} - Parsed lint result
*/
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 leadingSep = `.${sep}`;
let path = pathFull;
if (path.startsWith(leadingSep)) {
path = path.substring(2); // Remove "./" or ".\" from start of path
}
const lineNr = parseInt(line, 10);
lintResult.error.push({
path,
firstLine: lineNr,
lastLine: lineNr,
message: `${capitalizeFirstLetter(text)} (${rule})`,
});
}

return lintResult;
}
}

module.exports = Ruff;
2 changes: 2 additions & 0 deletions test/linters/linters.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const phpCodeSnifferParams = require("./params/php-codesniffer");
const prettierParams = require("./params/prettier");
const pylintParams = require("./params/pylint");
const ruboCopParams = require("./params/rubocop");
const ruffParams = require("./params/ruff");
const rustfmtParams = require("./params/rustfmt");
const stylelintParams = require("./params/stylelint");
const swiftFormatLockwood = require("./params/swift-format-lockwood");
Expand All @@ -44,6 +45,7 @@ const linterParams = [
prettierParams,
pylintParams,
ruboCopParams,
ruffParams,
rustfmtParams,
stylelintParams,
tscParams,
Expand Down
29 changes: 17 additions & 12 deletions test/linters/params/black.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
const { EOL } = require("os");
const { join } = require("path");

const Black = require("../../../src/linters/black");
const { TEST_DATE } = require("../../test-utils");

Expand All @@ -9,38 +12,40 @@ const extensions = ["py"];

// Linting without auto-fixing
function getLintParams(dir) {
const stdoutFile1 = `--- file1.py ${TEST_DATE}\n+++ file1.py ${TEST_DATE}\n@@ -1,10 +1,10 @@\n var_1 = "hello"\n var_2 = "world"\n \n \n-def main (): # Whitespace error\n+def main(): # Whitespace error\n print("hello " + var_2)\n \n \n def add(num_1, num_2):\n return num_1 + num_2\n@@ -19,8 +19,9 @@\n \n \n def divide(num_1, num_2):\n return num_1 / num_2\n \n+\n # Blank lines error\n \n main()`;
const stdoutFile2 = `--- file2.py ${TEST_DATE}\n+++ file2.py ${TEST_DATE}\n@@ -1,2 +1,2 @@\n def add(num_1, num_2):\n- return num_1 + num_2 # Indentation error\n+ return num_1 + num_2 # Indentation error`;
const absolutePathFile1 = join(dir, 'file1.py');
const stdoutFile1 = `--- ${absolutePathFile1} ${TEST_DATE}\n+++ ${absolutePathFile1} ${TEST_DATE}\n@@ -1,10 +1,10 @@\n var_1 = "hello"\n var_2 = "world"\n \n \n-def main (): # Whitespace error\n+def main(): # Whitespace error\n print("hello " + var_2)\n \n \n def add(num_1, num_2):\n return num_1 + num_2\n@@ -19,8 +19,9 @@\n \n \n def divide(num_1, num_2):\n return num_1 / num_2\n \n+\n # Blank lines error\n \n main()`;
const absolutePathFile2 = join(dir, 'file2.py');
pickfire marked this conversation as resolved.
Show resolved Hide resolved
const stdoutFile2 = `--- ${absolutePathFile2} ${TEST_DATE}\n+++ ${absolutePathFile2} ${TEST_DATE}\n@@ -1,2 +1,2 @@\n def add(num_1, num_2):\n- return num_1 + num_2 # Indentation error\n+ return num_1 + num_2 # Indentation error`;
return {
// Expected output of the linting function
cmdOutput: {
status: 1,
stdoutParts: [stdoutFile1, stdoutFile2],
stdout: `${stdoutFile1}\n \n${stdoutFile2}`,
stdoutParts: [stdoutFile2, stdoutFile1],
stdout: `${stdoutFile2}${EOL}${stdoutFile1}`,
},
// Expected output of the parsing function
lintResult: {
isSuccess: false,
warning: [],
error: [
{
path: "file1.py",
path: absolutePathFile2,
firstLine: 1,
lastLine: 3,
message: ` def add(num_1, num_2):\n- return num_1 + num_2 # Indentation error\n+ return num_1 + num_2 # Indentation error`,
},
{
path: absolutePathFile1,
firstLine: 1,
lastLine: 11,
message: ` var_1 = "hello"\n var_2 = "world"\n \n \n-def main (): # Whitespace error\n+def main(): # Whitespace error\n print("hello " + var_2)\n \n \n def add(num_1, num_2):\n return num_1 + num_2`,
},
{
path: "file1.py",
path: absolutePathFile1,
firstLine: 19,
lastLine: 27,
message: ` \n \n def divide(num_1, num_2):\n return num_1 / num_2\n \n+\n # Blank lines error\n \n main()`,
},
{
path: "file2.py",
firstLine: 1,
lastLine: 3,
message: ` def add(num_1, num_2):\n- return num_1 + num_2 # Indentation error\n+ return num_1 + num_2 # Indentation error`,
},
],
},
};
Expand Down
8 changes: 4 additions & 4 deletions test/linters/params/mypy.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const extensions = ["py"];

// Linting without auto-fixing
function getLintParams(dir) {
const stdoutPart1 = `file1.py:7: error: Dict entry 0 has incompatible type "str": "int"; expected "str": "str"`;
const stdoutPart2 = `file1.py:11: error: Argument 1 to "main" has incompatible type "List[str]"; expected "str"`;
const stdoutPart1 = `file1.py:7: error: Dict entry 0 has incompatible type "str": "int"; expected "str": "str" [dict-item]`;
const stdoutPart2 = `file1.py:11: error: Argument 1 to "main" has incompatible type "List[str]"; expected "str" [arg-type]`;
return {
// Expected output of the linting function
cmdOutput: {
Expand All @@ -28,13 +28,13 @@ function getLintParams(dir) {
path: "file1.py",
firstLine: 7,
lastLine: 7,
message: `Dict entry 0 has incompatible type "str": "int"; expected "str": "str"`,
message: `Dict entry 0 has incompatible type "str": "int"; expected "str": "str" [dict-item]`,
},
{
path: "file1.py",
firstLine: 11,
lastLine: 11,
message: `Argument 1 to "main" has incompatible type "List[str]"; expected "str"`,
message: `Argument 1 to "main" has incompatible type "List[str]"; expected "str" [arg-type]`,
},
],
},
Expand Down
67 changes: 67 additions & 0 deletions test/linters/params/ruff.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
const { EOL } = require("os");

const Ruff = require("../../../src/linters/ruff");

const testName = "ruff";
const linter = Ruff;
const args = "";
const commandPrefix = "";
const extensions = ["py"];

// Linting without auto-fixing
function getLintParams(dir) {
const stdoutFile1 = `file1.py:3:8: F401 [*] \`os\` imported but unused`;
const stdoutFile2 = `file2.py:1:4: F821 Undefined name \`a\`${EOL}file2.py:1:5: E701 Multiple statements on one line (colon)`;
return {
// Expected output of the linting function
cmdOutput: {
status: 1,
stdoutParts: [stdoutFile1, stdoutFile2],
stdout: `${stdoutFile1}${EOL}${stdoutFile2}`,
},
// Expected output of the parsing function
lintResult: {
isSuccess: false,
warning: [],
error: [
{
path: "file1.py",
firstLine: 3,
lastLine: 3,
message: "[*] `os` imported but unused (F401)",
},
{
path: "file2.py",
firstLine: 1,
lastLine: 1,
message: "Undefined name `a` (F821)",
},
{
path: "file2.py",
firstLine: 1,
lastLine: 1,
message: "Multiple statements on one line (colon) (E701)",
},
],
},
};
}

// Linting with auto-fixing
function getFixParams(dir) {
return {
// Expected output of the linting function
cmdOutput: {
status: 1,
stdout: "",
},
// Expected output of the parsing function
lintResult: {
isSuccess: false,
warning: [],
error: [],
},
};
}
Comment on lines +51 to +65

Choose a reason for hiding this comment

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

@pickfire is it possible that the expected values in the ruff + fix are wrong?

I ran the tests on my machine for ruff and without the following change the tests were failing:

function getFixParams(dir) {
	return {
		// Expected output of the linting function
		cmdOutput: {
			status: 0,
			stdout: "",
		},
		// Expected output of the parsing function
		lintResult: {
			isSuccess: true,
			warning: [],
			error: [],
		},
	};
}

the thing is the option --fix-only at

const fixArg = fix ? "--fix-only" : "";
return run(`${prefix} ruff check --quiet ${fixArg} ${args} .`, {
ruff doesn't report the violations:

--fix-only
          Fix any fixable lint violations, but don't report on leftover violations.

Here is my context:

$  uname -s -r -p -o 
Linux 5.19.0-41-generic x86_64 GNU/Linux

$ node --version
v16.20.0

$ jest --version
29.3.1

$ python --version
Python 3.10.11

$ pip freeze | grep ruff
ruff==0.0.267

Choose a reason for hiding this comment

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

Haven't looked at this PR closely, but FYI we recently changed --fix-only to always return a code of 0, unless you provide --exit-non-zero-on-fix, in which case, it'll exit 1 if it fixes any violations.

Relevant link to the breaking change list: https://github.com/charliermarsh/ruff/blob/main/BREAKING_CHANGES.md#--fix-only-now-exits-with-a-zero-exit-code-unless---exit-non-zero-on-fix-is-specified-4146


module.exports = [testName, linter, commandPrefix, extensions, args, getLintParams, getFixParams];
8 changes: 8 additions & 0 deletions test/linters/projects/ruff/file1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from typing import List

import os


def sum_even_numbers(numbers: List[int]) -> int:
"""Given a list of integers, return the sum of all even numbers in the list."""
return sum(num for num in numbers if num % 2 == 0)
1 change: 1 addition & 0 deletions test/linters/projects/ruff/file2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
if a: a = False
1 change: 1 addition & 0 deletions test/linters/projects/ruff/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ruff>=0.0.257
2 changes: 1 addition & 1 deletion test/test-utils.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { mkdtempSync, realpathSync } = require("fs");
const { join } = require("path");

const DATE_REGEX = /\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}.\d+ [+-]\d{4}/g;
const DATE_REGEX = /\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}.\d+\s*[+-](\d{4}|\d{2}:\d{2})/g;
const TEST_DATE = "2019-01-01 00:00:00.000000 +0000";

/**
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2613,10 +2613,10 @@ parent-module@^1.0.0:
dependencies:
callsites "^3.0.0"

parse-diff@^0.11.0:
version "0.11.0"
resolved "https://registry.yarnpkg.com/parse-diff/-/parse-diff-0.11.0.tgz#8b0394216aa2ae12fb532a907972aa152fcfd59e"
integrity sha512-9P+oyFJJU9jLd9/EEXf2nlyz6GNBxt2senI22/3ss1AVqf+ntxY9eNZJW9tqEyH8w8Ywfzhgixo061x70OCkzQ==
parse-diff@^0.11.1:
version "0.11.1"
resolved "https://registry.yarnpkg.com/parse-diff/-/parse-diff-0.11.1.tgz#d93ca2d225abed280782bccb1476711ca9dd84f0"
integrity sha512-Oq4j8LAOPOcssanQkIjxosjATBIEJhCxMCxPhMu+Ci4wdNmAEdx0O+a7gzbR2PyKXgKPvRLIN5g224+dJAsKHA==

parse-json@^5.2.0:
version "5.2.0"
Expand Down
Loading