Implement E722 (#166)

This commit is contained in:
Harutaka Kawamura 2022-09-12 22:04:39 +09:00 committed by GitHub
parent 825777edc1
commit 9414090617
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 100 additions and 44 deletions

View File

@ -9,6 +9,7 @@ fn main() {
CheckKind::AssertTuple,
CheckKind::DefaultExceptNotLast,
CheckKind::DoNotAssignLambda,
CheckKind::DoNotUseBareExcept,
CheckKind::DuplicateArgumentName,
CheckKind::FStringMissingPlaceholders,
CheckKind::FutureFeatureNotDefined("...".to_string()),

9
resources/test/fixtures/E722.py vendored Normal file
View File

@ -0,0 +1,9 @@
try:
pass
except:
pass
try:
pass
except Exception:
pass

View File

@ -8,6 +8,7 @@ select = [
"E712",
"E713",
"E714",
"E722",
"E731",
"E741",
"E742",

View File

@ -813,20 +813,45 @@ where
fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) {
match &excepthandler.node {
ExcepthandlerKind::ExceptHandler { name, .. } => match name {
Some(name) => {
if self.settings.select.contains(&CheckCode::E741) {
if let Some(check) =
checks::check_ambiguous_variable_name(name, excepthandler.location)
{
self.checks.push(check);
ExcepthandlerKind::ExceptHandler { type_, name, .. } => {
if self.settings.select.contains(&CheckCode::E722) && type_.is_none() {
self.checks.push(Check::new(
CheckKind::DoNotUseBareExcept,
excepthandler.location,
));
}
match name {
Some(name) => {
if self.settings.select.contains(&CheckCode::E741) {
if let Some(check) =
checks::check_ambiguous_variable_name(name, excepthandler.location)
{
self.checks.push(check);
}
}
}
let scope =
&self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
if scope.values.contains_key(name) {
let scope = &self.scopes
[*(self.scope_stack.last().expect("No current scope found."))];
if scope.values.contains_key(name) {
let parent = self.parents
[*(self.parent_stack.last().expect("No parent found."))];
self.handle_node_store(
&Expr::new(
excepthandler.location,
ExprKind::Name {
id: name.to_string(),
ctx: ExprContext::Store,
},
),
parent,
);
self.parents.push(parent);
}
let parent =
self.parents[*(self.parent_stack.last().expect("No parent found."))];
let scope = &self.scopes
[*(self.scope_stack.last().expect("No current scope found."))];
let definition = scope.values.get(name).cloned();
self.handle_node_store(
&Expr::new(
excepthandler.location,
@ -838,45 +863,29 @@ where
parent,
);
self.parents.push(parent);
}
let parent =
self.parents[*(self.parent_stack.last().expect("No parent found."))];
let scope =
&self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
let definition = scope.values.get(name).cloned();
self.handle_node_store(
&Expr::new(
excepthandler.location,
ExprKind::Name {
id: name.to_string(),
ctx: ExprContext::Store,
},
),
parent,
);
self.parents.push(parent);
walk_excepthandler(self, excepthandler);
walk_excepthandler(self, excepthandler);
let scope = &mut self.scopes
[*(self.scope_stack.last().expect("No current scope found."))];
if let Some(binding) = &scope.values.remove(name) {
if self.settings.select.contains(&CheckCode::F841)
&& binding.used.is_none()
{
self.checks.push(Check::new(
CheckKind::UnusedVariable(name.to_string()),
excepthandler.location,
));
}
}
let scope = &mut self.scopes
[*(self.scope_stack.last().expect("No current scope found."))];
if let Some(binding) = &scope.values.remove(name) {
if self.settings.select.contains(&CheckCode::F841) && binding.used.is_none()
{
self.checks.push(Check::new(
CheckKind::UnusedVariable(name.to_string()),
excepthandler.location,
));
if let Some(binding) = definition {
scope.values.insert(name.to_string(), binding);
}
}
if let Some(binding) = definition {
scope.values.insert(name.to_string(), binding);
}
None => walk_excepthandler(self, excepthandler),
}
None => walk_excepthandler(self, excepthandler),
},
}
}
}

View File

@ -14,6 +14,7 @@ pub enum CheckCode {
E712,
E713,
E714,
E722,
E731,
E741,
E742,
@ -53,6 +54,7 @@ impl FromStr for CheckCode {
"E711" => Ok(CheckCode::E711),
"E712" => Ok(CheckCode::E712),
"E713" => Ok(CheckCode::E713),
"E722" => Ok(CheckCode::E722),
"E714" => Ok(CheckCode::E714),
"E731" => Ok(CheckCode::E731),
"E741" => Ok(CheckCode::E741),
@ -95,6 +97,7 @@ impl CheckCode {
CheckCode::E712 => "E712",
CheckCode::E713 => "E713",
CheckCode::E714 => "E714",
CheckCode::E722 => "E722",
CheckCode::E731 => "E731",
CheckCode::E741 => "E741",
CheckCode::E742 => "E742",
@ -134,6 +137,7 @@ impl CheckCode {
CheckCode::E712 => &LintSource::AST,
CheckCode::E713 => &LintSource::AST,
CheckCode::E714 => &LintSource::AST,
CheckCode::E722 => &LintSource::AST,
CheckCode::E731 => &LintSource::AST,
CheckCode::E741 => &LintSource::AST,
CheckCode::E742 => &LintSource::AST,
@ -186,6 +190,7 @@ pub enum CheckKind {
AmbiguousFunctionName(String),
DefaultExceptNotLast,
DoNotAssignLambda,
DoNotUseBareExcept,
DuplicateArgumentName,
FStringMissingPlaceholders,
FutureFeatureNotDefined(String),
@ -224,6 +229,7 @@ impl CheckKind {
CheckKind::AmbiguousClassName(_) => "AmbiguousClassName",
CheckKind::AmbiguousFunctionName(_) => "AmbiguousFunctionName",
CheckKind::DefaultExceptNotLast => "DefaultExceptNotLast",
CheckKind::DoNotUseBareExcept => "DoNotUseBareExcept",
CheckKind::DuplicateArgumentName => "DuplicateArgumentName",
CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders",
CheckKind::FutureFeatureNotDefined(_) => "FutureFeatureNotDefined",
@ -262,6 +268,7 @@ impl CheckKind {
match self {
CheckKind::AssertTuple => &CheckCode::F631,
CheckKind::DefaultExceptNotLast => &CheckCode::F707,
CheckKind::DoNotUseBareExcept => &CheckCode::E722,
CheckKind::DuplicateArgumentName => &CheckCode::F831,
CheckKind::FStringMissingPlaceholders => &CheckCode::F541,
CheckKind::FutureFeatureNotDefined(_) => &CheckCode::F407,
@ -302,6 +309,7 @@ impl CheckKind {
CheckKind::AssertTuple => {
"Assert test is a non-empty tuple, which is always `True`".to_string()
}
CheckKind::DoNotUseBareExcept => "Do not use bare `except`".to_string(),
CheckKind::DefaultExceptNotLast => {
"an `except:` block as not the last exception handler".to_string()
}
@ -416,6 +424,7 @@ impl CheckKind {
CheckKind::AssertTuple => false,
CheckKind::DefaultExceptNotLast => false,
CheckKind::DoNotAssignLambda => false,
CheckKind::DoNotUseBareExcept => false,
CheckKind::DuplicateArgumentName => false,
CheckKind::FStringMissingPlaceholders => false,
CheckKind::FutureFeatureNotDefined(_) => false,

View File

@ -240,6 +240,31 @@ mod tests {
Ok(())
}
#[test]
fn e722() -> Result<()> {
let mut actual = check_path(
Path::new("./resources/test/fixtures/E722.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::E722]),
},
&fixer::Mode::Generate,
)?;
actual.sort_by_key(|check| check.location);
let expected = vec![Check {
kind: CheckKind::DoNotUseBareExcept,
location: Location::new(3, 1),
fix: None,
}];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test]
fn e714() -> Result<()> {
let mut actual = check_path(

View File

@ -265,6 +265,7 @@ other-attribute = 1
CheckCode::E712,
CheckCode::E713,
CheckCode::E714,
CheckCode::E722,
CheckCode::E731,
CheckCode::E741,
CheckCode::E742,

View File

@ -50,6 +50,7 @@ impl Settings {
CheckCode::E712,
CheckCode::E713,
CheckCode::E714,
CheckCode::E722,
CheckCode::E731,
CheckCode::E741,
CheckCode::E742,