From 35f914f9e26cfafbe4f945cb58ac7bb2d1142bcb Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Tue, 3 Jan 2023 23:27:38 +0100 Subject: [PATCH 1/3] Add task-tags setting Programmers often leave comments to themselves and others such as: # TODO: Use a faster algorithm? The keywords used to prefix such comments are just a convention and vary from project to project. Other common keywords include FIXME and HACK. The keywords in use for the codebase are of interest to ruff because ruff does also lint comments. For example the ERA lint detects commented-out code but ignores comments starting with such a keyword. Previously the ERA lint simply hardcoded the regular expression TODO|FIXME|XXX to achieve that. This commit introduces a new `task-tags` setting to make this configurable (and to allow other comment lints to recognize the same set of keywords). The term "task tags" has probably been popularized by the Eclipse IDE.[1] For Python there has been the proposal PEP 350[2], which referred to such keywords as "codetags". That proposal however has been rejected. We are choosing the term "task tags" over "code tags" because the former is more descriptive: a task tag describes a task. While according to the PEP 350 such keywords are also sometimes used for non-tasks e.g. NOBUG to describe a well-known problem that will never be addressed due to design problems or domain limitations, such keywords are so rare that we are neglecting them here in favor of more descriptive terminology. The vast majority of such keywords does describe tasks, so naming the setting "task-tags" is apt. [1]: https://www.eclipse.org/pdt/help/html/task_tags.htm [2]: https://peps.python.org/pep-0350/ Co-authored-by: Charlie Marsh --- README.md | 20 +++ flake8_to_ruff/src/converter.rs | 7 ++ ruff.schema.json | 10 ++ src/eradicate/checks.rs | 2 +- src/eradicate/detection.rs | 210 ++++++++++++++++++-------------- src/lib_wasm.rs | 1 + src/settings/configuration.rs | 3 + src/settings/mod.rs | 6 + src/settings/options.rs | 10 ++ src/settings/pyproject.rs | 6 + 10 files changed, 184 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index 17392d9d2f183..1b72d09421f78 100644 --- a/README.md +++ b/README.md @@ -2212,6 +2212,26 @@ target-version = "py37" --- +#### [`task-tags`](#task-tags) + +A list of task tags to recognize (e.g., "TODO", "FIXME", "XXX"). + +Comments starting with these tags will be ignored by commented-out code +detection (`ERA`). + +**Default value**: `["TODO", "FIXME", "XXX"]` + +**Type**: `Vec` + +**Example usage**: + +```toml +[tool.ruff] +task-tags = ["HACK"] +``` + +--- + #### [`unfixable`](#unfixable) A list of check code prefixes to consider un-autofix-able. diff --git a/flake8_to_ruff/src/converter.rs b/flake8_to_ruff/src/converter.rs index 861a4cea6b695..7a95d19c39d79 100644 --- a/flake8_to_ruff/src/converter.rs +++ b/flake8_to_ruff/src/converter.rs @@ -390,6 +390,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -450,6 +451,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -510,6 +512,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -570,6 +573,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -630,6 +634,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -699,6 +704,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -762,6 +768,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, diff --git a/ruff.schema.json b/ruff.schema.json index 895c7c930a625..038e01c80ce42 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -366,6 +366,16 @@ } ] }, + "task-tags": { + "description": "A list of task tags to recognize (e.g., \"TODO\", \"FIXME\", \"XXX\").\n\nComments starting with these tags will be ignored by commented-out code detection (`ERA`).", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "unfixable": { "description": "A list of check code prefixes to consider un-autofix-able.", "type": [ diff --git a/src/eradicate/checks.rs b/src/eradicate/checks.rs index 5cefb29b3a6e8..a47604a769a23 100644 --- a/src/eradicate/checks.rs +++ b/src/eradicate/checks.rs @@ -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.task_tags[..]) { let mut check = Check::new(CheckKind::CommentedOutCode, Range::new(start, end)); if matches!(autofix, flags::Autofix::Enabled) && settings.fixable.contains(&CheckCode::ERA001) diff --git a/src/eradicate/detection.rs b/src/eradicate/detection.rs index 4d368f9800cad..68b996549e779 100644 --- a/src/eradicate/detection.rs +++ b/src/eradicate/detection.rs @@ -4,7 +4,7 @@ use regex::Regex; static ALLOWLIST_REGEX: Lazy = 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 = Lazy::new(|| Regex::new(r"^[()\[\]{}\s]+$").unwrap()); @@ -30,7 +30,7 @@ static PARTIAL_DICTIONARY_REGEX: Lazy = static PRINT_RETURN_REGEX: Lazy = 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, task_tags: &[String]) -> bool { let line = if let Some(line) = line.trim().strip_prefix('#') { line.trim() } else { @@ -47,6 +47,12 @@ pub fn comment_contains_code(line: &str) -> bool { return false; } + if let Some(first) = line.split(&[' ', ':']).next() { + if task_tags.iter().any(|tag| tag == first) { + return false; + } + } + if CODING_COMMENT_REGEX.is_match(line) { return false; } @@ -97,142 +103,166 @@ 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:", &[])); // Unpacking assignments assert!(comment_contains_code( - "# user_content_type, _ = TimelineEvent.objects.using(db_alias).get_or_create(" - )); + "# user_content_type, _ = TimelineEvent.objects.using(db_alias).get_or_create(", + &[] + ),); assert!(comment_contains_code( - "# (user_content_type, _) = TimelineEvent.objects.using(db_alias).get_or_create(" - )); + "# (user_content_type, _) = TimelineEvent.objects.using(db_alias).get_or_create(", + &[] + ),); assert!(comment_contains_code( - "# ( user_content_type , _ )= TimelineEvent.objects.using(db_alias).get_or_create(" + "# ( user_content_type , _ )= TimelineEvent.objects.using(db_alias).get_or_create(", + &[] )); assert!(comment_contains_code( - "# app_label=\"core\", model=\"user\"" + "# app_label=\"core\", model=\"user\"", + &[] )); - assert!(comment_contains_code("# )")); + assert!(comment_contains_code("# )", &[])); // 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")); } } diff --git a/src/lib_wasm.rs b/src/lib_wasm.rs index 6a949dbbab329..5307b9823de25 100644 --- a/src/lib_wasm.rs +++ b/src/lib_wasm.rs @@ -112,6 +112,7 @@ pub fn defaultSettings() -> Result { show_source: None, src: None, unfixable: None, + task_tags: None, update_check: None, // Use default options for all plugins. flake8_annotations: Some(flake8_annotations::settings::Settings::default().into()), diff --git a/src/settings/configuration.rs b/src/settings/configuration.rs index 370d813113cf9..20cc9054d783b 100644 --- a/src/settings/configuration.rs +++ b/src/settings/configuration.rs @@ -52,6 +52,7 @@ pub struct Configuration { pub src: Option>, pub target_version: Option, pub unfixable: Option>, + pub task_tags: Option>, pub update_check: Option, // Plugins pub flake8_annotations: Option, @@ -149,6 +150,7 @@ impl Configuration { .transpose()?, target_version: options.target_version, unfixable: options.unfixable, + task_tags: options.task_tags, update_check: options.update_check, // Plugins flake8_annotations: options.flake8_annotations, @@ -209,6 +211,7 @@ impl Configuration { src: self.src.or(config.src), target_version: self.target_version.or(config.target_version), unfixable: self.unfixable.or(config.unfixable), + task_tags: self.task_tags.or(config.task_tags), update_check: self.update_check.or(config.update_check), // Plugins flake8_annotations: self.flake8_annotations.or(config.flake8_annotations), diff --git a/src/settings/mod.rs b/src/settings/mod.rs index ce10aa0fa2d15..13afcea517d60 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -60,6 +60,7 @@ pub struct Settings { pub show_source: bool, pub src: Vec, pub target_version: PythonVersion, + pub task_tags: Vec, pub update_check: bool, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, @@ -173,6 +174,9 @@ impl Settings { .src .unwrap_or_else(|| vec![project_root.to_path_buf()]), target_version: config.target_version.unwrap_or_default(), + task_tags: config.task_tags.unwrap_or_else(|| { + vec!["TODO".to_string(), "FIXME".to_string(), "XXX".to_string()] + }), update_check: config.update_check.unwrap_or(true), // Plugins flake8_annotations: config @@ -252,6 +256,7 @@ impl Settings { show_source: false, src: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, + task_tags: vec!["TODO".to_string(), "FIXME".to_string()], update_check: false, flake8_annotations: flake8_annotations::settings::Settings::default(), flake8_bugbear: flake8_bugbear::settings::Settings::default(), @@ -291,6 +296,7 @@ impl Settings { show_source: false, src: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, + task_tags: vec!["TODO".to_string()], update_check: false, flake8_annotations: flake8_annotations::settings::Settings::default(), flake8_bugbear: flake8_bugbear::settings::Settings::default(), diff --git a/src/settings/options.rs b/src/settings/options.rs index 8d23c6e028644..071eebf5e3451 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -339,6 +339,16 @@ pub struct Options { )] /// A list of check code prefixes to consider un-autofix-able. pub unfixable: Option>, + #[option( + default = r#"["TODO", "FIXME", "XXX"]"#, + value_type = "Vec", + example = r#"task-tags = ["HACK"]"# + )] + /// A list of task tags to recognize (e.g., "TODO", "FIXME", "XXX"). + /// + /// Comments starting with these tags will be ignored by commented-out code + /// detection (`ERA`). + pub task_tags: Option>, #[option( default = "true", value_type = "bool", diff --git a/src/settings/pyproject.rs b/src/settings/pyproject.rs index 8c02c31248dc2..ead566352003c 100644 --- a/src/settings/pyproject.rs +++ b/src/settings/pyproject.rs @@ -188,6 +188,7 @@ mod tests { src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -241,6 +242,7 @@ line-length = 79 src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, cache_dir: None, flake8_annotations: None, @@ -296,6 +298,7 @@ exclude = ["foo.py"] src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_errmsg: None, @@ -350,6 +353,7 @@ select = ["E501"] src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -405,6 +409,7 @@ ignore = ["E501"] src: None, target_version: None, unfixable: None, + task_tags: None, update_check: None, flake8_annotations: None, flake8_bugbear: None, @@ -490,6 +495,7 @@ other-attribute = 1 format: None, force_exclude: None, unfixable: None, + task_tags: None, update_check: None, cache_dir: None, per_file_ignores: Some(FxHashMap::from_iter([( From c207404823d0e25a51888dca3235df27a8c22a48 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Wed, 4 Jan 2023 00:08:19 +0100 Subject: [PATCH 2/3] Add pycodestyle::settings This step is split up into a separate commit so that the following commit has a cleaner diff. --- README.md | 2 ++ flake8_to_ruff/src/converter.rs | 7 +++++++ ruff.schema.json | 15 +++++++++++++++ src/lib_wasm.rs | 3 ++- src/pycodestyle/mod.rs | 1 + src/pycodestyle/settings.rs | 26 ++++++++++++++++++++++++++ src/settings/configuration.rs | 5 ++++- src/settings/mod.rs | 9 ++++++++- src/settings/options.rs | 5 ++++- src/settings/pyproject.rs | 6 ++++++ 10 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 src/pycodestyle/settings.rs diff --git a/README.md b/README.md index 1b72d09421f78..38ae3fc8efbf8 100644 --- a/README.md +++ b/README.md @@ -2978,6 +2978,8 @@ staticmethod-decorators = ["staticmethod", "stcmthd"] --- +### `pycodestyle` + ### `pydocstyle` #### [`convention`](#convention) diff --git a/flake8_to_ruff/src/converter.rs b/flake8_to_ruff/src/converter.rs index 7a95d19c39d79..79b629091e790 100644 --- a/flake8_to_ruff/src/converter.rs +++ b/flake8_to_ruff/src/converter.rs @@ -403,6 +403,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); @@ -464,6 +465,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); @@ -525,6 +527,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); @@ -586,6 +589,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); @@ -652,6 +656,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); @@ -717,6 +722,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: Some(pydocstyle::settings::Options { convention: Some(Convention::Numpy), }), @@ -786,6 +792,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }); diff --git a/ruff.schema.json b/ruff.schema.json index 038e01c80ce42..6e1604100831e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -288,6 +288,17 @@ } } }, + "pycodestyle": { + "description": "Options for the `pycodestyle` plugin.", + "anyOf": [ + { + "$ref": "#/definitions/Pycodestyle" + }, + { + "type": "null" + } + ] + }, "pydocstyle": { "description": "Options for the `pydocstyle` plugin.", "anyOf": [ @@ -1448,6 +1459,10 @@ }, "additionalProperties": false }, + "Pycodestyle": { + "type": "object", + "additionalProperties": false + }, "Pydocstyle": { "type": "object", "properties": { diff --git a/src/lib_wasm.rs b/src/lib_wasm.rs index 5307b9823de25..806c39e4b9b87 100644 --- a/src/lib_wasm.rs +++ b/src/lib_wasm.rs @@ -18,7 +18,7 @@ use crate::source_code_style::SourceCodeStyleDetector; use crate::{ directives, flake8_annotations, flake8_bugbear, flake8_errmsg, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort, - mccabe, pep8_naming, pydocstyle, pyupgrade, + mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade, }; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -130,6 +130,7 @@ pub fn defaultSettings() -> Result { isort: Some(isort::settings::Settings::default().into()), mccabe: Some(mccabe::settings::Settings::default().into()), pep8_naming: Some(pep8_naming::settings::Settings::default().into()), + pycodestyle: Some(pycodestyle::settings::Settings::default().into()), pydocstyle: Some(pydocstyle::settings::Settings::default().into()), pyupgrade: Some(pyupgrade::settings::Settings::default().into()), })?) diff --git a/src/pycodestyle/mod.rs b/src/pycodestyle/mod.rs index 8027c14f3950e..36faa8b942690 100644 --- a/src/pycodestyle/mod.rs +++ b/src/pycodestyle/mod.rs @@ -1,5 +1,6 @@ pub mod checks; pub mod plugins; +pub mod settings; #[cfg(test)] mod tests { diff --git a/src/pycodestyle/settings.rs b/src/pycodestyle/settings.rs new file mode 100644 index 0000000000000..ed37731d8814d --- /dev/null +++ b/src/pycodestyle/settings.rs @@ -0,0 +1,26 @@ +//! Settings for the `pycodestyle` plugin. + +use ruff_macros::ConfigurationOptions; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive( + Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema, +)] +#[serde(deny_unknown_fields, rename_all = "kebab-case", rename = "Pycodestyle")] +pub struct Options {} + +#[derive(Debug, Default, Hash)] +pub struct Settings {} + +impl From for Settings { + fn from(options: Options) -> Self { + Self {} + } +} + +impl From for Options { + fn from(settings: Settings) -> Self { + Self {} + } +} diff --git a/src/settings/configuration.rs b/src/settings/configuration.rs index 20cc9054d783b..77c48bf9c9c19 100644 --- a/src/settings/configuration.rs +++ b/src/settings/configuration.rs @@ -22,7 +22,7 @@ use crate::settings::types::{ use crate::{ flake8_annotations, flake8_bugbear, flake8_errmsg, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, fs, isort, - mccabe, pep8_naming, pydocstyle, pyupgrade, + mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade, }; #[derive(Debug, Default)] @@ -66,6 +66,7 @@ pub struct Configuration { pub isort: Option, pub mccabe: Option, pub pep8_naming: Option, + pub pycodestyle: Option, pub pydocstyle: Option, pub pyupgrade: Option, } @@ -164,6 +165,7 @@ impl Configuration { isort: options.isort, mccabe: options.mccabe, pep8_naming: options.pep8_naming, + pycodestyle: options.pycodestyle, pydocstyle: options.pydocstyle, pyupgrade: options.pyupgrade, }) @@ -229,6 +231,7 @@ impl Configuration { isort: self.isort.or(config.isort), mccabe: self.mccabe.or(config.mccabe), pep8_naming: self.pep8_naming.or(config.pep8_naming), + pycodestyle: self.pycodestyle.or(config.pycodestyle), pydocstyle: self.pydocstyle.or(config.pydocstyle), pyupgrade: self.pyupgrade.or(config.pyupgrade), } diff --git a/src/settings/mod.rs b/src/settings/mod.rs index 13afcea517d60..a5b38829561e5 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -26,7 +26,7 @@ use crate::settings::types::{ use crate::{ flake8_annotations, flake8_bugbear, flake8_errmsg, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort, - mccabe, one_time_warning, pep8_naming, pydocstyle, pyupgrade, + mccabe, one_time_warning, pep8_naming, pycodestyle, pydocstyle, pyupgrade, }; pub mod configuration; @@ -74,6 +74,7 @@ pub struct Settings { pub isort: isort::settings::Settings, pub mccabe: mccabe::settings::Settings, pub pep8_naming: pep8_naming::settings::Settings, + pub pycodestyle: pycodestyle::settings::Settings, pub pydocstyle: pydocstyle::settings::Settings, pub pyupgrade: pyupgrade::settings::Settings, } @@ -223,6 +224,10 @@ impl Settings { .pep8_naming .map(std::convert::Into::into) .unwrap_or_default(), + pycodestyle: config + .pycodestyle + .map(std::convert::Into::into) + .unwrap_or_default(), pydocstyle: config .pydocstyle .map(std::convert::Into::into) @@ -269,6 +274,7 @@ impl Settings { isort: isort::settings::Settings::default(), mccabe: mccabe::settings::Settings::default(), pep8_naming: pep8_naming::settings::Settings::default(), + pycodestyle: pycodestyle::settings::Settings::default(), pydocstyle: pydocstyle::settings::Settings::default(), pyupgrade: pyupgrade::settings::Settings::default(), } @@ -309,6 +315,7 @@ impl Settings { isort: isort::settings::Settings::default(), mccabe: mccabe::settings::Settings::default(), pep8_naming: pep8_naming::settings::Settings::default(), + pycodestyle: pycodestyle::settings::Settings::default(), pydocstyle: pydocstyle::settings::Settings::default(), pyupgrade: pyupgrade::settings::Settings::default(), } diff --git a/src/settings/options.rs b/src/settings/options.rs index 071eebf5e3451..b4d31ebdfb071 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -10,7 +10,7 @@ use crate::settings::types::{PythonVersion, SerializationFormat, Version}; use crate::{ flake8_annotations, flake8_bugbear, flake8_errmsg, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort, - mccabe, pep8_naming, pydocstyle, pyupgrade, + mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade, }; #[derive( @@ -391,6 +391,9 @@ pub struct Options { /// Options for the `pep8-naming` plugin. pub pep8_naming: Option, #[option_group] + /// Options for the `pycodestyle` plugin. + pub pycodestyle: Option, + #[option_group] /// Options for the `pydocstyle` plugin. pub pydocstyle: Option, #[option_group] diff --git a/src/settings/pyproject.rs b/src/settings/pyproject.rs index ead566352003c..cb21804745b9a 100644 --- a/src/settings/pyproject.rs +++ b/src/settings/pyproject.rs @@ -201,6 +201,7 @@ mod tests { isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }) @@ -256,6 +257,7 @@ line-length = 79 isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }) @@ -311,6 +313,7 @@ exclude = ["foo.py"] isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }) @@ -366,6 +369,7 @@ select = ["E501"] isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }) @@ -422,6 +426,7 @@ ignore = ["E501"] isort: None, mccabe: None, pep8_naming: None, + pycodestyle: None, pydocstyle: None, pyupgrade: None, }) @@ -598,6 +603,7 @@ other-attribute = 1 ]), staticmethod_decorators: Some(vec!["staticmethod".to_string()]), }), + pycodestyle: None, pydocstyle: None, pyupgrade: None, } From b66fa53753c325e3aeafaba53388d4a89221b890 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Wed, 4 Jan 2023 00:20:34 +0100 Subject: [PATCH 3/3] Add ignore-overlong-task-comments setting Imagine a .py file containing the following comment: # TODO: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed # do eiusmod tempor incididunt ut labore et dolore magna aliqua. Since `git grep` only matches individual lines `git grep TODO` would only output the first line of the comment, cutting off potentially important information. (git grep currently doesn't support multiline grepping). Projects using such a workflow therefore probably format the comment in a single line instead: # TODO: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. This commit introduces a setting to accomdate this workflow by making the line-length checks (`E501`) optionally ignore overlong lines if they start with a recognized task tag. Co-authored-by: Charlie Marsh --- README.md | 22 +++++- resources/test/fixtures/pycodestyle/E501_1.py | 6 ++ ruff.schema.json | 11 ++- src/checkers/lines.rs | 2 +- src/pycodestyle/checks.rs | 28 ++++--- src/pycodestyle/mod.rs | 18 +++++ src/pycodestyle/settings.rs | 28 ++++++- ...__pycodestyle__tests__task_tags_false.snap | 77 +++++++++++++++++++ ...f__pycodestyle__tests__task_tags_true.snap | 6 ++ src/settings/options.rs | 3 +- 10 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 resources/test/fixtures/pycodestyle/E501_1.py create mode 100644 src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_false.snap create mode 100644 src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_true.snap diff --git a/README.md b/README.md index 38ae3fc8efbf8..fcffe125b3fc7 100644 --- a/README.md +++ b/README.md @@ -2217,7 +2217,8 @@ target-version = "py37" A list of task tags to recognize (e.g., "TODO", "FIXME", "XXX"). Comments starting with these tags will be ignored by commented-out code -detection (`ERA`). +detection (`ERA`), and skipped by line-length checks (`E501`) if +`ignore-overlong-task-comments` is set to `true`. **Default value**: `["TODO", "FIXME", "XXX"]` @@ -2980,6 +2981,25 @@ staticmethod-decorators = ["staticmethod", "stcmthd"] ### `pycodestyle` +#### [`ignore-overlong-task-comments`](#ignore-overlong-task-comments) + +Whether or not line-length checks (`E501`) should be triggered for +comments starting with `task-tags` (by default: ["TODO", "FIXME", +and "XXX"]). + +**Default value**: `false` + +**Type**: `bool` + +**Example usage**: + +```toml +[tool.ruff.pycodestyle] +ignore-overlong-task-comments = true +``` + +--- + ### `pydocstyle` #### [`convention`](#convention) diff --git a/resources/test/fixtures/pycodestyle/E501_1.py b/resources/test/fixtures/pycodestyle/E501_1.py new file mode 100644 index 0000000000000..6c9e62fa81a0d --- /dev/null +++ b/resources/test/fixtures/pycodestyle/E501_1.py @@ -0,0 +1,6 @@ +# TODO: comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` +# TODO comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` +# TODO comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` +# FIXME: comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` +# FIXME comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` +# FIXME comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep` diff --git a/ruff.schema.json b/ruff.schema.json index 6e1604100831e..9c8990bd6ea9b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -378,7 +378,7 @@ ] }, "task-tags": { - "description": "A list of task tags to recognize (e.g., \"TODO\", \"FIXME\", \"XXX\").\n\nComments starting with these tags will be ignored by commented-out code detection (`ERA`).", + "description": "A list of task tags to recognize (e.g., \"TODO\", \"FIXME\", \"XXX\").\n\nComments starting with these tags will be ignored by commented-out code detection (`ERA`), and skipped by line-length checks (`E501`) if `ignore-overlong-task-comments` is set to `true`.", "type": [ "array", "null" @@ -1461,6 +1461,15 @@ }, "Pycodestyle": { "type": "object", + "properties": { + "ignore-overlong-task-comments": { + "description": "Whether or not line-length checks (`E501`) should be triggered for comments starting with `task-tags` (by default: [\"TODO\", \"FIXME\", and \"XXX\"]).", + "type": [ + "boolean", + "null" + ] + } + }, "additionalProperties": false }, "Pydocstyle": { diff --git a/src/checkers/lines.rs b/src/checkers/lines.rs index 2d244ef932795..6cd8d5397fa35 100644 --- a/src/checkers/lines.rs +++ b/src/checkers/lines.rs @@ -57,7 +57,7 @@ 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) { checks.push(check); } } diff --git a/src/pycodestyle/checks.rs b/src/pycodestyle/checks.rs index c4f5f893367be..d62dc13879b28 100644 --- a/src/pycodestyle/checks.rs +++ b/src/pycodestyle/checks.rs @@ -8,34 +8,44 @@ use crate::ast::helpers::except_range; use crate::ast::types::Range; use crate::autofix::Fix; use crate::registry::{Check, CheckKind}; +use crate::settings::Settings; use crate::source_code_locator::SourceCodeLocator; static URL_REGEX: Lazy = Lazy::new(|| Regex::new(r"^https?://\S+$").unwrap()); /// E501 -pub fn line_too_long(lineno: usize, line: &str, max_line_length: usize) -> Option { +pub fn line_too_long(lineno: usize, line: &str, settings: &Settings) -> Option { let line_length = line.chars().count(); - if line_length <= max_line_length { + if line_length <= settings.line_length { return None; } let mut chunks = line.split_whitespace(); - let (Some(first), Some(_)) = (chunks.next(), chunks.next()) else { + let (Some(first), Some(second)) = (chunks.next(), chunks.next()) else { // Single word / no printable chars - no way to make the line shorter return None; }; - // Do not enforce the line length for commented lines that end with a URL - // or contain only a single word. - if first == "#" && chunks.last().map_or(true, |c| URL_REGEX.is_match(c)) { - return None; + if first == "#" { + if settings.pycodestyle.ignore_overlong_task_comments { + let second = second.trim_end_matches(':'); + if settings.task_tags.iter().any(|tag| tag == second) { + return None; + } + } + + // Do not enforce the line length for commented lines that end with a URL + // or contain only a single word. + if chunks.last().map_or(true, |c| URL_REGEX.is_match(c)) { + return None; + } } Some(Check::new( - CheckKind::LineTooLong(line_length, max_line_length), + CheckKind::LineTooLong(line_length, settings.line_length), Range::new( - Location::new(lineno + 1, max_line_length), + Location::new(lineno + 1, settings.line_length), Location::new(lineno + 1, line_length), ), )) diff --git a/src/pycodestyle/mod.rs b/src/pycodestyle/mod.rs index 36faa8b942690..49ee8db613090 100644 --- a/src/pycodestyle/mod.rs +++ b/src/pycodestyle/mod.rs @@ -10,6 +10,7 @@ mod tests { use anyhow::Result; use test_case::test_case; + use super::settings::Settings; use crate::linter::test_path; use crate::registry::CheckCode; use crate::settings; @@ -57,4 +58,21 @@ mod tests { insta::assert_yaml_snapshot!(checks); Ok(()) } + + #[test_case(false)] + #[test_case(true)] + fn task_tags(ignore_overlong_task_comments: bool) -> Result<()> { + let snapshot = format!("task_tags_{ignore_overlong_task_comments}"); + let checks = test_path( + Path::new("./resources/test/fixtures/pycodestyle/E501_1.py"), + &settings::Settings { + pycodestyle: Settings { + ignore_overlong_task_comments, + }, + ..settings::Settings::for_rule(CheckCode::E501) + }, + )?; + insta::assert_yaml_snapshot!(snapshot, checks); + Ok(()) + } } diff --git a/src/pycodestyle/settings.rs b/src/pycodestyle/settings.rs index ed37731d8814d..a64e1e4426c8b 100644 --- a/src/pycodestyle/settings.rs +++ b/src/pycodestyle/settings.rs @@ -8,19 +8,39 @@ use serde::{Deserialize, Serialize}; Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema, )] #[serde(deny_unknown_fields, rename_all = "kebab-case", rename = "Pycodestyle")] -pub struct Options {} +pub struct Options { + #[option( + default = "false", + value_type = "bool", + example = r#" + ignore-overlong-task-comments = true + "# + )] + /// Whether or not line-length checks (`E501`) should be triggered for + /// comments starting with `task-tags` (by default: ["TODO", "FIXME", + /// and "XXX"]). + pub ignore_overlong_task_comments: Option, +} #[derive(Debug, Default, Hash)] -pub struct Settings {} +pub struct Settings { + pub ignore_overlong_task_comments: bool, +} impl From for Settings { fn from(options: Options) -> Self { - Self {} + Self { + ignore_overlong_task_comments: options + .ignore_overlong_task_comments + .unwrap_or_default(), + } } } impl From for Options { fn from(settings: Settings) -> Self { - Self {} + Self { + ignore_overlong_task_comments: Some(settings.ignore_overlong_task_comments), + } } } diff --git a/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_false.snap b/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_false.snap new file mode 100644 index 0000000000000..050ba2dede7ce --- /dev/null +++ b/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_false.snap @@ -0,0 +1,77 @@ +--- +source: src/pycodestyle/mod.rs +expression: checks +--- +- kind: + LineTooLong: + - 149 + - 88 + location: + row: 1 + column: 88 + end_location: + row: 1 + column: 149 + fix: ~ + parent: ~ +- kind: + LineTooLong: + - 148 + - 88 + location: + row: 2 + column: 88 + end_location: + row: 2 + column: 148 + fix: ~ + parent: ~ +- kind: + LineTooLong: + - 155 + - 88 + location: + row: 3 + column: 88 + end_location: + row: 3 + column: 155 + fix: ~ + parent: ~ +- kind: + LineTooLong: + - 150 + - 88 + location: + row: 4 + column: 88 + end_location: + row: 4 + column: 150 + fix: ~ + parent: ~ +- kind: + LineTooLong: + - 149 + - 88 + location: + row: 5 + column: 88 + end_location: + row: 5 + column: 149 + fix: ~ + parent: ~ +- kind: + LineTooLong: + - 156 + - 88 + location: + row: 6 + column: 88 + end_location: + row: 6 + column: 156 + fix: ~ + parent: ~ + diff --git a/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_true.snap b/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_true.snap new file mode 100644 index 0000000000000..803588cfbe0fc --- /dev/null +++ b/src/pycodestyle/snapshots/ruff__pycodestyle__tests__task_tags_true.snap @@ -0,0 +1,6 @@ +--- +source: src/pycodestyle/mod.rs +expression: checks +--- +[] + diff --git a/src/settings/options.rs b/src/settings/options.rs index b4d31ebdfb071..094cfe9dcc79a 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -347,7 +347,8 @@ pub struct Options { /// A list of task tags to recognize (e.g., "TODO", "FIXME", "XXX"). /// /// Comments starting with these tags will be ignored by commented-out code - /// detection (`ERA`). + /// detection (`ERA`), and skipped by line-length checks (`E501`) if + /// `ignore-overlong-task-comments` is set to `true`. pub task_tags: Option>, #[option( default = "true",