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

Enable pycodestyle rules under new "nursery" category #4407

Merged
merged 1 commit into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
1,212 changes: 611 additions & 601 deletions crates/ruff/src/codes.rs

Large diffs are not rendered by default.

18 changes: 13 additions & 5 deletions crates/ruff/src/rule_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ impl Visitor<'_> for SelectorVisitor {

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str(
"expected a string code identifying a linter or specific rule, or a partial rule code \
or ALL to refer to all rules",
"expected a string code identifying a linter or specific rule, or a partial rule code or ALL to refer to all rules",
)
}

Expand All @@ -141,13 +140,22 @@ impl From<RuleCodePrefix> for RuleSelector {
}
}

/// Returns `true` if the given rule should be selected by the `RuleSelector::All` selector.
fn select_all(rule: Rule) -> bool {
// Nursery rules have to be explicitly selected, so we ignore them when looking at
// prefixes.
!rule.is_nursery()
}

impl IntoIterator for &RuleSelector {
type IntoIter = RuleSelectorIter;
type Item = Rule;
type IntoIter = RuleSelectorIter;

fn into_iter(self) -> Self::IntoIter {
match self {
RuleSelector::All => RuleSelectorIter::All(Rule::iter()),
RuleSelector::All => {
RuleSelectorIter::All(Rule::iter().filter(|rule| select_all(*rule)))
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
}
RuleSelector::C => RuleSelectorIter::Chain(
Linter::Flake8Comprehensions
.into_iter()
Expand All @@ -165,7 +173,7 @@ impl IntoIterator for &RuleSelector {
}

pub enum RuleSelectorIter {
All(RuleIter),
All(std::iter::Filter<RuleIter, fn(&Rule) -> bool>),
Chain(std::iter::Chain<std::vec::IntoIter<Rule>, std::vec::IntoIter<Rule>>),
Vec(std::vec::IntoIter<Rule>),
}
Expand Down
11 changes: 11 additions & 0 deletions crates/ruff_cli/src/commands/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ pub(crate) fn rule(rule: Rule, format: HelpFormat) -> Result<()> {
output.push('\n');
}

if rule.is_nursery() {
output.push_str(&format!(
r#"This rule is part of the **nursery**, a collection of newer lints that are
still under development. As such, it must be enabled by explicitly selecting
{}."#,
rule.noqa_code()
));
output.push('\n');
output.push('\n');
}

if let Some(explanation) = rule.explanation() {
output.push_str(explanation.trim());
} else {
Expand Down
76 changes: 68 additions & 8 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use path_absolutize::path_dedot;
const BIN_NAME: &str = "ruff";

#[test]
fn test_stdin_success() -> Result<()> {
fn stdin_success() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
cmd.args(["-", "--format", "text", "--isolated"])
.write_stdin("")
Expand All @@ -22,7 +22,7 @@ fn test_stdin_success() -> Result<()> {
}

#[test]
fn test_stdin_error() -> Result<()> {
fn stdin_error() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["-", "--format", "text", "--isolated"])
Expand All @@ -40,7 +40,7 @@ Found 1 error.
}

#[test]
fn test_stdin_filename() -> Result<()> {
fn stdin_filename() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args([
Expand All @@ -66,7 +66,7 @@ Found 1 error.

#[cfg(unix)]
#[test]
fn test_stdin_json() -> Result<()> {
fn stdin_json() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args([
Expand Down Expand Up @@ -127,7 +127,7 @@ fn test_stdin_json() -> Result<()> {
}

#[test]
fn test_stdin_autofix() -> Result<()> {
fn stdin_autofix() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["-", "--format", "text", "--fix", "--isolated"])
Expand All @@ -142,7 +142,7 @@ fn test_stdin_autofix() -> Result<()> {
}

#[test]
fn test_stdin_autofix_when_not_fixable_should_still_print_contents() -> Result<()> {
fn stdin_autofix_when_not_fixable_should_still_print_contents() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["-", "--format", "text", "--fix", "--isolated"])
Expand All @@ -157,7 +157,7 @@ fn test_stdin_autofix_when_not_fixable_should_still_print_contents() -> Result<(
}

#[test]
fn test_stdin_autofix_when_no_issues_should_still_print_contents() -> Result<()> {
fn stdin_autofix_when_no_issues_should_still_print_contents() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["-", "--format", "text", "--fix", "--isolated"])
Expand All @@ -172,7 +172,7 @@ fn test_stdin_autofix_when_no_issues_should_still_print_contents() -> Result<()>
}

#[test]
fn test_show_source() -> Result<()> {
fn show_source() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["-", "--format", "text", "--show-source", "--isolated"])
Expand Down Expand Up @@ -217,3 +217,63 @@ fn show_statistics() -> Result<()> {
);
Ok(())
}

#[test]
fn nursery_prefix() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--select E` should detect E741, but not E225, which is in the nursery.
let output = cmd
.args(["-", "--format", "text", "--isolated", "--select", "E"])
.write_stdin("I=42\n")
.assert()
.failure();
assert_eq!(
str::from_utf8(&output.get_output().stdout)?,
r#"-:1:1: E741 Ambiguous variable name: `I`
Found 1 error.
"#
);

Ok(())
}

#[test]
fn nursery_all() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--select ALL` should detect E741, but not E225, which is in the nursery.
let output = cmd
.args(["-", "--format", "text", "--isolated", "--select", "E"])
.write_stdin("I=42\n")
.assert()
.failure();
assert_eq!(
str::from_utf8(&output.get_output().stdout)?,
r#"-:1:1: E741 Ambiguous variable name: `I`
Found 1 error.
"#
);

Ok(())
}

#[test]
fn nursery_direct() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--select E225` should detect E225.
let output = cmd
.args(["-", "--format", "text", "--isolated", "--select", "E225"])
.write_stdin("I=42\n")
.assert()
.failure();
assert_eq!(
str::from_utf8(&output.get_output().stdout)?,
r#"-:1:2: E225 Missing whitespace around operator
Found 1 error.
"#
);

Ok(())
}
17 changes: 14 additions & 3 deletions crates/ruff_dev/src/generate_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ pub(crate) fn main(args: &Args) -> Result<()> {
output.push('\n');
}

if rule.is_nursery() {
output.push_str(&format!(
r#"This rule is part of the **nursery**, a collection of newer lints that are
still under development. As such, it must be enabled by explicitly selecting
{}."#,
rule.noqa_code()
));
output.push('\n');
output.push('\n');
}

process_documentation(explanation.trim(), &mut output);

let filename = PathBuf::from(ROOT_DIR)
Expand Down Expand Up @@ -116,7 +127,7 @@ mod tests {

#[test]
fn test_process_documentation() {
let mut out = String::new();
let mut output = String::new();
process_documentation(
"
See also [`mccabe.max-complexity`].
Expand All @@ -127,10 +138,10 @@ Something [`else`][other].
- `mccabe.max-complexity`

[other]: http://example.com.",
&mut out,
&mut output,
);
assert_eq!(
out,
output,
"
See also [`mccabe.max-complexity`][mccabe.max-complexity].
Something [`else`][other].
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_diagnostics/src/violation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::fmt::{Debug, Display};

#[derive(Copy, Clone)]
#[derive(Debug, Copy, Clone)]
pub enum AutofixKind {
Sometimes,
Always,
Expand Down Expand Up @@ -30,7 +30,7 @@ pub trait Violation: Debug + PartialEq + Eq {
None
}

// TODO micha: Move `autofix_title` to `Fix`, add new `advice` method that is shown as an advice.
// TODO(micha): Move `autofix_title` to `Fix`, add new `advice` method that is shown as an advice.
// Change the `Diagnostic` renderer to show the advice, and render the fix message after the `Suggested fix: <here>`

/// Returns the title for the autofix. The message is also shown as an advice as part of the diagnostics.
Expand Down
9 changes: 4 additions & 5 deletions crates/ruff_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mod rule_namespace;
mod violation;

#[proc_macro_derive(ConfigurationOptions, attributes(option, doc, option_group))]
pub fn derive_config(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_config(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);

config::derive_impl(input)
Expand Down Expand Up @@ -43,13 +43,12 @@ pub fn cache_key(input: TokenStream) -> TokenStream {
}

#[proc_macro]
pub fn register_rules(item: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn register_rules(item: TokenStream) -> TokenStream {
let mapping = parse_macro_input!(item as register_rules::Input);
register_rules::register_rules(&mapping).into()
}

/// Adds an `explanation()` method from the doc comment and
/// `#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]`
/// Adds an `explanation()` method from the doc comment.
#[proc_macro_attribute]
pub fn violation(_attr: TokenStream, item: TokenStream) -> TokenStream {
let violation = parse_macro_input!(item as ItemStruct);
Expand All @@ -59,7 +58,7 @@ pub fn violation(_attr: TokenStream, item: TokenStream) -> TokenStream {
}

#[proc_macro_derive(RuleNamespace, attributes(prefix))]
pub fn derive_rule_namespace(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_rule_namespace(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);

rule_namespace::derive_impl(input)
Expand Down
Loading