Skip to content

Commit

Permalink
Introduce comment-tags setting (and make E501 skip them)
Browse files Browse the repository at this point in the history
It's quite common for TODO comments to be longer than line-length
so that you can quickly display them in full with `git grep TODO`.

Previously the ERA001 (CommentedOutCode) lint already hardcoded
TODO|FIXME|XXX within a regular expression to skip such comments.

This commit introduces a new comment-tags setting to make the comment
tags that should be skipped by lints configurable and adapts the E501
(LineTooLong) lint to respect that new setting.
  • Loading branch information
not-my-profile committed Jan 2, 2023
1 parent 07e47be commit b5f0a40
Show file tree
Hide file tree
Showing 13 changed files with 190 additions and 88 deletions.
18 changes: 18 additions & 0 deletions README.md
Expand Up @@ -1726,6 +1726,24 @@ cache-dir = "~/.cache/ruff"
---
#### [`comment-tags`](#comment-tags)
A list of comment tags to recognize (e.g. TODO, FIXME, XXX).
Comments starting with these tags will be skipped by E501 and ERA001.
**Default value**: `["TODO", "FIXME", "XXX"]`
**Type**: `Vec<String>`
**Example usage**:
```toml
[tool.ruff]
comment-tags = ["TODO", "FIXME", "HACK"]
```
---
#### [`dummy-variable-rgx`](#dummy-variable-rgx)
A regular expression used to identify "dummy" variables, or those which
Expand Down
7 changes: 7 additions & 0 deletions flake8_to_ruff/src/converter.rs
Expand Up @@ -324,6 +324,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -383,6 +384,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -442,6 +444,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -501,6 +504,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -560,6 +564,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -663,6 +668,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down Expand Up @@ -725,6 +731,7 @@ mod tests {
src: None,
target_version: None,
unfixable: None,
comment_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bugbear: None,
Expand Down
5 changes: 5 additions & 0 deletions resources/test/fixtures/pycodestyle/E501.py
Expand Up @@ -60,3 +60,8 @@ def caller(string: str) -> None:
_ = """
Source: https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
"""

# OK
# TODO: comments starting with one of the configured comment-tags may be longer than line-length so that you can easily find them with `git grep`
# TODO comments starting with one of the configured comment-tags may be longer than line-length so that you can easily find them with `git grep`
# TODO comments starting with one of the configured comment-tags may be longer than line-length so that you can easily find them with `git grep`
10 changes: 10 additions & 0 deletions ruff.schema.json
Expand Up @@ -22,6 +22,16 @@
"null"
]
},
"comment-tags": {
"description": "A list of comment tags to recognize (e.g. TODO, FIXME, XXX). Comments starting with these tags will be skipped by E501 and ERA001.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
"dummy-variable-rgx": {
"description": "A regular expression used to identify \"dummy\" variables, or those which should be ignored when evaluating (e.g.) unused-variable checks. The default expression matches `_`, `__`, and `_var`, but not `_var_`.",
"type": [
Expand Down
4 changes: 3 additions & 1 deletion src/checkers/lines.rs
Expand Up @@ -57,7 +57,9 @@ pub fn check_lines(
}

if enforce_line_too_long {
if let Some(check) = line_too_long(index, line, settings.line_length) {
if let Some(check) =
line_too_long(index, line, settings.line_length, &settings.comment_tags)
{
checks.push(check);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/eradicate/checks.rs
Expand Up @@ -31,7 +31,7 @@ pub fn commented_out_code(
let line = locator.slice_source_code_range(&Range::new(location, end_location));

// Verify that the comment is on its own line, and that it contains code.
if is_standalone_comment(&line) && comment_contains_code(&line) {
if is_standalone_comment(&line) && comment_contains_code(&line, &settings.comment_tags[..]) {
let mut check = Check::new(CheckKind::CommentedOutCode, Range::new(start, end));
if matches!(autofix, flags::Autofix::Enabled)
&& settings.fixable.contains(&CheckCode::ERA001)
Expand Down
192 changes: 109 additions & 83 deletions src/eradicate/detection.rs
Expand Up @@ -4,7 +4,7 @@ use regex::Regex;

static ALLOWLIST_REGEX: Lazy<Regex> = Lazy::new(|| {
Regex::new(
r"^(?i)(?:pylint|pyright|noqa|nosec|type:\s*ignore|fmt:\s*(on|off)|isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?])?)|TODO|FIXME|XXX)"
r"^(?i)(?:pylint|pyright|noqa|nosec|type:\s*ignore|fmt:\s*(on|off)|isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?])?))"
).unwrap()
});
static BRACKET_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^[()\[\]{}\s]+$").unwrap());
Expand All @@ -30,7 +30,7 @@ static PARTIAL_DICTIONARY_REGEX: Lazy<Regex> =
static PRINT_RETURN_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(print|return)\b\s*").unwrap());

/// Returns `true` if a comment contains Python code.
pub fn comment_contains_code(line: &str) -> bool {
pub fn comment_contains_code(line: &str, comment_tags: &[String]) -> bool {
let line = if let Some(line) = line.trim().strip_prefix('#') {
line.trim()
} else {
Expand All @@ -47,6 +47,12 @@ pub fn comment_contains_code(line: &str) -> bool {
return false;
}

if let Some(first) = line.split(&[' ', ':']).next() {
if comment_tags.iter().any(|tag| tag == first) {
return false;
}
}

if CODING_COMMENT_REGEX.is_match(line) {
return false;
}
Expand Down Expand Up @@ -97,127 +103,147 @@ mod tests {

#[test]
fn comment_contains_code_basic() {
assert!(comment_contains_code("# x = 1"));
assert!(comment_contains_code("#from foo import eradicate"));
assert!(comment_contains_code("#import eradicate"));
assert!(comment_contains_code(r#"#"key": value,"#));
assert!(comment_contains_code(r#"#"key": "value","#));
assert!(comment_contains_code(r#"#"key": 1 + 1,"#));
assert!(comment_contains_code("#'key': 1 + 1,"));
assert!(comment_contains_code(r#"#"key": {"#));
assert!(comment_contains_code("#}"));
assert!(comment_contains_code("#} )]"));

assert!(!comment_contains_code("#"));
assert!(!comment_contains_code("# This is a (real) comment."));
assert!(!comment_contains_code("# 123"));
assert!(!comment_contains_code("# 123.1"));
assert!(!comment_contains_code("# 1, 2, 3"));
assert!(!comment_contains_code("x = 1 # x = 1"));
assert!(comment_contains_code("# x = 1", &[]));
assert!(comment_contains_code("#from foo import eradicate", &[]));
assert!(comment_contains_code("#import eradicate", &[]));
assert!(comment_contains_code(r#"#"key": value,"#, &[]));
assert!(comment_contains_code(r#"#"key": "value","#, &[]));
assert!(comment_contains_code(r#"#"key": 1 + 1,"#, &[]));
assert!(comment_contains_code("#'key': 1 + 1,", &[]));
assert!(comment_contains_code(r#"#"key": {"#, &[]));
assert!(comment_contains_code("#}", &[]));
assert!(comment_contains_code("#} )]", &[]));

assert!(!comment_contains_code("#", &[]));
assert!(!comment_contains_code("# This is a (real) comment.", &[]));
assert!(!comment_contains_code("# 123", &[]));
assert!(!comment_contains_code("# 123.1", &[]));
assert!(!comment_contains_code("# 1, 2, 3", &[]));
assert!(!comment_contains_code("x = 1 # x = 1", &[]));
assert!(!comment_contains_code(
"# pylint: disable=redefined-outer-name",
&[]
),);
assert!(!comment_contains_code(
"# pylint: disable=redefined-outer-name"
"# Issue #999: This is not code",
&[]
));
assert!(!comment_contains_code("# Issue #999: This is not code"));

// TODO(charlie): This should be `true` under aggressive mode.
assert!(!comment_contains_code("#},"));
assert!(!comment_contains_code("#},", &[]));
}

#[test]
fn comment_contains_code_with_print() {
assert!(comment_contains_code("#print"));
assert!(comment_contains_code("#print(1)"));
assert!(comment_contains_code("#print 1"));
assert!(comment_contains_code("#print", &[]));
assert!(comment_contains_code("#print(1)", &[]));
assert!(comment_contains_code("#print 1", &[]));

assert!(!comment_contains_code("#to print"));
assert!(!comment_contains_code("#to print", &[]));
}

#[test]
fn comment_contains_code_with_return() {
assert!(comment_contains_code("#return x"));
assert!(comment_contains_code("#return x", &[]));

assert!(!comment_contains_code("#to print"));
assert!(!comment_contains_code("#to print", &[]));
}

#[test]
fn comment_contains_code_with_multiline() {
assert!(comment_contains_code("#else:"));
assert!(comment_contains_code("# else : "));
assert!(comment_contains_code(r#"# "foo %d" % \\"#));
assert!(comment_contains_code("#elif True:"));
assert!(comment_contains_code("#x = foo("));
assert!(comment_contains_code("#except Exception:"));

assert!(!comment_contains_code("# this is = to that :("));
assert!(!comment_contains_code("#else"));
assert!(!comment_contains_code("#or else:"));
assert!(!comment_contains_code("#else True:"));
assert!(comment_contains_code("#else:", &[]));
assert!(comment_contains_code("# else : ", &[]));
assert!(comment_contains_code(r#"# "foo %d" % \\"#, &[]));
assert!(comment_contains_code("#elif True:", &[]));
assert!(comment_contains_code("#x = foo(", &[]));
assert!(comment_contains_code("#except Exception:", &[]));

assert!(!comment_contains_code("# this is = to that :(", &[]));
assert!(!comment_contains_code("#else", &[]));
assert!(!comment_contains_code("#or else:", &[]));
assert!(!comment_contains_code("#else True:", &[]));

// TODO(charlie): This should be `true` under aggressive mode.
assert!(!comment_contains_code("#def foo():"));
assert!(!comment_contains_code("#def foo():", &[]));
}

#[test]
fn comment_contains_code_with_sentences() {
assert!(!comment_contains_code("#code is good"));
assert!(!comment_contains_code("#code is good", &[]));
}

#[test]
fn comment_contains_code_with_encoding() {
assert!(comment_contains_code("# codings=utf-8"));
assert!(comment_contains_code("# codings=utf-8", &[]));

assert!(!comment_contains_code("# coding=utf-8"));
assert!(!comment_contains_code("#coding= utf-8"));
assert!(!comment_contains_code("# coding: utf-8"));
assert!(!comment_contains_code("# encoding: utf8"));
assert!(!comment_contains_code("# coding=utf-8", &[]));
assert!(!comment_contains_code("#coding= utf-8", &[]));
assert!(!comment_contains_code("# coding: utf-8", &[]));
assert!(!comment_contains_code("# encoding: utf8", &[]));
}

#[test]
fn comment_contains_code_with_default_allowlist() {
assert!(!comment_contains_code("# pylint: disable=A0123"));
assert!(!comment_contains_code("# pylint:disable=A0123"));
assert!(!comment_contains_code("# pylint: disable = A0123"));
assert!(!comment_contains_code("# pylint:disable = A0123"));
assert!(!comment_contains_code("# pyright: reportErrorName=true"));
assert!(!comment_contains_code("# noqa"));
assert!(!comment_contains_code("# NOQA"));
assert!(!comment_contains_code("# noqa: A123"));
assert!(!comment_contains_code("# noqa:A123"));
assert!(!comment_contains_code("# nosec"));
assert!(!comment_contains_code("# fmt: on"));
assert!(!comment_contains_code("# fmt: off"));
assert!(!comment_contains_code("# fmt:on"));
assert!(!comment_contains_code("# fmt:off"));
assert!(!comment_contains_code("# isort: on"));
assert!(!comment_contains_code("# isort:on"));
assert!(!comment_contains_code("# isort: off"));
assert!(!comment_contains_code("# isort:off"));
assert!(!comment_contains_code("# isort: skip"));
assert!(!comment_contains_code("# isort:skip"));
assert!(!comment_contains_code("# isort: skip_file"));
assert!(!comment_contains_code("# isort:skip_file"));
assert!(!comment_contains_code("# isort: split"));
assert!(!comment_contains_code("# isort:split"));
assert!(!comment_contains_code("# isort: dont-add-imports"));
assert!(!comment_contains_code("# isort:dont-add-imports"));
assert!(!comment_contains_code("# pylint: disable=A0123", &[]));
assert!(!comment_contains_code("# pylint:disable=A0123", &[]));
assert!(!comment_contains_code("# pylint: disable = A0123", &[]));
assert!(!comment_contains_code("# pylint:disable = A0123", &[]));
assert!(!comment_contains_code(
"# pyright: reportErrorName=true",
&[]
));
assert!(!comment_contains_code("# noqa", &[]));
assert!(!comment_contains_code("# NOQA", &[]));
assert!(!comment_contains_code("# noqa: A123", &[]));
assert!(!comment_contains_code("# noqa:A123", &[]));
assert!(!comment_contains_code("# nosec", &[]));
assert!(!comment_contains_code("# fmt: on", &[]));
assert!(!comment_contains_code("# fmt: off", &[]));
assert!(!comment_contains_code("# fmt:on", &[]));
assert!(!comment_contains_code("# fmt:off", &[]));
assert!(!comment_contains_code("# isort: on", &[]));
assert!(!comment_contains_code("# isort:on", &[]));
assert!(!comment_contains_code("# isort: off", &[]));
assert!(!comment_contains_code("# isort:off", &[]));
assert!(!comment_contains_code("# isort: skip", &[]));
assert!(!comment_contains_code("# isort:skip", &[]));
assert!(!comment_contains_code("# isort: skip_file", &[]));
assert!(!comment_contains_code("# isort:skip_file", &[]));
assert!(!comment_contains_code("# isort: split", &[]));
assert!(!comment_contains_code("# isort:split", &[]));
assert!(!comment_contains_code("# isort: dont-add-imports", &[]));
assert!(!comment_contains_code("# isort:dont-add-imports", &[]));
assert!(!comment_contains_code(
"# isort: dont-add-imports: [\"import os\"]",
&[]
));
assert!(!comment_contains_code(
"# isort:dont-add-imports: [\"import os\"]",
&[]
));
assert!(!comment_contains_code(
"# isort: dont-add-imports:[\"import os\"]",
&[]
));
assert!(!comment_contains_code(
"# isort: dont-add-imports: [\"import os\"]"
"# isort:dont-add-imports:[\"import os\"]",
&[]
));
assert!(!comment_contains_code("# type: ignore", &[]));
assert!(!comment_contains_code("# type:ignore", &[]));
assert!(!comment_contains_code("# type: ignore[import]", &[]));
assert!(!comment_contains_code("# type:ignore[import]", &[]));
assert!(!comment_contains_code(
"# isort:dont-add-imports: [\"import os\"]"
"# TODO: Do that",
&["TODO".to_string()]
));
assert!(!comment_contains_code(
"# isort: dont-add-imports:[\"import os\"]"
"# FIXME: Fix that",
&["FIXME".to_string()]
));
assert!(!comment_contains_code(
"# isort:dont-add-imports:[\"import os\"]"
"# XXX: What ever",
&["XXX".to_string()]
));
assert!(!comment_contains_code("# type: ignore"));
assert!(!comment_contains_code("# type:ignore"));
assert!(!comment_contains_code("# type: ignore[import]"));
assert!(!comment_contains_code("# type:ignore[import]"));
assert!(!comment_contains_code("# TODO: Do that"));
assert!(!comment_contains_code("# FIXME: Fix that"));
assert!(!comment_contains_code("# XXX: What ever"));
}
}
1 change: 1 addition & 0 deletions src/lib_wasm.rs
Expand Up @@ -112,6 +112,7 @@ pub fn defaultSettings() -> Result<JsValue, JsValue> {
show_source: None,
src: None,
unfixable: None,
comment_tags: None,
update_check: None,
// Use default options for all plugins.
flake8_annotations: Some(flake8_annotations::settings::Settings::default().into()),
Expand Down

0 comments on commit b5f0a40

Please sign in to comment.