Skip to content

Commit

Permalink
Add documentation for remaining undocumented lint rules (#7750)
Browse files Browse the repository at this point in the history
  • Loading branch information
aspizu authored Oct 2, 2023
1 parent 4d2de89 commit 6a4437e
Show file tree
Hide file tree
Showing 14 changed files with 339 additions and 21 deletions.
2 changes: 0 additions & 2 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use std::fmt::Formatter;

use strum_macros::{AsRefStr, EnumIter};

use ruff_diagnostics::Violation;

use crate::registry::{AsRule, Linter};
use crate::rule_selector::is_single_rule_selector;
use crate::rules;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl Violation for MissingReturnTypeClassMethod {
/// ## References
/// - [PEP 484](https://www.python.org/dev/peps/pep-0484/#the-any-type)
/// - [Python documentation: `typing.Any`](https://docs.python.org/3/library/typing.html#typing.Any)
/// - [Mypy: The Any type](https://mypy.readthedocs.io/en/stable/kinds_of_types.html#the-any-type)
/// - [Mypy documentation: The Any type](https://mypy.readthedocs.io/en/stable/kinds_of_types.html#the-any-type)
#[violation]
pub struct AnyType {
name: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,32 @@ impl Violation for SubprocessWithoutShellEqualsTrue {
}
}

/// ## What it does
/// Checks for method calls that set the `shell` parameter to `true` when
/// invoking a subprocess.
///
/// ## Why is this bad?
/// Setting the `shell` parameter to `true` when invoking a subprocess can
/// introduce security vulnerabilities, as it allows shell metacharacters and
/// whitespace to be passed to child processes, potentially leading to shell
/// injection attacks. It is recommended to avoid using `shell=True` unless
/// absolutely necessary, and when used, to ensure that all inputs are properly
/// sanitized and quoted to prevent such vulnerabilities.
///
/// ## Known problems
/// Prone to false positives as it is triggered on any function call with a
/// `shell=True` parameter.
///
/// ## Example
/// ```python
/// import subprocess
///
/// user_input = input("Enter a command: ")
/// subprocess.run(user_input, shell=True)
/// ```
///
/// ## References
/// - [Python documentation: Security Considerations](https://docs.python.org/3/library/subprocess.html#security-considerations)
#[violation]
pub struct CallWithShellEqualsTrue;

Expand All @@ -98,6 +124,42 @@ impl Violation for CallWithShellEqualsTrue {
}
}

/// ## What it does
/// Checks for calls that start a process with a shell, providing guidance on
/// whether the usage is safe or not.
///
/// ## Why is this bad?
/// Starting a process with a shell can introduce security risks, such as
/// code injection vulnerabilities. It's important to be aware of whether the
/// usage of the shell is safe or not.
///
/// This rule triggers on functions like `os.system`, `popen`, etc., which
/// start processes with a shell. It evaluates whether the provided command
/// is a literal string or an expression. If the command is a literal string,
/// it's considered safe. If the command is an expression, it's considered
/// (potentially) unsafe.
///
/// ## Example
/// ```python
/// import os
///
/// # Safe usage (literal string)
/// command = "ls -l"
/// os.system(command)
///
/// # Potentially unsafe usage (expression)
/// cmd = get_user_input()
/// os.system(cmd)
/// ```
///
/// ## Note
/// The `subprocess` module provides more powerful facilities for spawning new
/// processes and retrieving their results, and using that module is preferable
/// to using `os.system` or similar functions. Consider replacing such usages
/// with `subprocess.call` or related functions.
///
/// ## References
/// - [Python documentation: `subprocess`](https://docs.python.org/3/library/subprocess.html)
#[violation]
pub struct StartProcessWithAShell {
seems_safe: bool,
Expand All @@ -114,6 +176,26 @@ impl Violation for StartProcessWithAShell {
}
}

/// ## What it does
/// Checks for functions that start a process without a shell.
///
/// ## Why is this bad?
/// The `subprocess` module provides more powerful facilities for spawning new
/// processes and retrieving their results; using that module is preferable to
/// using these functions.
///
/// ## Example
/// ```python
/// os.spawnlp(os.P_NOWAIT, "/bin/mycmd", "mycmd", "myarg")
/// ```
///
/// Use instead:
/// ```python
/// subprocess.Popen(["/bin/mycmd", "myarg"])
/// ```
///
/// ## References
/// - [Python documentation: Replacing the `os.spawn` family](https://docs.python.org/3/library/subprocess.html#replacing-the-os-spawn-family)
#[violation]
pub struct StartProcessWithNoShell;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::registry::AsRule;
/// ```
/// ## References
/// - [Python documentation: The `Any` type](https://docs.python.org/3/library/typing.html#the-any-type)
/// - [Mypy documentation](https://mypy.readthedocs.io/en/latest/dynamic_typing.html#any-vs-object)
/// - [Mypy documentation: Any vs. object](https://mypy.readthedocs.io/en/latest/dynamic_typing.html#any-vs-object)
#[violation]
pub struct AnyEqNeAnnotation {
method_name: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,25 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for the presence of docstrings in stub files.
///
/// ## Why is this bad?
/// Stub files should omit docstrings, as they're intended to provide type
/// hints, rather than documentation.
///
/// ## Example
/// ```python
/// def func(param: int) -> str:
/// """This is a docstring."""
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def func(param: int) -> str:
/// ...
/// ```
#[violation]
pub struct DocstringInStub;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for the presence of the `from __future__ import annotations` import
/// statement in stub files.
///
/// ## Why is this bad?
/// Stub files are already evaluated under `annotations` semantics. As such,
/// the `from __future__ import annotations` import statement has no effect
/// and should be omitted.
///
/// ## Resources
/// - [Static Typing with Python: Type Stubs](https://typing.readthedocs.io/en/latest/source/stubs.html)
#[violation]
pub struct FutureAnnotationsInStub;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ use ruff_macros::{derive_message_formats, violation};
use crate::checkers::ast::Checker;
use crate::registry::AsRule;

#[violation]
pub struct NumericLiteralTooLong;

/// ## What it does
/// Checks for numeric literals with a string representation longer than ten
/// characters.
Expand All @@ -23,13 +20,18 @@ pub struct NumericLiteralTooLong;
///
/// ## Example
/// ```python
/// def foo(arg: int = 12345678901) -> None: ...
/// def foo(arg: int = 12345678901) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def foo(arg: int = ...) -> None: ...
/// def foo(arg: int = ...) -> None:
/// ...
/// ```
#[violation]
pub struct NumericLiteralTooLong;

impl AlwaysFixableViolation for NumericLiteralTooLong {
#[derive_message_formats]
fn message(&self) -> String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,32 @@ use crate::checkers::ast::Checker;
use crate::fix;
use crate::registry::AsRule;

/// ## What it does
/// Checks for the presence of the `pass` statement within a class body
/// in a stub (`.pyi`) file.
///
/// ## Why is this bad?
/// In stub files, class definitions are intended to provide type hints, but
/// are never actually evaluated. As such, it's unnecessary to include a `pass`
/// statement in a class body, since it has no effect.
///
/// Instead of `pass`, prefer `...` to indicate that the class body is empty
/// and adhere to common stub file conventions.
///
/// ## Example
/// ```python
/// class MyClass:
/// pass
/// ```
///
/// Use instead:
/// ```python
/// class MyClass:
/// ...
/// ```
///
/// ## References
/// - [Mypy documentation: Stub files](https://mypy.readthedocs.io/en/stable/stubs.html)
#[violation]
pub struct PassInClassBody;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,26 @@ use ruff_macros::{derive_message_formats, violation};
use crate::checkers::ast::Checker;
use crate::registry::Rule;

/// ## What it does
/// Checks for quoted type annotations in stub (`.pyi`) files, which should be avoided.
///
/// ## Why is this bad?
/// Stub files are evaluated using `annotations` semantics, as if
/// `from __future__ import annotations` were included in the file. As such,
/// quotes are never required for type annotations in stub files, and should be
/// omitted.
///
/// ## Example
/// ```python
/// def function() -> "int":
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def function() -> int:
/// ...
/// ```
#[violation]
pub struct QuotedAnnotationInStub;

Expand Down
Loading

0 comments on commit 6a4437e

Please sign in to comment.