From 849091d846cc092bb2d835ab0e2f5db7d573fd98 Mon Sep 17 00:00:00 2001 From: Nazia Povey Date: Mon, 3 Apr 2023 11:38:41 -0400 Subject: [PATCH] When checking module visibility, don't check entire ancestry (#3835) --- .../pydocstyle/_unrelated/_no_pkg_priv.py | 0 .../pydocstyle/_unrelated/pkg/D100_pub.py | 0 .../pydocstyle/_unrelated/pkg/__init__.py | 0 .../_unrelated/pkg/_priv/__init__.py | 0 .../_unrelated/pkg/_priv/no_D100_priv.py | 0 crates/ruff/src/rules/pydocstyle/mod.rs | 3 ++ ...sts__D100__unrelated___no_pkg_priv.py.snap | 6 ++++ ...ts__D100__unrelated__pkg__D100_pub.py.snap | 19 +++++++++++ ...nrelated__pkg___priv__no_D100_priv.py.snap | 6 ++++ crates/ruff_python_ast/src/context.rs | 3 +- crates/ruff_python_ast/src/visibility.rs | 32 ++++++++----------- 11 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/_no_pkg_priv.py create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/D100_pub.py create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/__init__.py create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/__init__.py create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/no_D100_priv.py create mode 100644 crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated___no_pkg_priv.py.snap create mode 100644 crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg__D100_pub.py.snap create mode 100644 crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg___priv__no_D100_priv.py.snap diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/_no_pkg_priv.py b/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/_no_pkg_priv.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/D100_pub.py b/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/D100_pub.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/__init__.py b/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/__init__.py b/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/no_D100_priv.py b/crates/ruff/resources/test/fixtures/pydocstyle/_unrelated/pkg/_priv/no_D100_priv.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff/src/rules/pydocstyle/mod.rs b/crates/ruff/src/rules/pydocstyle/mod.rs index 8ea388fa0b..0aca01369e 100644 --- a/crates/ruff/src/rules/pydocstyle/mod.rs +++ b/crates/ruff/src/rules/pydocstyle/mod.rs @@ -60,6 +60,9 @@ mod tests { #[test_case(Rule::UndocumentedPublicMethod, Path::new("D.py"); "D102_0")] #[test_case(Rule::UndocumentedPublicMethod, Path::new("setter.py"); "D102_1")] #[test_case(Rule::UndocumentedPublicModule, Path::new("D.py"); "D100")] + #[test_case(Rule::UndocumentedPublicModule, Path::new("_unrelated/pkg/D100_pub.py"); "D100_ignore_unrelated_pub")] + #[test_case(Rule::UndocumentedPublicModule, Path::new("_unrelated/pkg/_priv/no_D100_priv.py"); "no_d100_priv")] + #[test_case(Rule::UndocumentedPublicModule, Path::new("_unrelated/_no_pkg_priv.py"); "no_d100_priv_script")] #[test_case(Rule::UndocumentedPublicNestedClass, Path::new("D.py"); "D106")] #[test_case(Rule::UndocumentedPublicPackage, Path::new("D.py"); "D104_0")] #[test_case(Rule::UndocumentedPublicPackage, Path::new("D104/__init__.py"); "D104_1")] diff --git a/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated___no_pkg_priv.py.snap b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated___no_pkg_priv.py.snap new file mode 100644 index 0000000000..fbf8d348f9 --- /dev/null +++ b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated___no_pkg_priv.py.snap @@ -0,0 +1,6 @@ +--- +source: crates/ruff/src/rules/pydocstyle/mod.rs +expression: diagnostics +--- +[] + diff --git a/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg__D100_pub.py.snap b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg__D100_pub.py.snap new file mode 100644 index 0000000000..e506aa4ef8 --- /dev/null +++ b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg__D100_pub.py.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/src/rules/pydocstyle/mod.rs +expression: diagnostics +--- +- kind: + name: UndocumentedPublicModule + body: Missing docstring in public module + suggestion: ~ + fixable: false + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 0 + fix: + edits: [] + parent: ~ + diff --git a/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg___priv__no_D100_priv.py.snap b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg___priv__no_D100_priv.py.snap new file mode 100644 index 0000000000..fbf8d348f9 --- /dev/null +++ b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D100__unrelated__pkg___priv__no_D100_priv.py.snap @@ -0,0 +1,6 @@ +--- +source: crates/ruff/src/rules/pydocstyle/mod.rs +expression: diagnostics +--- +[] + diff --git a/crates/ruff_python_ast/src/context.rs b/crates/ruff_python_ast/src/context.rs index bc7a4bb2cf..857311d44a 100644 --- a/crates/ruff_python_ast/src/context.rs +++ b/crates/ruff_python_ast/src/context.rs @@ -63,6 +63,7 @@ impl<'a> Context<'a> { path: &'a Path, module_path: Option>, ) -> Self { + let visibility = module_visibility(module_path.as_deref(), path); Self { typing_modules, module_path, @@ -79,7 +80,7 @@ impl<'a> Context<'a> { body_index: 0, visible_scope: VisibleScope { modifier: Modifier::Module, - visibility: module_visibility(path), + visibility, }, in_annotation: false, in_type_definition: false, diff --git a/crates/ruff_python_ast/src/visibility.rs b/crates/ruff_python_ast/src/visibility.rs index f65653872b..7945590d27 100644 --- a/crates/ruff_python_ast/src/visibility.rs +++ b/crates/ruff_python_ast/src/visibility.rs @@ -131,28 +131,24 @@ fn stem(path: &str) -> &str { } /// Return the `Visibility` of the Python file at `Path` based on its name. -pub fn module_visibility(path: &Path) -> Visibility { - let mut components = path.iter().rev(); - - // Is the module itself private? - // Ex) `_foo.py` (but not `__init__.py`) - if let Some(filename) = components.next() { - let module_name = filename.to_string_lossy(); - let module_name = stem(&module_name); - if is_private_module(module_name) { +pub fn module_visibility(module_path: Option<&[String]>, path: &Path) -> Visibility { + if let Some(module_path) = module_path { + if module_path.iter().any(|m| is_private_module(m)) { return Visibility::Private; } - } - - // Is the module in a private parent? - // Ex) `_foo/bar.py` - for component in components { - let module_name = component.to_string_lossy(); - if is_private_module(&module_name) { - return Visibility::Private; + } else { + // When module_path is None, path is a script outside a package, so just + // check to see if the module name itself is private. + // Ex) `_foo.py` (but not `__init__.py`) + let mut components = path.iter().rev(); + if let Some(filename) = components.next() { + let module_name = filename.to_string_lossy(); + let module_name = stem(&module_name); + if is_private_module(module_name) { + return Visibility::Private; + } } } - Visibility::Public }