Create per-rule pages and link from README (#2644)

This commit is contained in:
Charlie Marsh 2023-02-07 18:15:05 -05:00 committed by GitHub
parent f1cdd108e6
commit 271e4fda8c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 156 additions and 40 deletions

3
.gitignore vendored
View file

@ -1,7 +1,8 @@
# Local cache # Local cache
.ruff_cache .ruff_cache
crates/ruff/resources/test/cpython crates/ruff/resources/test/cpython
docs/ docs/*
!docs/rules
mkdocs.yml mkdocs.yml
.overrides .overrides

View file

@ -957,7 +957,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/) on PyPI
| B014 | duplicate-handler-exception | Exception handler with duplicate exception: `{name}` | 🛠 | | B014 | duplicate-handler-exception | Exception handler with duplicate exception: `{name}` | 🛠 |
| B015 | useless-comparison | Pointless comparison. This comparison does nothing but waste CPU instructions. Either prepend `assert` or remove it. | | | B015 | useless-comparison | Pointless comparison. This comparison does nothing but waste CPU instructions. Either prepend `assert` or remove it. | |
| B016 | cannot-raise-literal | Cannot raise a literal. Did you intend to return it or raise an Exception? | | | B016 | cannot-raise-literal | Cannot raise a literal. Did you intend to return it or raise an Exception? | |
| B017 | no-assert-raises-exception | `assertRaises(Exception)` should be considered evil | | | [B017](https://github.com/charliermarsh/ruff/blob/main/docs/rules/assert-raises-exception.md) | [assert-raises-exception](https://github.com/charliermarsh/ruff/blob/main/docs/rules/assert-raises-exception.md) | `assertRaises(Exception)` should be considered evil | |
| B018 | useless-expression | Found useless expression. Either assign it to a variable or remove it. | | | B018 | useless-expression | Found useless expression. Either assign it to a variable or remove it. | |
| B019 | cached-instance-method | Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks | | | B019 | cached-instance-method | Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks | |
| B020 | loop-variable-overrides-iterator | Loop control variable `{name}` overrides iterable it iterates | | | B020 | loop-variable-overrides-iterator | Loop control variable `{name}` overrides iterable it iterates | |

View file

@ -1563,7 +1563,7 @@ where
} }
} }
StmtKind::With { items, body, .. } => { StmtKind::With { items, body, .. } => {
if self.settings.rules.enabled(&Rule::NoAssertRaisesException) { if self.settings.rules.enabled(&Rule::AssertRaisesException) {
flake8_bugbear::rules::assert_raises_exception(self, stmt, items); flake8_bugbear::rules::assert_raises_exception(self, stmt, items);
} }
if self if self

View file

@ -146,7 +146,7 @@ ruff_macros::define_rule_mapping!(
B014 => rules::flake8_bugbear::rules::DuplicateHandlerException, B014 => rules::flake8_bugbear::rules::DuplicateHandlerException,
B015 => rules::flake8_bugbear::rules::UselessComparison, B015 => rules::flake8_bugbear::rules::UselessComparison,
B016 => rules::flake8_bugbear::rules::CannotRaiseLiteral, B016 => rules::flake8_bugbear::rules::CannotRaiseLiteral,
B017 => rules::flake8_bugbear::rules::NoAssertRaisesException, B017 => rules::flake8_bugbear::rules::AssertRaisesException,
B018 => rules::flake8_bugbear::rules::UselessExpression, B018 => rules::flake8_bugbear::rules::UselessExpression,
B019 => rules::flake8_bugbear::rules::CachedInstanceMethod, B019 => rules::flake8_bugbear::rules::CachedInstanceMethod,
B020 => rules::flake8_bugbear::rules::LoopVariableOverridesIterator, B020 => rules::flake8_bugbear::rules::LoopVariableOverridesIterator,

View file

@ -29,7 +29,7 @@ mod tests {
#[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"); "B014")] #[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"); "B014")]
#[test_case(Rule::UselessComparison, Path::new("B015.py"); "B015")] #[test_case(Rule::UselessComparison, Path::new("B015.py"); "B015")]
#[test_case(Rule::CannotRaiseLiteral, Path::new("B016.py"); "B016")] #[test_case(Rule::CannotRaiseLiteral, Path::new("B016.py"); "B016")]
#[test_case(Rule::NoAssertRaisesException, Path::new("B017.py"); "B017")] #[test_case(Rule::AssertRaisesException, Path::new("B017.py"); "B017")]
#[test_case(Rule::UselessExpression, Path::new("B018.py"); "B018")] #[test_case(Rule::UselessExpression, Path::new("B018.py"); "B018")]
#[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"); "B019")] #[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"); "B019")]
#[test_case(Rule::LoopVariableOverridesIterator, Path::new("B020.py"); "B020")] #[test_case(Rule::LoopVariableOverridesIterator, Path::new("B020.py"); "B020")]

View file

@ -10,15 +10,25 @@ define_violation!(
/// ### What it does /// ### What it does
/// Checks for `self.assertRaises(Exception)`. /// Checks for `self.assertRaises(Exception)`.
/// ///
/// ## Why is this bad? /// ### Why is this bad?
/// `assertRaises(Exception)` can lead to your test passing even if the /// `assertRaises(Exception)` can lead to your test passing even if the
/// code being tested is never executed due to a typo. /// code being tested is never executed due to a typo.
/// ///
/// Either assert for a more specific exception (builtin or custom), use /// Either assert for a more specific exception (builtin or custom), use
/// `assertRaisesRegex` or the context manager form of `assertRaises`. /// `assertRaisesRegex` or the context manager form of `assertRaises`.
pub struct NoAssertRaisesException; ///
/// ### Example
/// ```python
/// self.assertRaises(Exception, foo)
/// ```
///
/// Use instead:
/// ```python
/// self.assertRaises(SomeSpecificException, foo)
/// ```
pub struct AssertRaisesException;
); );
impl Violation for NoAssertRaisesException { impl Violation for AssertRaisesException {
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
format!("`assertRaises(Exception)` should be considered evil") format!("`assertRaises(Exception)` should be considered evil")
@ -51,7 +61,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With
} }
checker.diagnostics.push(Diagnostic::new( checker.diagnostics.push(Diagnostic::new(
NoAssertRaisesException, AssertRaisesException,
Range::from_located(stmt), Range::from_located(stmt),
)); ));
} }

View file

@ -3,7 +3,7 @@ pub use abstract_base_class::{
EmptyMethodWithoutAbstractDecorator, EmptyMethodWithoutAbstractDecorator,
}; };
pub use assert_false::{assert_false, DoNotAssertFalse}; pub use assert_false::{assert_false, DoNotAssertFalse};
pub use assert_raises_exception::{assert_raises_exception, NoAssertRaisesException}; pub use assert_raises_exception::{assert_raises_exception, AssertRaisesException};
pub use assignment_to_os_environ::{assignment_to_os_environ, AssignmentToOsEnviron}; pub use assignment_to_os_environ::{assignment_to_os_environ, AssignmentToOsEnviron};
pub use cached_instance_method::{cached_instance_method, CachedInstanceMethod}; pub use cached_instance_method::{cached_instance_method, CachedInstanceMethod};
pub use cannot_raise_literal::{cannot_raise_literal, CannotRaiseLiteral}; pub use cannot_raise_literal::{cannot_raise_literal, CannotRaiseLiteral};

View file

@ -1,9 +1,9 @@
--- ---
source: src/rules/flake8_bugbear/mod.rs source: crates/ruff/src/rules/flake8_bugbear/mod.rs
expression: diagnostics expression: diagnostics
--- ---
- kind: - kind:
NoAssertRaisesException: ~ AssertRaisesException: ~
location: location:
row: 22 row: 22
column: 8 column: 8

View file

@ -278,25 +278,31 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> {
let mut stdout = BufWriter::new(io::stdout().lock()); let mut stdout = BufWriter::new(io::stdout().lock());
match format { match format {
HelpFormat::Text => { HelpFormat::Text => {
writeln!(stdout, "{}\n", rule.as_ref())?; writeln!(
writeln!(stdout, "Code: {} ({})\n", rule.code(), linter.name())?; stdout,
"[{}] {} ({})",
linter.name(),
rule.as_ref(),
rule.code(),
)?;
writeln!(stdout)?;
if let Some(explanation) = rule.explanation() { if let Some(explanation) = rule.explanation() {
writeln!(stdout, "{}\n", explanation)?; writeln!(stdout, "{}", explanation.trim())?;
} else { } else {
writeln!(stdout, "Message formats:\n")?; writeln!(stdout, "Message formats:")?;
for format in rule.message_formats() { for format in rule.message_formats() {
writeln!(stdout, "* {format}")?; writeln!(stdout, "* {format}")?;
} }
} }
if let Some(autofix) = rule.autofixable() { if let Some(autofix) = rule.autofixable() {
writeln!(stdout)?;
writeln!( writeln!(
stdout, stdout,
"{}", "{}",
match autofix.available { match autofix.available {
AutofixAvailability::Sometimes => "Autofix is sometimes available.\n", AutofixAvailability::Sometimes => "Autofix is sometimes available.",
AutofixAvailability::Always => "Autofix is always available.\n", AutofixAvailability::Always => "Autofix is always available.",
} }
)?; )?;
} }

View file

@ -2,7 +2,9 @@
use anyhow::Result; use anyhow::Result;
use crate::{generate_cli_help, generate_json_schema, generate_options, generate_rules_table}; use crate::{
generate_cli_help, generate_docs, generate_json_schema, generate_options, generate_rules_table,
};
#[derive(clap::Args)] #[derive(clap::Args)]
pub struct Args { pub struct Args {
@ -12,6 +14,9 @@ pub struct Args {
} }
pub fn main(args: &Args) -> Result<()> { pub fn main(args: &Args) -> Result<()> {
generate_docs::main(&generate_docs::Args {
dry_run: args.dry_run,
})?;
generate_json_schema::main(&generate_json_schema::Args { generate_json_schema::main(&generate_json_schema::Args {
dry_run: args.dry_run, dry_run: args.dry_run,
})?; })?;

View file

@ -1 +1,32 @@
//! Generate Markdown documentation for applicable rules.
#![allow(clippy::print_stdout, clippy::print_stderr)]
use std::fs;
use anyhow::Result;
use strum::IntoEnumIterator;
use ruff::registry::Rule;
#[derive(clap::Args)]
pub struct Args {
/// Write the generated docs to stdout (rather than to the filesystem).
#[arg(long)]
pub(crate) dry_run: bool,
}
pub fn main(args: &Args) -> Result<()> {
for rule in Rule::iter() {
if let Some(explanation) = rule.explanation() {
let explanation = format!("# {} ({})\n\n{}", rule.as_ref(), rule.code(), explanation);
if args.dry_run {
println!("{}", explanation);
} else {
fs::create_dir_all("docs/rules")?;
fs::write(format!("docs/rules/{}.md", rule.as_ref()), explanation)?;
}
}
}
Ok(())
}

View file

@ -14,6 +14,8 @@ const TABLE_END_PRAGMA: &str = "<!-- End auto-generated sections. -->";
const TOC_BEGIN_PRAGMA: &str = "<!-- Begin auto-generated table of contents. -->"; const TOC_BEGIN_PRAGMA: &str = "<!-- Begin auto-generated table of contents. -->";
const TOC_END_PRAGMA: &str = "<!-- End auto-generated table of contents. -->"; const TOC_END_PRAGMA: &str = "<!-- End auto-generated table of contents. -->";
const URL_PREFIX: &str = "https://github.com/charliermarsh/ruff/blob/main/docs/rules";
#[derive(clap::Args)] #[derive(clap::Args)]
pub struct Args { pub struct Args {
/// Write the generated table to stdout (rather than to `README.md`). /// Write the generated table to stdout (rather than to `README.md`).
@ -32,6 +34,19 @@ fn generate_table(table_out: &mut String, rules: impl IntoIterator<Item = Rule>)
Some(_) => "🛠", Some(_) => "🛠",
}; };
if rule.explanation().is_some() {
table_out.push_str(&format!(
"| [{}]({}/{}.md) | [{}]({}/{}.md) | {} | {} |",
rule.code(),
URL_PREFIX,
rule.as_ref(),
rule.as_ref(),
URL_PREFIX,
rule.as_ref(),
rule.message_formats()[0].replace('|', r"\|"),
fix_token
));
} else {
table_out.push_str(&format!( table_out.push_str(&format!(
"| {} | {} | {} | {} |", "| {} | {} | {} | {} |",
rule.code(), rule.code(),
@ -39,6 +54,7 @@ fn generate_table(table_out: &mut String, rules: impl IntoIterator<Item = Rule>)
rule.message_formats()[0].replace('|', r"\|"), rule.message_formats()[0].replace('|', r"\|"),
fix_token fix_token
)); ));
}
table_out.push('\n'); table_out.push('\n');
} }
table_out.push('\n'); table_out.push('\n');

View file

@ -39,6 +39,8 @@ enum Command {
GenerateOptions(generate_options::Args), GenerateOptions(generate_options::Args),
/// Generate CLI help. /// Generate CLI help.
GenerateCliHelp(generate_cli_help::Args), GenerateCliHelp(generate_cli_help::Args),
/// Generate Markdown docs.
GenerateDocs(generate_docs::Args),
/// Print the AST for a given Python file. /// Print the AST for a given Python file.
PrintAST(print_ast::Args), PrintAST(print_ast::Args),
/// Print the LibCST CST for a given Python file. /// Print the LibCST CST for a given Python file.
@ -57,6 +59,7 @@ fn main() -> Result<()> {
Command::GenerateRulesTable(args) => generate_rules_table::main(args)?, Command::GenerateRulesTable(args) => generate_rules_table::main(args)?,
Command::GenerateOptions(args) => generate_options::main(args)?, Command::GenerateOptions(args) => generate_options::main(args)?,
Command::GenerateCliHelp(args) => generate_cli_help::main(args)?, Command::GenerateCliHelp(args) => generate_cli_help::main(args)?,
Command::GenerateDocs(args) => generate_docs::main(args)?,
Command::PrintAST(args) => print_ast::main(args)?, Command::PrintAST(args) => print_ast::main(args)?,
Command::PrintCST(args) => print_cst::main(args)?, Command::PrintCST(args) => print_cst::main(args)?,
Command::PrintTokens(args) => print_tokens::main(args)?, Command::PrintTokens(args) => print_tokens::main(args)?,

View file

@ -37,7 +37,8 @@ impl Parse for LintMeta {
let value = lit.value(); let value = lit.value();
let line = value.strip_prefix(' ').unwrap_or(&value); let line = value.strip_prefix(' ').unwrap_or(&value);
if line.starts_with("```") { if line.starts_with("```") {
explanation += "```\n"; explanation += line;
explanation.push('\n');
in_code = !in_code; in_code = !in_code;
} else if !(in_code && line.starts_with("# ")) { } else if !(in_code && line.starts_with("# ")) {
explanation += line; explanation += line;
@ -61,7 +62,13 @@ impl Parse for LintMeta {
pub fn define_violation(input: &TokenStream, meta: LintMeta) -> TokenStream { pub fn define_violation(input: &TokenStream, meta: LintMeta) -> TokenStream {
let LintMeta { explanation, name } = meta; let LintMeta { explanation, name } = meta;
let output = quote! { if explanation.is_empty() {
quote! {
#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#input
}
} else {
quote! {
#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#input #input
@ -70,6 +77,6 @@ pub fn define_violation(input: &TokenStream, meta: LintMeta) -> TokenStream {
Some(#explanation) Some(#explanation)
} }
} }
}; }
output }
} }

View file

@ -0,0 +1,21 @@
# assert-raises-exception (B017)
### What it does
Checks for the use of `assertRaises(Exception)`.
### Why is this bad?
`assertRaises(Exception)` can lead to your test passing even if the
code being tested is never executed (e.g., due to a typo).
Assert for a more specific exception (builtin or custom), use
`assertRaisesRegex` or the context manager form of `assertRaises`.
### Example
```python
self.assertRaises(Exception, foo)
```
Use instead:
```python
self.assertRaises(SomeSpecificException, foo)
```

View file

@ -21,10 +21,15 @@ def snake_case(name: str) -> str:
).lstrip("_") ).lstrip("_")
def main(*, name: str, code: str, linter: str) -> None: # noqa: PLR0915 def main(*, name: str, code: str, linter: str) -> None:
"""Generate boilerplate for a new rule.""" """Generate boilerplate for a new rule."""
# Create a test fixture. # Create a test fixture.
with (ROOT_DIR / "crates/ruff/resources/test/fixtures" / dir_name(linter) / f"{code}.py").open( with (
ROOT_DIR
/ "crates/ruff/resources/test/fixtures"
/ dir_name(linter)
/ f"{code}.py"
).open(
"a", "a",
): ):
pass pass

View file

@ -36,6 +36,12 @@ def main() -> None:
raise ValueError(msg) raise ValueError(msg)
content = content.replace(DOCUMENTATION_LINK, "") content = content.replace(DOCUMENTATION_LINK, "")
# Replace all GitHub links with relative links.
content = content.replace(
"https://github.com/charliermarsh/ruff/blob/main/docs/rules/",
"rules/",
)
Path("docs").mkdir(parents=True, exist_ok=True) Path("docs").mkdir(parents=True, exist_ok=True)
# Split the README.md into sections. # Split the README.md into sections.
@ -71,8 +77,13 @@ def main() -> None:
] ]
config["extra"] = {"analytics": {"provider": "fathom"}} config["extra"] = {"analytics": {"provider": "fathom"}}
Path(".overrides/partials/integrations/analytics").mkdir(parents=True, exist_ok=True) Path(".overrides/partials/integrations/analytics").mkdir(
with Path(".overrides/partials/integrations/analytics/fathom.html").open("w+") as fp: parents=True,
exist_ok=True,
)
with Path(".overrides/partials/integrations/analytics/fathom.html").open(
"w+",
) as fp:
fp.write(FATHOM_SCRIPT) fp.write(FATHOM_SCRIPT)
with Path("mkdocs.yml").open("w+") as fp: with Path("mkdocs.yml").open("w+") as fp: