Remove output format text and use format full by default (#12010)

Resolves #7349
This commit is contained in:
Micha Reiser
2024-06-26 09:40:18 +02:00
parent c0d2f439b7
commit 41203ea208
10 changed files with 317 additions and 72 deletions

View File

@@ -5,7 +5,7 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::sync::Arc;
use anyhow::bail;
use anyhow::{anyhow, bail};
use clap::builder::{TypedValueParser, ValueParserFactory};
use clap::{command, Parser};
use colored::Colorize;
@@ -21,7 +21,7 @@ use ruff_linter::settings::types::{
ExtensionPair, FilePattern, OutputFormat, PatternPrefixPair, PerFileIgnore, PreviewMode,
PythonVersion, UnsafeFixes,
};
use ruff_linter::{warn_user, RuleParser, RuleSelector, RuleSelectorParser};
use ruff_linter::{RuleParser, RuleSelector, RuleSelectorParser};
use ruff_source_file::{LineIndex, OneIndexed};
use ruff_text_size::TextRange;
use ruff_workspace::configuration::{Configuration, RuleSelection};
@@ -691,10 +691,7 @@ impl CheckCommand {
unsafe_fixes: resolve_bool_arg(self.unsafe_fixes, self.no_unsafe_fixes)
.map(UnsafeFixes::from),
force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
output_format: resolve_output_format(
self.output_format,
resolve_bool_arg(self.preview, self.no_preview).unwrap_or_default(),
),
output_format: resolve_output_format(self.output_format)?,
show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes),
extension: self.extension,
};
@@ -922,20 +919,15 @@ The path `{value}` does not point to a configuration file"
}
}
#[allow(deprecated)]
fn resolve_output_format(
output_format: Option<OutputFormat>,
preview: bool,
) -> Option<OutputFormat> {
Some(match output_format {
Some(o) => o,
None => return None
}).map(|format| match format {
OutputFormat::Text => {
warn_user!("`--output-format=text` is deprecated. Use `--output-format=full` or `--output-format=concise` instead. `text` will be treated as `{}`.", OutputFormat::default(preview));
OutputFormat::default(preview)
},
other => other
})
) -> anyhow::Result<Option<OutputFormat>> {
if let Some(OutputFormat::Text) = output_format {
Err(anyhow!("`--output-format=text` is no longer supported. Use `--output-format=full` or `--output-format=concise` instead."))
} else {
Ok(output_format)
}
}
/// CLI settings that are distinct from configuration (commands, lists of files,

View File

@@ -335,10 +335,10 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
let preview = pyproject_config.settings.linter.preview.is_enabled();
if cli.watch {
if output_format != OutputFormat::default(preview) {
if output_format != OutputFormat::default() {
warn_user!(
"`--output-format {}` is always used in watch mode.",
OutputFormat::default(preview)
OutputFormat::default()
);
}

View File

@@ -240,6 +240,7 @@ impl Printer {
}
if !self.flags.intersects(Flags::SHOW_VIOLATIONS) {
#[allow(deprecated)]
if matches!(
self.format,
OutputFormat::Text
@@ -275,8 +276,7 @@ impl Printer {
OutputFormat::Junit => {
JunitEmitter.emit(writer, &diagnostics.messages, &context)?;
}
OutputFormat::Concise
| OutputFormat::Full => {
OutputFormat::Concise | OutputFormat::Full => {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
.with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
@@ -324,6 +324,7 @@ impl Printer {
OutputFormat::Sarif => {
SarifEmitter.emit(writer, &diagnostics.messages, &context)?;
}
#[allow(deprecated)]
OutputFormat::Text => unreachable!("Text is deprecated and should have been automatically converted to the default serialization format")
}
@@ -367,6 +368,7 @@ impl Printer {
}
match self.format {
#[allow(deprecated)]
OutputFormat::Text | OutputFormat::Full | OutputFormat::Concise => {
// Compute the maximum number of digits in the count and code, for all messages,
// to enable pretty-printing.

View File

@@ -9,9 +9,9 @@ const BIN_NAME: &str = "ruff";
const STDIN: &str = "l = 1";
fn ruff_check(output_format: Option<String>) -> Command {
fn ruff_check(output_format: OutputFormat) -> Command {
let mut cmd = Command::new(get_cargo_bin(BIN_NAME));
let output_format = output_format.unwrap_or(format!("{}", OutputFormat::default(false)));
let output_format = output_format.to_string();
cmd.arg("check")
.arg("--output-format")
.arg(output_format)
@@ -22,15 +22,15 @@ fn ruff_check(output_format: Option<String>) -> Command {
}
#[test]
#[allow(deprecated)]
fn ensure_output_format_is_deprecated() {
assert_cmd_snapshot!(ruff_check(Some("text".into())).pass_stdin(STDIN), @r###"
assert_cmd_snapshot!(ruff_check(OutputFormat::Text).pass_stdin(STDIN), @r###"
success: false
exit_code: 1
exit_code: 2
----- stdout -----
-:1:1: E741 Ambiguous variable name: `l`
Found 1 error.
----- stderr -----
warning: `--output-format=text` is deprecated. Use `--output-format=full` or `--output-format=concise` instead. `text` will be treated as `concise`.
ruff failed
Cause: `--output-format=text` is no longer supported. Use `--output-format=full` or `--output-format=concise` instead.
"###);
}

View File

@@ -116,6 +116,12 @@ fn stdin_error() {
exit_code: 1
----- stdout -----
-:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -134,6 +140,12 @@ fn stdin_filename() {
exit_code: 1
----- stdout -----
F401.py:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -163,7 +175,19 @@ import bar # unused import
exit_code: 1
----- stdout -----
bar.py:2:8: F401 [*] `bar` imported but unused
|
2 | import bar # unused import
| ^^^ F401
|
= help: Remove unused import: `bar`
foo.py:2:8: F401 [*] `foo` imported but unused
|
2 | import foo # unused import
| ^^^ F401
|
= help: Remove unused import: `foo`
Found 2 errors.
[*] 2 fixable with the `--fix` option.
@@ -185,6 +209,12 @@ fn check_warn_stdin_filename_with_files() {
exit_code: 1
----- stdout -----
F401.py:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -205,6 +235,12 @@ fn stdin_source_type_py() {
exit_code: 1
----- stdout -----
TCH.py:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -436,6 +472,11 @@ fn stdin_fix_jupyter() {
}
----- stderr -----
Jupyter.ipynb:cell 3:1:7: F821 Undefined name `x`
|
1 | print(x)
| ^ F821
|
Found 3 errors (2 fixed, 1 remaining).
"###);
}
@@ -529,7 +570,19 @@ fn stdin_override_parser_ipynb() {
exit_code: 1
----- stdout -----
Jupyter.py:cell 1:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Jupyter.py:cell 3:1:8: F401 [*] `sys` imported but unused
|
1 | import sys
| ^^^ F401
|
= help: Remove unused import: `sys`
Found 2 errors.
[*] 2 fixable with the `--fix` option.
@@ -553,6 +606,12 @@ fn stdin_override_parser_py() {
exit_code: 1
----- stdout -----
F401.ipynb:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -575,6 +634,14 @@ fn stdin_fix_when_not_fixable_should_still_print_contents() {
----- stderr -----
-:3:4: F634 If test is a tuple, which is always `True`
|
1 | import sys
2 |
3 | if (1, 2):
| ^^^^^^ F634
4 | print(sys.version)
|
Found 2 errors (1 fixed, 1 remaining).
"###);
}
@@ -732,6 +799,11 @@ fn stdin_parse_error() {
exit_code: 1
----- stdout -----
-:1:16: E999 SyntaxError: Expected one or more symbol names after import
|
1 | from foo import
| ^ E999
|
Found 1 error.
----- stderr -----
@@ -748,7 +820,19 @@ fn stdin_multiple_parse_error() {
exit_code: 1
----- stdout -----
-:1:16: E999 SyntaxError: Expected one or more symbol names after import
|
1 | from foo import
| ^ E999
2 | bar =
|
-:2:6: E999 SyntaxError: Expected an expression
|
1 | from foo import
2 | bar =
| ^ E999
|
Found 2 errors.
----- stderr -----
@@ -1135,6 +1219,9 @@ fn redirect_direct() {
exit_code: 1
----- stdout -----
-:1:1: RUF950 Hey this is a test rule that was redirected from another.
|
|
Found 1 error.
----- stderr -----
@@ -1167,6 +1254,9 @@ fn redirect_prefix() {
exit_code: 1
----- stdout -----
-:1:1: RUF950 Hey this is a test rule that was redirected from another.
|
|
Found 1 error.
----- stderr -----
@@ -1184,6 +1274,9 @@ fn deprecated_direct() {
exit_code: 1
----- stdout -----
-:1:1: RUF920 Hey this is a deprecated test rule.
|
|
Found 1 error.
----- stderr -----
@@ -1201,7 +1294,13 @@ fn deprecated_multiple_direct() {
exit_code: 1
----- stdout -----
-:1:1: RUF920 Hey this is a deprecated test rule.
|
|
-:1:1: RUF921 Hey this is another deprecated test rule.
|
|
Found 2 errors.
----- stderr -----
@@ -1220,7 +1319,13 @@ fn deprecated_indirect() {
exit_code: 1
----- stdout -----
-:1:1: RUF920 Hey this is a deprecated test rule.
|
|
-:1:1: RUF921 Hey this is another deprecated test rule.
|
|
Found 2 errors.
----- stderr -----
@@ -1372,6 +1477,12 @@ fn check_input_from_argfile() -> Result<()> {
exit_code: 1
----- stdout -----
/path/to/a.py:1:8: F401 [*] `os` imported but unused
|
1 | import os
| ^^ F401
|
= help: Remove unused import: `os`
Found 1 error.
[*] 1 fixable with the `--fix` option.
@@ -1393,7 +1504,13 @@ fn check_hints_hidden_unsafe_fixes() {
exit_code: 1
----- stdout -----
-:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
@@ -1411,6 +1528,11 @@ fn check_hints_hidden_unsafe_fixes_with_no_safe_fixes() {
exit_code: 1
----- stdout -----
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
1 | x = {'a': 1, 'a': 1}
| RUF902
|
Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
@@ -1429,7 +1551,13 @@ fn check_no_hint_for_hidden_unsafe_fixes_when_disabled() {
exit_code: 1
----- stdout -----
-:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
[*] 1 fixable with the --fix option.
@@ -1449,6 +1577,11 @@ fn check_no_hint_for_hidden_unsafe_fixes_with_no_safe_fixes_when_disabled() {
exit_code: 1
----- stdout -----
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
1 | x = {'a': 1, 'a': 1}
| RUF902
|
Found 1 error.
----- stderr -----
@@ -1466,7 +1599,13 @@ fn check_shows_unsafe_fixes_with_opt_in() {
exit_code: 1
----- stdout -----
-:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 [*] Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
[*] 2 fixable with the --fix option.
@@ -1488,6 +1627,11 @@ fn fix_applies_safe_fixes_by_default() {
----- stderr -----
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
1 | # fix from stable-test-rule-safe-fix
| RUF902
|
Found 2 errors (1 fixed, 1 remaining).
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
"###);
@@ -1525,6 +1669,11 @@ fn fix_does_not_apply_display_only_fixes() {
def add_to_list(item, some_list=[]): ...
----- stderr -----
-:1:1: RUF903 Hey this is a stable test rule with a display only fix.
|
1 | def add_to_list(item, some_list=[]): ...
| RUF903
|
Found 1 error.
"###);
}
@@ -1543,6 +1692,11 @@ fn fix_does_not_apply_display_only_fixes_with_unsafe_fixes_enabled() {
def add_to_list(item, some_list=[]): ...
----- stderr -----
-:1:1: RUF903 Hey this is a stable test rule with a display only fix.
|
1 | def add_to_list(item, some_list=[]): ...
| RUF903
|
Found 1 error.
"###);
}
@@ -1560,6 +1714,9 @@ fn fix_only_unsafe_fixes_available() {
----- stderr -----
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
|
Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
"###);
@@ -1696,7 +1853,13 @@ extend-unsafe-fixes = ["RUF901"]
exit_code: 1
----- stdout -----
-:1:1: RUF901 Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).
@@ -1728,7 +1891,13 @@ extend-safe-fixes = ["RUF902"]
exit_code: 1
----- stdout -----
-:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 [*] Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
[*] 2 fixable with the `--fix` option.
@@ -1762,7 +1931,13 @@ extend-safe-fixes = ["RUF902"]
exit_code: 1
----- stdout -----
-:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
|
|
-:1:1: RUF902 Hey this is a stable test rule with an unsafe fix.
|
|
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
@@ -1798,12 +1973,61 @@ extend-safe-fixes = ["RUF9"]
exit_code: 1
----- stdout -----
-:1:1: RUF900 Hey this is a stable test rule.
|
1 | x = {'a': 1, 'a': 1}
| RUF900
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF901 Hey this is a stable test rule with a safe fix.
|
1 | x = {'a': 1, 'a': 1}
| RUF901
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF902 [*] Hey this is a stable test rule with an unsafe fix.
|
1 | x = {'a': 1, 'a': 1}
| RUF902
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF903 Hey this is a stable test rule with a display only fix.
|
1 | x = {'a': 1, 'a': 1}
| RUF903
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF920 Hey this is a deprecated test rule.
|
1 | x = {'a': 1, 'a': 1}
| RUF920
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF921 Hey this is another deprecated test rule.
|
1 | x = {'a': 1, 'a': 1}
| RUF921
2 | print(('foo'))
3 | print(str('foo'))
|
-:1:1: RUF950 Hey this is a test rule that was redirected from another.
|
1 | x = {'a': 1, 'a': 1}
| RUF950
2 | print(('foo'))
3 | print(str('foo'))
|
Found 7 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
@@ -1865,6 +2089,12 @@ def log(x, base) -> float:
exit_code: 1
----- stdout -----
-:2:5: D417 Missing argument description in the docstring for `log`: `base`
|
2 | def log(x, base) -> float:
| ^^^ D417
3 | """Calculate natural log of a value
|
Found 1 error.
----- stderr -----
@@ -1895,6 +2125,14 @@ select = ["RUF017"]
exit_code: 1
----- stdout -----
-:3:1: RUF017 Avoid quadratic list summation
|
1 | x = [1, 2, 3]
2 | y = [4, 5, 6]
3 | sum([x, y], [])
| ^^^^^^^^^^^^^^^ RUF017
|
= help: Replace with `functools.reduce`
Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
@@ -1927,6 +2165,14 @@ unfixable = ["RUF"]
exit_code: 1
----- stdout -----
-:3:1: RUF017 Avoid quadratic list summation
|
1 | x = [1, 2, 3]
2 | y = [4, 5, 6]
3 | sum([x, y], [])
| ^^^^^^^^^^^^^^^ RUF017
|
= help: Replace with `functools.reduce`
Found 1 error.
----- stderr -----

View File

@@ -17,7 +17,7 @@ Settings path: "[BASEPATH]/pyproject.toml"
cache_dir = "[BASEPATH]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_format = full
show_fixes = false
unsafe_fixes = hint

View File

@@ -1,3 +1,5 @@
#![allow(deprecated)]
use std::fmt::{Display, Formatter};
use std::hash::{Hash, Hasher};
use std::ops::Deref;
@@ -500,13 +502,19 @@ impl FromIterator<ExtensionPair> for ExtensionMapping {
}
}
#[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)]
#[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Debug, Hash, Default)]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[serde(rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum OutputFormat {
// Remove the module level `#![allow(deprecated)` when removing the text variant.
// Adding the `#[deprecated]` attribute to text creates clippy warnings about
// using a deprecated item in the derived code and there seems to be no way to suppress the clippy error
// other than disabling the warning for the entire module and/or moving `OutputFormat` to another module.
#[deprecated(note = "Use `concise` or `full` instead")]
Text,
Concise,
#[default]
Full,
Json,
JsonLines,
@@ -540,16 +548,6 @@ impl Display for OutputFormat {
}
}
impl OutputFormat {
pub fn default(preview: bool) -> Self {
if preview {
Self::Full
} else {
Self::Concise
}
}
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)]
#[serde(try_from = "String")]
pub struct RequiredVersion(VersionSpecifiers);

View File

@@ -220,9 +220,7 @@ impl Configuration {
fix: self.fix.unwrap_or(false),
fix_only: self.fix_only.unwrap_or(false),
unsafe_fixes: self.unsafe_fixes.unwrap_or_default(),
output_format: self
.output_format
.unwrap_or_else(|| OutputFormat::default(global_preview.is_enabled())),
output_format: self.output_format.unwrap_or_default(),
show_fixes: self.show_fixes.unwrap_or(false),
file_resolver: FileResolverSettings {
@@ -429,17 +427,16 @@ impl Configuration {
return Err(anyhow!("The `tab-size` option has been renamed to `indent-width` to emphasize that it configures the indentation used by the formatter as well as the tab width. Please update {config_to_update} to use `indent-width = <value>` instead."));
}
let output_format = {
options
.output_format
.map(|format| match format {
OutputFormat::Text => {
warn_user_once!(r#"Setting `output_format` to "text" is deprecated. Use "full" or "concise" instead. "text" will be treated as "{}"."#, OutputFormat::default(options.preview.unwrap_or_default()));
OutputFormat::default(options.preview.unwrap_or_default())
},
other => other
})
};
#[allow(deprecated)]
if options.output_format == Some(OutputFormat::Text) {
let config_to_update = path.map_or_else(
|| String::from("your `--config` CLI arguments"),
|path| format!("`{}`", fs::relativize_path(path)),
);
return Err(anyhow!(
r#"The option `output_format=text` is no longer supported. Update {config_to_update} to use `output-format="concise"` or `output-format="full"` instead."#
));
}
Ok(Self {
builtins: options.builtins,
@@ -505,7 +502,7 @@ impl Configuration {
fix: options.fix,
fix_only: options.fix_only,
unsafe_fixes: options.unsafe_fixes.map(UnsafeFixes::from),
output_format,
output_format: options.output_format,
force_exclude: options.force_exclude,
line_length: options.line_length,
indent_width: options.indent_width,

View File

@@ -44,7 +44,7 @@ impl Default for Settings {
cache_dir: cache_dir(project_root),
fix: false,
fix_only: false,
output_format: OutputFormat::default(false),
output_format: OutputFormat::default(),
show_fixes: false,
unsafe_fixes: UnsafeFixes::default(),
linter: LinterSettings::new(project_root),

40
ruff.schema.json generated
View File

@@ -2294,21 +2294,31 @@
"additionalProperties": false
},
"OutputFormat": {
"type": "string",
"enum": [
"text",
"concise",
"full",
"json",
"json-lines",
"junit",
"grouped",
"github",
"gitlab",
"pylint",
"rdjson",
"azure",
"sarif"
"oneOf": [
{
"type": "string",
"enum": [
"concise",
"full",
"json",
"json-lines",
"junit",
"grouped",
"github",
"gitlab",
"pylint",
"rdjson",
"azure",
"sarif"
]
},
{
"deprecated": true,
"type": "string",
"enum": [
"text"
]
}
]
},
"ParametrizeNameType": {