Skip to content

Commit

Permalink
Only validate __all__ bindings for global scope (#2738)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Feb 10, 2023
1 parent 0377834 commit d2b09d7
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 37 deletions.
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F822_2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
__all__ = ["foo"]

foo = 1


def bar():
pass
69 changes: 33 additions & 36 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4647,8 +4647,10 @@ impl<'a> Checker<'a> {
// Identify any valid runtime imports. If a module is imported at runtime, and
// used at runtime, then by default, we avoid flagging any other
// imports from that model as typing-only.
let runtime_imports: Vec<Vec<&Binding>> = if !self.settings.flake8_type_checking.strict
&& (self
let runtime_imports: Vec<Vec<&Binding>> = if self.settings.flake8_type_checking.strict {
vec![]
} else {
if self
.settings
.rules
.enabled(&Rule::RuntimeImportInTypeCheckingBlock)
Expand All @@ -4663,29 +4665,41 @@ impl<'a> Checker<'a> {
|| self
.settings
.rules
.enabled(&Rule::TypingOnlyStandardLibraryImport))
{
self.scopes
.iter()
.map(|scope| {
scope
.bindings
.values()
.map(|index| &self.bindings[*index])
.filter(|binding| {
flake8_type_checking::helpers::is_valid_runtime_import(binding)
})
.collect::<Vec<_>>()
})
.collect::<Vec<_>>()
} else {
vec![]
.enabled(&Rule::TypingOnlyStandardLibraryImport)
{
self.scopes
.iter()
.map(|scope| {
scope
.bindings
.values()
.map(|index| &self.bindings[*index])
.filter(|binding| {
flake8_type_checking::helpers::is_valid_runtime_import(binding)
})
.collect::<Vec<_>>()
})
.collect::<Vec<_>>()
} else {
vec![]
}
};

let mut diagnostics: Vec<Diagnostic> = vec![];
for (index, stack) in self.dead_scopes.iter().rev() {
let scope = &self.scopes[*index];

// F822
if *index == GLOBAL_SCOPE_INDEX {
if self.settings.rules.enabled(&Rule::UndefinedExport) {
if let Some((names, range)) = &all_names {
diagnostics.extend(pyflakes::rules::undefined_export(
names, range, self.path, scope,
));
}
}
}

// PLW0602
if self
.settings
Expand Down Expand Up @@ -4714,23 +4728,6 @@ impl<'a> Checker<'a> {
continue;
}

if self.settings.rules.enabled(&Rule::UndefinedExport) {
if !scope.import_starred && !self.path.ends_with("__init__.py") {
if let Some((names, range)) = &all_names {
for &name in names {
if !scope.bindings.contains_key(name) {
diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedExport {
name: name.to_string(),
},
*range,
));
}
}
}
}
}

// Look for any bindings that were redefined in another scope, and remain
// unused. Note that we only store references in `redefinitions` if
// the bindings are in different scopes.
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ mod tests {
#[test_case(Rule::UndefinedName, Path::new("F821_8.pyi"); "F821_8")]
#[test_case(Rule::UndefinedExport, Path::new("F822_0.py"); "F822_0")]
#[test_case(Rule::UndefinedExport, Path::new("F822_1.py"); "F822_1")]
#[test_case(Rule::UndefinedExport, Path::new("F822_2.py"); "F822_2")]
#[test_case(Rule::UndefinedLocal, Path::new("F823.py"); "F823")]
#[test_case(Rule::UnusedVariable, Path::new("F841_0.py"); "F841_0")]
#[test_case(Rule::UnusedVariable, Path::new("F841_1.py"); "F841_1")]
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/pyflakes/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub(crate) use strings::{
StringDotFormatExtraPositionalArguments, StringDotFormatInvalidFormat,
StringDotFormatMissingArguments, StringDotFormatMixingAutomatic,
};
pub use undefined_export::UndefinedExport;
pub use undefined_export::{undefined_export, UndefinedExport};
pub use undefined_local::{undefined_local, UndefinedLocal};
pub use undefined_name::UndefinedName;
pub use unused_annotation::{unused_annotation, UnusedAnnotation};
Expand Down
26 changes: 26 additions & 0 deletions crates/ruff/src/rules/pyflakes/rules/undefined_export.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::ast::types::{Range, Scope};
use crate::registry::Diagnostic;
use ruff_macros::{define_violation, derive_message_formats};
use std::path::Path;

use crate::violation::Violation;

Expand All @@ -14,3 +17,26 @@ impl Violation for UndefinedExport {
format!("Undefined name `{name}` in `__all__`")
}
}

/// F822
pub fn undefined_export(
names: &[&str],
range: &Range,
path: &Path,
scope: &Scope,
) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();
if !scope.import_starred && !path.ends_with("__init__.py") {
for name in names {
if !scope.bindings.contains_key(name) {
diagnostics.push(Diagnostic::new(
UndefinedExport {
name: (*name).to_string(),
},
*range,
));
}
}
}
diagnostics
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/ruff/src/rules/pyflakes/mod.rs
expression: diagnostics
---
[]

0 comments on commit d2b09d7

Please sign in to comment.