Skip to content

Commit

Permalink
Add nursery
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 15, 2023
1 parent 68f8f43 commit 4a45cd4
Show file tree
Hide file tree
Showing 10 changed files with 904 additions and 704 deletions.
1,210 changes: 610 additions & 600 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)))
}
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

0 comments on commit 4a45cd4

Please sign in to comment.