From 74f746168ca69d5d6c53fa5ec455733e0007e3ac Mon Sep 17 00:00:00 2001
From: InSyncWithFoo <insyncwithfoo@gmail.com>
Date: Mon, 13 Jan 2025 00:20:00 +0000
Subject: [PATCH 1/5] [`flake8-pytest-style`] Test function parameters with
 default arguments (`PT028`)

---
 .../fixtures/flake8_pytest_style/PT028.py     |  16 ++
 .../flake8_pytest_style/is_pytest_test.py     |  17 ++
 .../src/checkers/ast/analyze/statement.rs     |   3 +
 crates/ruff_linter/src/codes.rs               |   1 +
 .../src/rules/flake8_pytest_style/mod.rs      |  12 ++
 .../flake8_pytest_style/rules/helpers.rs      |  71 +++++++-
 .../rules/flake8_pytest_style/rules/mod.rs    |   2 +
 .../rules/test_functions.rs                   |  83 +++++++++
 ...es__flake8_pytest_style__tests__PT028.snap | 165 ++++++++++++++++++
 ...8_pytest_style__tests__is_pytest_test.snap |  79 +++++++++
 ruff.schema.json                              |   1 +
 11 files changed, 446 insertions(+), 4 deletions(-)
 create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
 create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/is_pytest_test.py
 create mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
 create mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
 create mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
new file mode 100644
index 0000000000000..531a1994c80d8
--- /dev/null
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
@@ -0,0 +1,16 @@
+# Errors
+
+def test_foo(a=1): ...
+def test_foo(a = 1): ...
+def test_foo(a = (1)): ...
+def test_foo(a: int=1): ...
+def test_foo(a: int = 1): ...
+def test_foo(a: (int) = 1): ...
+def test_foo(a: int = (1)): ...
+def test_foo(a: (int) = (1)): ...
+
+
+# No errors
+
+def test_foo(a): ...
+def test_foo(a: int): ...
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/is_pytest_test.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/is_pytest_test.py
new file mode 100644
index 0000000000000..9383451c9fe93
--- /dev/null
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/is_pytest_test.py
@@ -0,0 +1,17 @@
+# Errors
+
+def test_this_is_a_test(a=1): ...
+def testThisIsAlsoATest(a=1): ...
+
+class TestClass:
+    def test_this_too_is_a_test(self, a=1): ...
+    def testAndOfCourseThis(self, a=1): ...
+
+
+# No errors
+
+def this_is_not_a_test(a=1): ...
+
+class TestClassLookalike:
+    def __init__(self): ...
+    def test_this_is_not_a_test(self, a=1): ...
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
index 72e7e3ddda535..8bce8c1429d87 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -373,6 +373,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::PostInitDefault) {
                 ruff::rules::post_init_default(checker, function_def);
             }
+            if checker.enabled(Rule::PytestParameterWithDefaultArgument) {
+                flake8_pytest_style::rules::parameter_with_default_argument(checker, function_def);
+            }
         }
         Stmt::Return(_) => {
             if checker.enabled(Rule::ReturnOutsideFunction) {
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index 4d19a0e04c606..cc38f50b41c44 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -830,6 +830,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8PytestStyle, "025") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestErroneousUseFixturesOnFixture),
         (Flake8PytestStyle, "026") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestUseFixturesWithoutParameters),
         (Flake8PytestStyle, "027") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestUnittestRaisesAssertion),
+        (Flake8PytestStyle, "028") => (RuleGroup::Preview, rules::flake8_pytest_style::rules::PytestParameterWithDefaultArgument),
         (Flake8PytestStyle, "029") => (RuleGroup::Preview, rules::flake8_pytest_style::rules::PytestWarnsWithoutWarning),
         (Flake8PytestStyle, "030") => (RuleGroup::Preview, rules::flake8_pytest_style::rules::PytestWarnsTooBroad),
         (Flake8PytestStyle, "031") => (RuleGroup::Preview, rules::flake8_pytest_style::rules::PytestWarnsWithMultipleStatements),
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
index de8390f619148..7cb3486d0141d 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
@@ -18,6 +18,12 @@ mod tests {
     use super::settings::Settings;
     use super::types;
 
+    #[test_case(
+        Rule::PytestParameterWithDefaultArgument,
+        Path::new("is_pytest_test.py"),
+        Settings::default(),
+        "is_pytest_test"
+    )]
     #[test_case(
         Rule::PytestFixtureIncorrectParenthesesStyle,
         Path::new("PT001.py"),
@@ -275,6 +281,12 @@ mod tests {
         Settings::default(),
         "PT027_1"
     )]
+    #[test_case(
+        Rule::PytestParameterWithDefaultArgument,
+        Path::new("PT028.py"),
+        Settings::default(),
+        "PT028"
+    )]
     #[test_case(
         Rule::PytestWarnsWithoutWarning,
         Path::new("PT029.py"),
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
index d09474023eb10..0dcdbb3ffad28 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
@@ -1,10 +1,13 @@
-use std::fmt;
-
+use crate::checkers::ast::Checker;
 use ruff_python_ast::helpers::map_callable;
 use ruff_python_ast::name::UnqualifiedName;
-use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword};
-use ruff_python_semantic::SemanticModel;
+use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword, Stmt, StmtFunctionDef};
+use ruff_python_semantic::analyze::visibility;
+use ruff_python_semantic::{ScopeKind, SemanticModel};
 use ruff_python_trivia::PythonWhitespace;
+use std::ffi::OsStr;
+use std::fmt;
+use std::path::Path;
 
 pub(super) fn get_mark_decorators(
     decorators: &[Decorator],
@@ -45,6 +48,66 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) -
             matches!(qualified_name.segments(), ["pytest", "mark", "parametrize"])
         })
 }
+/// Whether `path` likely leads to a Pytest test file.
+///
+/// Pytest determines a test file using [a number of conventions and settings][1].
+/// Only the conventions are reflected here.
+/// This function returns true if the file name (`.py` notwithstanding):
+///
+/// * Starts with `test_`, or
+/// * Ends with `_test`
+///
+/// It is thus prone to both false positives and false negatives.
+///
+/// [1]: https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery
+#[allow(dead_code)]
+pub(crate) fn is_likely_pytest_test_file(path: &Path) -> bool {
+    if !matches!(path.extension().and_then(OsStr::to_str), Some("py")) {
+        return false;
+    }
+
+    let Some(file_stem) = path.file_stem().and_then(OsStr::to_str) else {
+        return false;
+    };
+
+    file_stem.starts_with("test_") || file_stem.ends_with("_test")
+}
+
+/// Whether the currently checked `func` is likely to be a Pytest test.
+///
+/// A normal Pytest test function is one whose name starts with `test` and is either:
+///
+/// * Placed at module-level, or
+/// * Placed within a class whose name starts with `Test` and does not have an `__init__` method.
+///
+/// See also [`is_likely_pytest_test_file`].
+pub(crate) fn is_likely_pytest_test(func: &StmtFunctionDef, checker: &Checker) -> bool {
+    let semantic = checker.semantic();
+
+    if !func.name.starts_with("test") {
+        return false;
+    }
+
+    if semantic.scope_id.is_global() {
+        return true;
+    }
+
+    let ScopeKind::Class(class) = semantic.current_scope().kind else {
+        return false;
+    };
+
+    if !class.name.starts_with("Test") {
+        return false;
+    }
+
+    class.body.iter().all(|stmt| {
+        let Stmt::FunctionDef(function) = stmt else {
+            return true;
+        };
+
+        !visibility::is_init(&function.name)
+    })
+}
 
 pub(super) fn keyword_is_literal(keyword: &Keyword, literal: &str) -> bool {
     if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &keyword.value {
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/mod.rs
index 81b63314d129f..128a1c5b4ff4b 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/mod.rs
@@ -6,6 +6,7 @@ pub(crate) use marks::*;
 pub(crate) use parametrize::*;
 pub(crate) use patch::*;
 pub(crate) use raises::*;
+pub(crate) use test_functions::*;
 pub(crate) use warns::*;
 
 mod assertion;
@@ -17,5 +18,6 @@ mod marks;
 mod parametrize;
 mod patch;
 mod raises;
+mod test_functions;
 mod unittest_assert;
 mod warns;
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
new file mode 100644
index 0000000000000..6dd889e98334b
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
@@ -0,0 +1,83 @@
+use crate::checkers::ast::Checker;
+use crate::rules::flake8_pytest_style::rules::helpers::is_likely_pytest_test;
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::{ParameterWithDefault, StmtFunctionDef};
+use ruff_text_size::Ranged;
+
+/// ## What it does
+/// Checks for parameters of test functions with default arguments.
+///
+/// ## Why is this bad?
+///
+///
+/// ## Example
+///
+/// ```python
+/// def test_foo(a=1):
+///     ...
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// def test_foo(a):
+///     ...
+/// ```
+///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe, as modifying a function signature can
+/// change the behavior of the code.
+///
+/// ## References
+/// - [Original Pytest issue](https://github.com/pytest-dev/pytest/issues/12693)
+#[derive(ViolationMetadata)]
+pub(crate) struct PytestParameterWithDefaultArgument {
+    parameter_name: String,
+}
+
+impl AlwaysFixableViolation for PytestParameterWithDefaultArgument {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!(
+            "Test function parameter `{}` has default argument",
+            self.parameter_name
+        )
+    }
+
+    fn fix_title(&self) -> String {
+        "Remove default argument".to_string()
+    }
+}
+
+/// PT028
+pub(crate) fn parameter_with_default_argument(
+    checker: &mut Checker,
+    function_def: &StmtFunctionDef,
+) {
+    if !is_likely_pytest_test(function_def, checker) {
+        return;
+    }
+
+    let parameters = function_def.parameters.as_ref();
+
+    for ParameterWithDefault {
+        parameter,
+        default,
+        range: pwd_range,
+    } in parameters.iter_non_variadic_params()
+    {
+        let Some(default) = default else {
+            continue;
+        };
+
+        let parameter_name = parameter.name.to_string();
+        let kind = PytestParameterWithDefaultArgument { parameter_name };
+        let diagnostic = Diagnostic::new(kind, default.range());
+
+        let edit = Edit::deletion(parameter.end(), pwd_range.end());
+        let fix = Fix::unsafe_edit(edit);
+
+        checker.diagnostics.push(diagnostic.with_fix(fix));
+    }
+}
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
new file mode 100644
index 0000000000000..0e15b71fc108f
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
@@ -0,0 +1,165 @@
+---
+source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
+---
+PT028.py:3:16: PT028 [*] Test function parameter `a` has default argument
+  |
+1 | # Errors
+2 | 
+3 | def test_foo(a=1): ...
+  |                ^ PT028
+4 | def test_foo(a = 1): ...
+5 | def test_foo(a = (1)): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+1 1 | # Errors
+2 2 | 
+3   |-def test_foo(a=1): ...
+  3 |+def test_foo(a): ...
+4 4 | def test_foo(a = 1): ...
+5 5 | def test_foo(a = (1)): ...
+6 6 | def test_foo(a: int=1): ...
+
+PT028.py:4:18: PT028 [*] Test function parameter `a` has default argument
+  |
+3 | def test_foo(a=1): ...
+4 | def test_foo(a = 1): ...
+  |                  ^ PT028
+5 | def test_foo(a = (1)): ...
+6 | def test_foo(a: int=1): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+1 1 | # Errors
+2 2 | 
+3 3 | def test_foo(a=1): ...
+4   |-def test_foo(a = 1): ...
+  4 |+def test_foo(a): ...
+5 5 | def test_foo(a = (1)): ...
+6 6 | def test_foo(a: int=1): ...
+7 7 | def test_foo(a: int = 1): ...
+
+PT028.py:5:19: PT028 [*] Test function parameter `a` has default argument
+  |
+3 | def test_foo(a=1): ...
+4 | def test_foo(a = 1): ...
+5 | def test_foo(a = (1)): ...
+  |                   ^ PT028
+6 | def test_foo(a: int=1): ...
+7 | def test_foo(a: int = 1): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+2 2 | 
+3 3 | def test_foo(a=1): ...
+4 4 | def test_foo(a = 1): ...
+5   |-def test_foo(a = (1)): ...
+  5 |+def test_foo(a): ...
+6 6 | def test_foo(a: int=1): ...
+7 7 | def test_foo(a: int = 1): ...
+8 8 | def test_foo(a: (int) = 1): ...
+
+PT028.py:6:21: PT028 [*] Test function parameter `a` has default argument
+  |
+4 | def test_foo(a = 1): ...
+5 | def test_foo(a = (1)): ...
+6 | def test_foo(a: int=1): ...
+  |                     ^ PT028
+7 | def test_foo(a: int = 1): ...
+8 | def test_foo(a: (int) = 1): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+3 3 | def test_foo(a=1): ...
+4 4 | def test_foo(a = 1): ...
+5 5 | def test_foo(a = (1)): ...
+6   |-def test_foo(a: int=1): ...
+  6 |+def test_foo(a: int): ...
+7 7 | def test_foo(a: int = 1): ...
+8 8 | def test_foo(a: (int) = 1): ...
+9 9 | def test_foo(a: int = (1)): ...
+
+PT028.py:7:23: PT028 [*] Test function parameter `a` has default argument
+  |
+5 | def test_foo(a = (1)): ...
+6 | def test_foo(a: int=1): ...
+7 | def test_foo(a: int = 1): ...
+  |                       ^ PT028
+8 | def test_foo(a: (int) = 1): ...
+9 | def test_foo(a: int = (1)): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+4 4 | def test_foo(a = 1): ...
+5 5 | def test_foo(a = (1)): ...
+6 6 | def test_foo(a: int=1): ...
+7   |-def test_foo(a: int = 1): ...
+  7 |+def test_foo(a: int): ...
+8 8 | def test_foo(a: (int) = 1): ...
+9 9 | def test_foo(a: int = (1)): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+
+PT028.py:8:25: PT028 [*] Test function parameter `a` has default argument
+   |
+ 6 | def test_foo(a: int=1): ...
+ 7 | def test_foo(a: int = 1): ...
+ 8 | def test_foo(a: (int) = 1): ...
+   |                         ^ PT028
+ 9 | def test_foo(a: int = (1)): ...
+10 | def test_foo(a: (int) = (1)): ...
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+5 5 | def test_foo(a = (1)): ...
+6 6 | def test_foo(a: int=1): ...
+7 7 | def test_foo(a: int = 1): ...
+8   |-def test_foo(a: (int) = 1): ...
+  8 |+def test_foo(a: (int)): ...
+9 9 | def test_foo(a: int = (1)): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+11 11 | 
+
+PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
+   |
+ 7 | def test_foo(a: int = 1): ...
+ 8 | def test_foo(a: (int) = 1): ...
+ 9 | def test_foo(a: int = (1)): ...
+   |                        ^ PT028
+10 | def test_foo(a: (int) = (1)): ...
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+6  6  | def test_foo(a: int=1): ...
+7  7  | def test_foo(a: int = 1): ...
+8  8  | def test_foo(a: (int) = 1): ...
+9     |-def test_foo(a: int = (1)): ...
+   9  |+def test_foo(a: int): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+11 11 | 
+12 12 | 
+
+PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
+   |
+ 8 | def test_foo(a: (int) = 1): ...
+ 9 | def test_foo(a: int = (1)): ...
+10 | def test_foo(a: (int) = (1)): ...
+   |                          ^ PT028
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+7  7  | def test_foo(a: int = 1): ...
+8  8  | def test_foo(a: (int) = 1): ...
+9  9  | def test_foo(a: int = (1)): ...
+10    |-def test_foo(a: (int) = (1)): ...
+   10 |+def test_foo(a: (int)): ...
+11 11 | 
+12 12 | 
+13 13 | # No errors
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap
new file mode 100644
index 0000000000000..4bb560f424cfc
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap
@@ -0,0 +1,79 @@
+---
+source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
+---
+is_pytest_test.py:3:27: PT028 [*] Test function parameter `a` has default argument
+  |
+1 | # Errors
+2 | 
+3 | def test_this_is_a_test(a=1): ...
+  |                           ^ PT028
+4 | def testThisIsAlsoATest(a=1): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+1 1 | # Errors
+2 2 | 
+3   |-def test_this_is_a_test(a=1): ...
+  3 |+def test_this_is_a_test(a): ...
+4 4 | def testThisIsAlsoATest(a=1): ...
+5 5 | 
+6 6 | class TestClass:
+
+is_pytest_test.py:4:27: PT028 [*] Test function parameter `a` has default argument
+  |
+3 | def test_this_is_a_test(a=1): ...
+4 | def testThisIsAlsoATest(a=1): ...
+  |                           ^ PT028
+5 | 
+6 | class TestClass:
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+1 1 | # Errors
+2 2 | 
+3 3 | def test_this_is_a_test(a=1): ...
+4   |-def testThisIsAlsoATest(a=1): ...
+  4 |+def testThisIsAlsoATest(a): ...
+5 5 | 
+6 6 | class TestClass:
+7 7 |     def test_this_too_is_a_test(self, a=1): ...
+
+is_pytest_test.py:7:41: PT028 [*] Test function parameter `a` has default argument
+  |
+6 | class TestClass:
+7 |     def test_this_too_is_a_test(self, a=1): ...
+  |                                         ^ PT028
+8 |     def testAndOfCourseThis(self, a=1): ...
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+4 4 | def testThisIsAlsoATest(a=1): ...
+5 5 | 
+6 6 | class TestClass:
+7   |-    def test_this_too_is_a_test(self, a=1): ...
+  7 |+    def test_this_too_is_a_test(self, a): ...
+8 8 |     def testAndOfCourseThis(self, a=1): ...
+9 9 | 
+10 10 | 
+
+is_pytest_test.py:8:37: PT028 [*] Test function parameter `a` has default argument
+  |
+6 | class TestClass:
+7 |     def test_this_too_is_a_test(self, a=1): ...
+8 |     def testAndOfCourseThis(self, a=1): ...
+  |                                     ^ PT028
+  |
+  = help: Remove default argument
+
+ℹ Unsafe fix
+5 5 | 
+6 6 | class TestClass:
+7 7 |     def test_this_too_is_a_test(self, a=1): ...
+8   |-    def testAndOfCourseThis(self, a=1): ...
+  8 |+    def testAndOfCourseThis(self, a): ...
+9 9 | 
+10 10 | 
+11 11 | # No errors
diff --git a/ruff.schema.json b/ruff.schema.json
index 321ac8f5e39c3..f302053296d2a 100644
--- a/ruff.schema.json
+++ b/ruff.schema.json
@@ -3715,6 +3715,7 @@
         "PT025",
         "PT026",
         "PT027",
+        "PT028",
         "PT029",
         "PT03",
         "PT030",

From c57a0b1d42207cbe63d603e9f8547cd294296cfd Mon Sep 17 00:00:00 2001
From: InSyncWithFoo <insyncwithfoo@gmail.com>
Date: Mon, 13 Jan 2025 02:35:30 +0000
Subject: [PATCH 2/5] Description

---
 .../rules/flake8_pytest_style/rules/test_functions.rs    | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
index 6dd889e98334b..8b77377489078 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
@@ -9,20 +9,19 @@ use ruff_text_size::Ranged;
 /// Checks for parameters of test functions with default arguments.
 ///
 /// ## Why is this bad?
-///
+/// Such a parameter will always have the default value during the test
+/// regardless of whether a fixture with the same name is defined.
 ///
 /// ## Example
 ///
 /// ```python
-/// def test_foo(a=1):
-///     ...
+/// def test_foo(a=1): ...
 /// ```
 ///
 /// Use instead:
 ///
 /// ```python
-/// def test_foo(a):
-///     ...
+/// def test_foo(a): ...
 /// ```
 ///
 /// ## Fix safety

From 20868c03742359318a649f42682e2f9e528acf04 Mon Sep 17 00:00:00 2001
From: InSyncWithFoo <insyncwithfoo@gmail.com>
Date: Mon, 13 Jan 2025 09:08:01 +0000
Subject: [PATCH 3/5] Per review

---
 .../fixtures/flake8_pytest_style/PT028.py     |  1 +
 .../flake8_pytest_style/rules/helpers.rs      |  8 +++
 ...es__flake8_pytest_style__tests__PT028.snap | 67 +++++++++++++++++--
 3 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
index 531a1994c80d8..1eb5360d4c231 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT028.py
@@ -8,6 +8,7 @@ def test_foo(a: int = 1): ...
 def test_foo(a: (int) = 1): ...
 def test_foo(a: int = (1)): ...
 def test_foo(a: (int) = (1)): ...
+def test_foo(a=1, /, b=2, *, c=3): ...
 
 
 # No errors
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
index 0dcdbb3ffad28..f51b6a17ed12b 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
@@ -60,6 +60,8 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) -
 /// It is thus prone to both false positives and false negatives.
 ///
 /// [1]: https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery
+///
+/// Reference: [Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
 #[allow(dead_code)]
 pub(crate) fn is_likely_pytest_test_file(path: &Path) -> bool {
     if !matches!(path.extension().and_then(OsStr::to_str), Some("py")) {
@@ -80,7 +82,13 @@ pub(crate) fn is_likely_pytest_test_file(path: &Path) -> bool {
 /// * Placed at module-level, or
 /// * Placed within a class whose name starts with `Test` and does not have an `__init__` method.
 ///
+/// During test discovery, Pytest respects a few settings which we do not have access to,
+/// like `testpaths` and `norecursedirs`.
+/// This function is thus prone to both false positives and false negatives.
+///
 /// See also [`is_likely_pytest_test_file`].
+///
+/// Reference: [Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
 pub(crate) fn is_likely_pytest_test(func: &StmtFunctionDef, checker: &Checker) -> bool {
     let semantic = checker.semantic();
 
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
index 0e15b71fc108f..f53ed790abbbb 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
@@ -123,7 +123,7 @@ PT028.py:8:25: PT028 [*] Test function parameter `a` has default argument
   8 |+def test_foo(a: (int)): ...
 9 9 | def test_foo(a: int = (1)): ...
 10 10 | def test_foo(a: (int) = (1)): ...
-11 11 | 
+11 11 | def test_foo(a=1, /, b=2, *, c=3): ...
 
 PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
    |
@@ -132,6 +132,7 @@ PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
  9 | def test_foo(a: int = (1)): ...
    |                        ^ PT028
 10 | def test_foo(a: (int) = (1)): ...
+11 | def test_foo(a=1, /, b=2, *, c=3): ...
    |
    = help: Remove default argument
 
@@ -142,7 +143,7 @@ PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
 9     |-def test_foo(a: int = (1)): ...
    9  |+def test_foo(a: int): ...
 10 10 | def test_foo(a: (int) = (1)): ...
-11 11 | 
+11 11 | def test_foo(a=1, /, b=2, *, c=3): ...
 12 12 | 
 
 PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
@@ -151,6 +152,7 @@ PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
  9 | def test_foo(a: int = (1)): ...
 10 | def test_foo(a: (int) = (1)): ...
    |                          ^ PT028
+11 | def test_foo(a=1, /, b=2, *, c=3): ...
    |
    = help: Remove default argument
 
@@ -160,6 +162,63 @@ PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
 9  9  | def test_foo(a: int = (1)): ...
 10    |-def test_foo(a: (int) = (1)): ...
    10 |+def test_foo(a: (int)): ...
-11 11 | 
+11 11 | def test_foo(a=1, /, b=2, *, c=3): ...
 12 12 | 
-13 13 | # No errors
+13 13 | 
+
+PT028.py:11:16: PT028 [*] Test function parameter `a` has default argument
+   |
+ 9 | def test_foo(a: int = (1)): ...
+10 | def test_foo(a: (int) = (1)): ...
+11 | def test_foo(a=1, /, b=2, *, c=3): ...
+   |                ^ PT028
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+8  8  | def test_foo(a: (int) = 1): ...
+9  9  | def test_foo(a: int = (1)): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+11    |-def test_foo(a=1, /, b=2, *, c=3): ...
+   11 |+def test_foo(a, /, b=2, *, c=3): ...
+12 12 | 
+13 13 | 
+14 14 | # No errors
+
+PT028.py:11:24: PT028 [*] Test function parameter `b` has default argument
+   |
+ 9 | def test_foo(a: int = (1)): ...
+10 | def test_foo(a: (int) = (1)): ...
+11 | def test_foo(a=1, /, b=2, *, c=3): ...
+   |                        ^ PT028
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+8  8  | def test_foo(a: (int) = 1): ...
+9  9  | def test_foo(a: int = (1)): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+11    |-def test_foo(a=1, /, b=2, *, c=3): ...
+   11 |+def test_foo(a=1, /, b, *, c=3): ...
+12 12 | 
+13 13 | 
+14 14 | # No errors
+
+PT028.py:11:32: PT028 [*] Test function parameter `c` has default argument
+   |
+ 9 | def test_foo(a: int = (1)): ...
+10 | def test_foo(a: (int) = (1)): ...
+11 | def test_foo(a=1, /, b=2, *, c=3): ...
+   |                                ^ PT028
+   |
+   = help: Remove default argument
+
+ℹ Unsafe fix
+8  8  | def test_foo(a: (int) = 1): ...
+9  9  | def test_foo(a: int = (1)): ...
+10 10 | def test_foo(a: (int) = (1)): ...
+11    |-def test_foo(a=1, /, b=2, *, c=3): ...
+   11 |+def test_foo(a=1, /, b=2, *, c): ...
+12 12 | 
+13 13 | 
+14 14 | # No errors

From bad60aaf876bf7126e83fad6a452ab8bc335617e Mon Sep 17 00:00:00 2001
From: InSyncWithFoo <insyncwithfoo@gmail.com>
Date: Mon, 13 Jan 2025 11:17:03 +0000
Subject: [PATCH 4/5] Per review

---
 .../rules/test_functions.rs                   | 10 ++---
 ...es__flake8_pytest_style__tests__PT028.snap | 44 +++++++++----------
 ...8_pytest_style__tests__is_pytest_test.snap | 16 +++----
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
index 8b77377489078..6276db0c0766f 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs
@@ -1,6 +1,6 @@
 use crate::checkers::ast::Checker;
 use crate::rules::flake8_pytest_style::rules::helpers::is_likely_pytest_test;
-use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_diagnostics::{Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast::{ParameterWithDefault, StmtFunctionDef};
 use ruff_text_size::Ranged;
@@ -35,7 +35,7 @@ pub(crate) struct PytestParameterWithDefaultArgument {
     parameter_name: String,
 }
 
-impl AlwaysFixableViolation for PytestParameterWithDefaultArgument {
+impl Violation for PytestParameterWithDefaultArgument {
     #[derive_message_formats]
     fn message(&self) -> String {
         format!(
@@ -44,8 +44,8 @@ impl AlwaysFixableViolation for PytestParameterWithDefaultArgument {
         )
     }
 
-    fn fix_title(&self) -> String {
-        "Remove default argument".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Remove default argument".to_string())
     }
 }
 
@@ -75,7 +75,7 @@ pub(crate) fn parameter_with_default_argument(
         let diagnostic = Diagnostic::new(kind, default.range());
 
         let edit = Edit::deletion(parameter.end(), pwd_range.end());
-        let fix = Fix::unsafe_edit(edit);
+        let fix = Fix::display_only_edit(edit);
 
         checker.diagnostics.push(diagnostic.with_fix(fix));
     }
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
index f53ed790abbbb..118bcac45c160 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT028.snap
@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
 ---
-PT028.py:3:16: PT028 [*] Test function parameter `a` has default argument
+PT028.py:3:16: PT028 Test function parameter `a` has default argument
   |
 1 | # Errors
 2 | 
@@ -12,7 +12,7 @@ PT028.py:3:16: PT028 [*] Test function parameter `a` has default argument
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 1 1 | # Errors
 2 2 | 
 3   |-def test_foo(a=1): ...
@@ -21,7 +21,7 @@ PT028.py:3:16: PT028 [*] Test function parameter `a` has default argument
 5 5 | def test_foo(a = (1)): ...
 6 6 | def test_foo(a: int=1): ...
 
-PT028.py:4:18: PT028 [*] Test function parameter `a` has default argument
+PT028.py:4:18: PT028 Test function parameter `a` has default argument
   |
 3 | def test_foo(a=1): ...
 4 | def test_foo(a = 1): ...
@@ -31,7 +31,7 @@ PT028.py:4:18: PT028 [*] Test function parameter `a` has default argument
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 1 1 | # Errors
 2 2 | 
 3 3 | def test_foo(a=1): ...
@@ -41,7 +41,7 @@ PT028.py:4:18: PT028 [*] Test function parameter `a` has default argument
 6 6 | def test_foo(a: int=1): ...
 7 7 | def test_foo(a: int = 1): ...
 
-PT028.py:5:19: PT028 [*] Test function parameter `a` has default argument
+PT028.py:5:19: PT028 Test function parameter `a` has default argument
   |
 3 | def test_foo(a=1): ...
 4 | def test_foo(a = 1): ...
@@ -52,7 +52,7 @@ PT028.py:5:19: PT028 [*] Test function parameter `a` has default argument
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 2 2 | 
 3 3 | def test_foo(a=1): ...
 4 4 | def test_foo(a = 1): ...
@@ -62,7 +62,7 @@ PT028.py:5:19: PT028 [*] Test function parameter `a` has default argument
 7 7 | def test_foo(a: int = 1): ...
 8 8 | def test_foo(a: (int) = 1): ...
 
-PT028.py:6:21: PT028 [*] Test function parameter `a` has default argument
+PT028.py:6:21: PT028 Test function parameter `a` has default argument
   |
 4 | def test_foo(a = 1): ...
 5 | def test_foo(a = (1)): ...
@@ -73,7 +73,7 @@ PT028.py:6:21: PT028 [*] Test function parameter `a` has default argument
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 3 3 | def test_foo(a=1): ...
 4 4 | def test_foo(a = 1): ...
 5 5 | def test_foo(a = (1)): ...
@@ -83,7 +83,7 @@ PT028.py:6:21: PT028 [*] Test function parameter `a` has default argument
 8 8 | def test_foo(a: (int) = 1): ...
 9 9 | def test_foo(a: int = (1)): ...
 
-PT028.py:7:23: PT028 [*] Test function parameter `a` has default argument
+PT028.py:7:23: PT028 Test function parameter `a` has default argument
   |
 5 | def test_foo(a = (1)): ...
 6 | def test_foo(a: int=1): ...
@@ -94,7 +94,7 @@ PT028.py:7:23: PT028 [*] Test function parameter `a` has default argument
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 4 4 | def test_foo(a = 1): ...
 5 5 | def test_foo(a = (1)): ...
 6 6 | def test_foo(a: int=1): ...
@@ -104,7 +104,7 @@ PT028.py:7:23: PT028 [*] Test function parameter `a` has default argument
 9 9 | def test_foo(a: int = (1)): ...
 10 10 | def test_foo(a: (int) = (1)): ...
 
-PT028.py:8:25: PT028 [*] Test function parameter `a` has default argument
+PT028.py:8:25: PT028 Test function parameter `a` has default argument
    |
  6 | def test_foo(a: int=1): ...
  7 | def test_foo(a: int = 1): ...
@@ -115,7 +115,7 @@ PT028.py:8:25: PT028 [*] Test function parameter `a` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 5 5 | def test_foo(a = (1)): ...
 6 6 | def test_foo(a: int=1): ...
 7 7 | def test_foo(a: int = 1): ...
@@ -125,7 +125,7 @@ PT028.py:8:25: PT028 [*] Test function parameter `a` has default argument
 10 10 | def test_foo(a: (int) = (1)): ...
 11 11 | def test_foo(a=1, /, b=2, *, c=3): ...
 
-PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
+PT028.py:9:24: PT028 Test function parameter `a` has default argument
    |
  7 | def test_foo(a: int = 1): ...
  8 | def test_foo(a: (int) = 1): ...
@@ -136,7 +136,7 @@ PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 6  6  | def test_foo(a: int=1): ...
 7  7  | def test_foo(a: int = 1): ...
 8  8  | def test_foo(a: (int) = 1): ...
@@ -146,7 +146,7 @@ PT028.py:9:24: PT028 [*] Test function parameter `a` has default argument
 11 11 | def test_foo(a=1, /, b=2, *, c=3): ...
 12 12 | 
 
-PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
+PT028.py:10:26: PT028 Test function parameter `a` has default argument
    |
  8 | def test_foo(a: (int) = 1): ...
  9 | def test_foo(a: int = (1)): ...
@@ -156,7 +156,7 @@ PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 7  7  | def test_foo(a: int = 1): ...
 8  8  | def test_foo(a: (int) = 1): ...
 9  9  | def test_foo(a: int = (1)): ...
@@ -166,7 +166,7 @@ PT028.py:10:26: PT028 [*] Test function parameter `a` has default argument
 12 12 | 
 13 13 | 
 
-PT028.py:11:16: PT028 [*] Test function parameter `a` has default argument
+PT028.py:11:16: PT028 Test function parameter `a` has default argument
    |
  9 | def test_foo(a: int = (1)): ...
 10 | def test_foo(a: (int) = (1)): ...
@@ -175,7 +175,7 @@ PT028.py:11:16: PT028 [*] Test function parameter `a` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 8  8  | def test_foo(a: (int) = 1): ...
 9  9  | def test_foo(a: int = (1)): ...
 10 10 | def test_foo(a: (int) = (1)): ...
@@ -185,7 +185,7 @@ PT028.py:11:16: PT028 [*] Test function parameter `a` has default argument
 13 13 | 
 14 14 | # No errors
 
-PT028.py:11:24: PT028 [*] Test function parameter `b` has default argument
+PT028.py:11:24: PT028 Test function parameter `b` has default argument
    |
  9 | def test_foo(a: int = (1)): ...
 10 | def test_foo(a: (int) = (1)): ...
@@ -194,7 +194,7 @@ PT028.py:11:24: PT028 [*] Test function parameter `b` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 8  8  | def test_foo(a: (int) = 1): ...
 9  9  | def test_foo(a: int = (1)): ...
 10 10 | def test_foo(a: (int) = (1)): ...
@@ -204,7 +204,7 @@ PT028.py:11:24: PT028 [*] Test function parameter `b` has default argument
 13 13 | 
 14 14 | # No errors
 
-PT028.py:11:32: PT028 [*] Test function parameter `c` has default argument
+PT028.py:11:32: PT028 Test function parameter `c` has default argument
    |
  9 | def test_foo(a: int = (1)): ...
 10 | def test_foo(a: (int) = (1)): ...
@@ -213,7 +213,7 @@ PT028.py:11:32: PT028 [*] Test function parameter `c` has default argument
    |
    = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 8  8  | def test_foo(a: (int) = 1): ...
 9  9  | def test_foo(a: int = (1)): ...
 10 10 | def test_foo(a: (int) = (1)): ...
diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap
index 4bb560f424cfc..df63491d7e3b0 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__is_pytest_test.snap
@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
 ---
-is_pytest_test.py:3:27: PT028 [*] Test function parameter `a` has default argument
+is_pytest_test.py:3:27: PT028 Test function parameter `a` has default argument
   |
 1 | # Errors
 2 | 
@@ -11,7 +11,7 @@ is_pytest_test.py:3:27: PT028 [*] Test function parameter `a` has default argume
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 1 1 | # Errors
 2 2 | 
 3   |-def test_this_is_a_test(a=1): ...
@@ -20,7 +20,7 @@ is_pytest_test.py:3:27: PT028 [*] Test function parameter `a` has default argume
 5 5 | 
 6 6 | class TestClass:
 
-is_pytest_test.py:4:27: PT028 [*] Test function parameter `a` has default argument
+is_pytest_test.py:4:27: PT028 Test function parameter `a` has default argument
   |
 3 | def test_this_is_a_test(a=1): ...
 4 | def testThisIsAlsoATest(a=1): ...
@@ -30,7 +30,7 @@ is_pytest_test.py:4:27: PT028 [*] Test function parameter `a` has default argume
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 1 1 | # Errors
 2 2 | 
 3 3 | def test_this_is_a_test(a=1): ...
@@ -40,7 +40,7 @@ is_pytest_test.py:4:27: PT028 [*] Test function parameter `a` has default argume
 6 6 | class TestClass:
 7 7 |     def test_this_too_is_a_test(self, a=1): ...
 
-is_pytest_test.py:7:41: PT028 [*] Test function parameter `a` has default argument
+is_pytest_test.py:7:41: PT028 Test function parameter `a` has default argument
   |
 6 | class TestClass:
 7 |     def test_this_too_is_a_test(self, a=1): ...
@@ -49,7 +49,7 @@ is_pytest_test.py:7:41: PT028 [*] Test function parameter `a` has default argume
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 4 4 | def testThisIsAlsoATest(a=1): ...
 5 5 | 
 6 6 | class TestClass:
@@ -59,7 +59,7 @@ is_pytest_test.py:7:41: PT028 [*] Test function parameter `a` has default argume
 9 9 | 
 10 10 | 
 
-is_pytest_test.py:8:37: PT028 [*] Test function parameter `a` has default argument
+is_pytest_test.py:8:37: PT028 Test function parameter `a` has default argument
   |
 6 | class TestClass:
 7 |     def test_this_too_is_a_test(self, a=1): ...
@@ -68,7 +68,7 @@ is_pytest_test.py:8:37: PT028 [*] Test function parameter `a` has default argume
   |
   = help: Remove default argument
 
-ℹ Unsafe fix
+ℹ Display-only fix
 5 5 | 
 6 6 | class TestClass:
 7 7 |     def test_this_too_is_a_test(self, a=1): ...

From 749824f0ff384007f5c4ade29305db80ff52a5e3 Mon Sep 17 00:00:00 2001
From: InSyncWithFoo <insyncwithfoo@gmail.com>
Date: Mon, 13 Jan 2025 11:46:01 +0000
Subject: [PATCH 5/5] Per review

---
 .../flake8_pytest_style/rules/helpers.rs      | 37 ++-----------------
 1 file changed, 4 insertions(+), 33 deletions(-)

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
index f51b6a17ed12b..7b886dd5cc653 100644
--- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
+++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs
@@ -5,9 +5,7 @@ use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword, Stmt, Stm
 use ruff_python_semantic::analyze::visibility;
 use ruff_python_semantic::{ScopeKind, SemanticModel};
 use ruff_python_trivia::PythonWhitespace;
-use std::ffi::OsStr;
 use std::fmt;
-use std::path::Path;
 
 pub(super) fn get_mark_decorators(
     decorators: &[Decorator],
@@ -48,32 +46,6 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) -
             matches!(qualified_name.segments(), ["pytest", "mark", "parametrize"])
         })
 }
-/// Whether `path` likely leads to a Pytest test file.
-///
-/// Pytest determines a test file using [a number of conventions and settings][1].
-/// Only the conventions are reflected here.
-/// This function returns true if the file name (`.py` notwithstanding):
-///
-/// * Starts with `test_`, or
-/// * Ends with `_test`
-///
-/// It is thus prone to both false positives and false negatives.
-///
-/// [1]: https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery
-///
-/// Reference: [Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
-#[allow(dead_code)]
-pub(crate) fn is_likely_pytest_test_file(path: &Path) -> bool {
-    if !matches!(path.extension().and_then(OsStr::to_str), Some("py")) {
-        return false;
-    }
-
-    let Some(file_stem) = path.file_stem().and_then(OsStr::to_str) else {
-        return false;
-    };
-
-    file_stem.starts_with("test_") || file_stem.ends_with("_test")
-}
 
 /// Whether the currently checked `func` is likely to be a Pytest test.
 ///
@@ -82,13 +54,12 @@ pub(crate) fn is_likely_pytest_test_file(path: &Path) -> bool {
 /// * Placed at module-level, or
 /// * Placed within a class whose name starts with `Test` and does not have an `__init__` method.
 ///
-/// During test discovery, Pytest respects a few settings which we do not have access to,
-/// like `testpaths` and `norecursedirs`.
+/// During test discovery, Pytest respects a few settings which we do not have access to.
 /// This function is thus prone to both false positives and false negatives.
 ///
-/// See also [`is_likely_pytest_test_file`].
-///
-/// Reference: [Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
+/// References:
+/// - [`pytest` documentation: Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
+/// - [`pytest` documentation: Changing naming conventions](https://docs.pytest.org/en/stable/example/pythoncollection.html#changing-naming-conventions)
 pub(crate) fn is_likely_pytest_test(func: &StmtFunctionDef, checker: &Checker) -> bool {
     let semantic = checker.semantic();