Skip to content

Commit 6bb6cb1

Browse files
charliermarshcharliermarsh
authored
Implement F822 (#94)
1 parent e9412c9 commit 6bb6cb1

File tree

8 files changed

+97
-32
lines changed

8 files changed

+97
-32
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ OPTIONS:
118118
| F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method |
119119
| F706 | ReturnOutsideFunction | a `return` statement outside of a function/method |
120120
| F821 | UndefinedName | Undefined name `...` |
121+
| F822 | UndefinedExport | Undefined name `...` in __all__ |
121122
| F823 | UndefinedLocal | Local variable `...` referenced before assignment |
122123
| F831 | DuplicateArgumentName | Duplicate argument name in function definition |
123124
| F841 | UnusedVariable | Local variable `...` is assigned to but never used |

‎examples/generate_rules_table.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ fn main() {
1212
CheckKind::ReturnOutsideFunction,
1313
CheckKind::UndefinedLocal("...".to_string()),
1414
CheckKind::UndefinedName("...".to_string()),
15+
CheckKind::UndefinedExport("...".to_string()),
1516
CheckKind::UnusedImport("...".to_string()),
1617
CheckKind::UnusedVariable("...".to_string()),
1718
CheckKind::UselessObjectInheritance("...".to_string()),

‎resources/test/fixtures/F822.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
a = 1
2+
3+
__all__ = ["a", "b"]

‎resources/test/fixtures/pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ select = [
1010
"F704",
1111
"F706",
1212
"F821",
13+
"F822",
1314
"F823",
1415
"F831",
1516
"F841",

‎src/check_ast.rs‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::collections::BTreeSet;
2+
use std::path::Path;
23

34
use rustpython_parser::ast::{
45
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt,
@@ -15,6 +16,7 @@ use crate::visitor::{walk_excepthandler, Visitor};
1516

1617
struct Checker<'a> {
1718
settings: &'a Settings,
19+
path: &'a str,
1820
checks: Vec<Check>,
1921
scopes: Vec<Scope>,
2022
dead_scopes: Vec<Scope>,
@@ -24,9 +26,10 @@ struct Checker<'a> {
2426
}
2527

2628
impl Checker<'_> {
27-
pub fn new(settings: &Settings) -> Checker {
29+
pub fn new<'a>(settings: &'a Settings, path: &'a str) -> Checker<'a> {
2830
Checker {
2931
settings,
32+
path,
3033
checks: vec![],
3134
scopes: vec![],
3235
dead_scopes: vec![],
@@ -673,19 +676,40 @@ impl Checker<'_> {
673676
}
674677

675678
fn check_dead_scopes(&mut self) {
676-
if self.settings.select.contains(&CheckCode::F401) {
677-
for scope in &self.dead_scopes {
678-
let all_binding = match scope.values.get("__all__") {
679-
Some(binding) => match &binding.kind {
680-
BindingKind::Export(names) => Some(names),
681-
_ => None,
682-
},
683-
_ => None,
684-
};
679+
if !self.settings.select.contains(&CheckCode::F822)
680+
&& !self.settings.select.contains(&CheckCode::F401)
681+
{
682+
return;
683+
}
684+
685+
for scope in &self.dead_scopes {
686+
let all_binding = scope.values.get("__all__");
687+
let all_names = all_binding.and_then(|binding| match &binding.kind {
688+
BindingKind::Export(names) => Some(names),
689+
_ => None,
690+
});
691+
692+
if self.settings.select.contains(&CheckCode::F822)
693+
&& !Path::new(self.path).ends_with("__init__.py")
694+
{
695+
if let Some(binding) = all_binding {
696+
if let Some(names) = all_names {
697+
for name in names {
698+
if !scope.values.contains_key(name) {
699+
self.checks.push(Check {
700+
kind: CheckKind::UndefinedExport(name.to_string()),
701+
location: binding.location,
702+
});
703+
}
704+
}
705+
}
706+
}
707+
}
685708

709+
if self.settings.select.contains(&CheckCode::F401) {
686710
for (name, binding) in scope.values.iter().rev() {
687711
let used = binding.used.is_some()
688-
|| all_binding
712+
|| all_names
689713
.map(|names| names.contains(name))
690714
.unwrap_or_default();
691715

@@ -708,7 +732,7 @@ impl Checker<'_> {
708732
}
709733

710734
pub fn check_ast(python_ast: &Suite, settings: &Settings, path: &str) -> Vec<Check> {
711-
let mut checker = Checker::new(settings);
735+
let mut checker = Checker::new(settings, path);
712736
checker.push_scope(Scope::new(ScopeKind::Module));
713737
checker.bind_builtins();
714738

‎src/checks.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub enum CheckCode {
1616
F704,
1717
F706,
1818
F821,
19+
F822,
1920
F823,
2021
F831,
2122
F841,
@@ -36,6 +37,7 @@ impl FromStr for CheckCode {
3637
"F704" => Ok(CheckCode::F704),
3738
"F706" => Ok(CheckCode::F706),
3839
"F821" => Ok(CheckCode::F821),
40+
"F822" => Ok(CheckCode::F822),
3941
"F823" => Ok(CheckCode::F823),
4042
"F831" => Ok(CheckCode::F831),
4143
"F841" => Ok(CheckCode::F841),
@@ -58,6 +60,7 @@ impl CheckCode {
5860
CheckCode::F706 => "F706",
5961
CheckCode::F821 => "F821",
6062
CheckCode::F823 => "F823",
63+
CheckCode::F822 => "F822",
6164
CheckCode::F831 => "F831",
6265
CheckCode::F841 => "F841",
6366
CheckCode::F901 => "F901",
@@ -76,6 +79,7 @@ impl CheckCode {
7679
CheckCode::F704 => &LintSource::AST,
7780
CheckCode::F706 => &LintSource::AST,
7881
CheckCode::F821 => &LintSource::AST,
82+
CheckCode::F822 => &LintSource::AST,
7983
CheckCode::F823 => &LintSource::AST,
8084
CheckCode::F831 => &LintSource::AST,
8185
CheckCode::F841 => &LintSource::AST,
@@ -101,6 +105,7 @@ pub enum CheckKind {
101105
RaiseNotImplemented,
102106
ReturnOutsideFunction,
103107
UndefinedLocal(String),
108+
UndefinedExport(String),
104109
UndefinedName(String),
105110
UnusedImport(String),
106111
UnusedVariable(String),
@@ -120,6 +125,7 @@ impl CheckKind {
120125
CheckKind::RaiseNotImplemented => "RaiseNotImplemented",
121126
CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction",
122127
CheckKind::UndefinedLocal(_) => "UndefinedLocal",
128+
CheckKind::UndefinedExport(_) => "UndefinedExport",
123129
CheckKind::UndefinedName(_) => "UndefinedName",
124130
CheckKind::UnusedImport(_) => "UnusedImport",
125131
CheckKind::UnusedVariable(_) => "UnusedVariable",
@@ -138,6 +144,7 @@ impl CheckKind {
138144
CheckKind::LineTooLong => &CheckCode::E501,
139145
CheckKind::RaiseNotImplemented => &CheckCode::F901,
140146
CheckKind::ReturnOutsideFunction => &CheckCode::F706,
147+
CheckKind::UndefinedExport(_) => &CheckCode::F822,
141148
CheckKind::UndefinedLocal(_) => &CheckCode::F823,
142149
CheckKind::UndefinedName(_) => &CheckCode::F821,
143150
CheckKind::UnusedImport(_) => &CheckCode::F401,
@@ -165,6 +172,9 @@ impl CheckKind {
165172
CheckKind::ReturnOutsideFunction => {
166173
"a `return` statement outside of a function/method".to_string()
167174
}
175+
CheckKind::UndefinedExport(name) => {
176+
format!("Undefined name `{name}` in __all__")
177+
}
168178
CheckKind::UndefinedName(name) => {
169179
format!("Undefined name `{name}`")
170180
}

‎src/linter.rs‎

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -331,33 +331,21 @@ mod tests {
331331
}
332332

333333
#[test]
334-
fn f831() -> Result<()> {
334+
fn f822() -> Result<()> {
335335
let actual = check_path(
336-
Path::new("./resources/test/fixtures/F831.py"),
336+
Path::new("./resources/test/fixtures/F822.py"),
337337
&settings::Settings {
338338
line_length: 88,
339339
exclude: vec![],
340-
select: BTreeSet::from([CheckCode::F831]),
340+
select: BTreeSet::from([CheckCode::F822]),
341341
},
342342
&cache::Mode::None,
343343
)?;
344-
let expected = vec![
345-
Message {
346-
kind: CheckKind::DuplicateArgumentName,
347-
location: Location::new(1, 25),
348-
filename: "./resources/test/fixtures/F831.py".to_string(),
349-
},
350-
Message {
351-
kind: CheckKind::DuplicateArgumentName,
352-
location: Location::new(5, 28),
353-
filename: "./resources/test/fixtures/F831.py".to_string(),
354-
},
355-
Message {
356-
kind: CheckKind::DuplicateArgumentName,
357-
location: Location::new(9, 27),
358-
filename: "./resources/test/fixtures/F831.py".to_string(),
359-
},
360-
];
344+
let expected = vec![Message {
345+
kind: CheckKind::UndefinedExport("b".to_string()),
346+
location: Location::new(3, 1),
347+
filename: "./resources/test/fixtures/F822.py".to_string(),
348+
}];
361349
assert_eq!(actual.len(), expected.len());
362350
for i in 0..actual.len() {
363351
assert_eq!(actual[i], expected[i]);
@@ -390,6 +378,42 @@ mod tests {
390378
Ok(())
391379
}
392380

381+
#[test]
382+
fn f831() -> Result<()> {
383+
let actual = check_path(
384+
Path::new("./resources/test/fixtures/F831.py"),
385+
&settings::Settings {
386+
line_length: 88,
387+
exclude: vec![],
388+
select: BTreeSet::from([CheckCode::F831]),
389+
},
390+
&cache::Mode::None,
391+
)?;
392+
let expected = vec![
393+
Message {
394+
kind: CheckKind::DuplicateArgumentName,
395+
location: Location::new(1, 25),
396+
filename: "./resources/test/fixtures/F831.py".to_string(),
397+
},
398+
Message {
399+
kind: CheckKind::DuplicateArgumentName,
400+
location: Location::new(5, 28),
401+
filename: "./resources/test/fixtures/F831.py".to_string(),
402+
},
403+
Message {
404+
kind: CheckKind::DuplicateArgumentName,
405+
location: Location::new(9, 27),
406+
filename: "./resources/test/fixtures/F831.py".to_string(),
407+
},
408+
];
409+
assert_eq!(actual.len(), expected.len());
410+
for i in 0..actual.len() {
411+
assert_eq!(actual[i], expected[i]);
412+
}
413+
414+
Ok(())
415+
}
416+
393417
#[test]
394418
fn f841() -> Result<()> {
395419
let actual = check_path(

‎src/pyproject.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ other-attribute = 1
245245
CheckCode::F704,
246246
CheckCode::F706,
247247
CheckCode::F821,
248+
CheckCode::F822,
248249
CheckCode::F823,
249250
CheckCode::F831,
250251
CheckCode::F841,

0 commit comments

Comments
 (0)