Skip to content

Commit

Permalink
Remove temporary comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jun 15, 2023
1 parent b19964b commit 9c36a04
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 97 deletions.
30 changes: 30 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F841_3.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,33 @@ def f():

def f():
toplevel = tt = 1


def f(provided: int) -> int:
match provided:
case [_, *x]:
pass


def f(provided: int) -> int:
match provided:
case x:
pass


def f(provided: int) -> int:
match provided:
case Foo(bar) as x:
pass


def f(provided: int) -> int:
match provided:
case {"foo": 0, **x}:
pass


def f(provided: int) -> int:
match provided:
case {**x}:
pass
23 changes: 4 additions & 19 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,6 @@ where
let name = alias.asname.as_ref().unwrap_or(&alias.name);
self.add_binding(
name,
// NOTE: Incorrect, needs to be the range of the alias.
alias.identifier(self.locator),
BindingKind::FutureImportation,
BindingFlags::empty(),
Expand All @@ -830,7 +829,6 @@ where
let qualified_name = &alias.name;
self.add_binding(
name,
// NOTE: Incorrect, needs to be the range of `name`.
alias.identifier(self.locator),
BindingKind::SubmoduleImportation(SubmoduleImportation {
qualified_name,
Expand All @@ -842,7 +840,6 @@ where
let qualified_name = &alias.name;
self.add_binding(
name,
// NOTE: Incorrect, needs to be the range of `name`.
alias.identifier(self.locator),
BindingKind::Importation(Importation { qualified_name }),
if alias
Expand Down Expand Up @@ -1088,7 +1085,6 @@ where

self.add_binding(
name,
// NOTE: Incorrect, needs to be the range of `name`.
alias.identifier(self.locator),
BindingKind::FutureImportation,
BindingFlags::empty(),
Expand Down Expand Up @@ -1150,7 +1146,6 @@ where
helpers::format_import_from_member(level, module, &alias.name);
self.add_binding(
name,
// NOTE: Incorrect, needs to be the range of `name`.
alias.identifier(self.locator),
BindingKind::FromImportation(FromImportation { qualified_name }),
if alias
Expand Down Expand Up @@ -1877,7 +1872,6 @@ where

self.add_binding(
name,
// NOTE: Correct!
stmt.identifier(self.locator),
BindingKind::FunctionDefinition,
BindingFlags::empty(),
Expand Down Expand Up @@ -2101,7 +2095,6 @@ where
self.semantic.pop_definition();
self.add_binding(
name,
// NOTE: Correct!
stmt.identifier(self.locator),
BindingKind::ClassDefinition,
BindingFlags::empty(),
Expand Down Expand Up @@ -3931,7 +3924,6 @@ where
// Add the bound exception name to the scope.
let binding_id = self.add_binding(
name,
// NOTE: Correct, we already extract this.
range,
BindingKind::Assignment,
BindingFlags::empty(),
Expand All @@ -3942,7 +3934,6 @@ where
// Remove it from the scope immediately after.
self.add_binding(
name,
// NOTE: Correct
range,
BindingKind::UnboundException,
BindingFlags::empty(),
Expand Down Expand Up @@ -4056,7 +4047,9 @@ where

fn visit_pattern(&mut self, pattern: &'b Pattern) {
if let Pattern::MatchAs(ast::PatternMatchAs {
name: Some(name), ..
// If there's a pattern, then there's an alias; skip pattern, skip `as`, take alias.
name: Some(name),
..
})
| Pattern::MatchStar(ast::PatternMatchStar {
name: Some(name),
Expand All @@ -4068,9 +4061,7 @@ where
{
self.add_binding(
name,
// TODO(charlie): This needs to use the range of the identifier, not the range of
// the pattern.
pattern.range(),
pattern.identifier(self.locator),
BindingKind::Assignment,
BindingFlags::empty(),
);
Expand Down Expand Up @@ -4447,7 +4438,6 @@ impl<'a> Checker<'a> {
) {
self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::Annotation,
BindingFlags::empty(),
Expand All @@ -4458,7 +4448,6 @@ impl<'a> Checker<'a> {
if matches!(parent, Stmt::For(_) | Stmt::AsyncFor(_)) {
self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::LoopVar,
BindingFlags::empty(),
Expand All @@ -4469,7 +4458,6 @@ impl<'a> Checker<'a> {
if helpers::is_unpacking_assignment(parent, expr) {
self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::UnpackedAssignment,
BindingFlags::empty(),
Expand Down Expand Up @@ -4523,7 +4511,6 @@ impl<'a> Checker<'a> {

self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::Export(Export { names }),
BindingFlags::empty(),
Expand All @@ -4538,7 +4525,6 @@ impl<'a> Checker<'a> {
{
self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::NamedExprAssignment,
BindingFlags::empty(),
Expand All @@ -4548,7 +4534,6 @@ impl<'a> Checker<'a> {

self.add_binding(
id,
// NOTE: Correct, range of the name.
expr.range(),
BindingKind::Assignment,
BindingFlags::empty(),
Expand Down
66 changes: 66 additions & 0 deletions crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,9 @@ F841_3.py:110:5: F841 [*] Local variable `toplevel` is assigned to but never use
109 109 | def f():
110 |- toplevel = tt = 1
110 |+ tt = 1
111 111 |
112 112 |
113 113 | def f(provided: int) -> int:

F841_3.py:110:16: F841 [*] Local variable `tt` is assigned to but never used
|
Expand All @@ -517,5 +520,68 @@ F841_3.py:110:16: F841 [*] Local variable `tt` is assigned to but never used
109 109 | def f():
110 |- toplevel = tt = 1
110 |+ toplevel = 1
111 111 |
112 112 |
113 113 | def f(provided: int) -> int:

F841_3.py:115:19: F841 Local variable `x` is assigned to but never used
|
113 | def f(provided: int) -> int:
114 | match provided:
115 | case [_, *x]:
| ^ F841
116 | pass
|
= help: Remove assignment to unused variable `x`

F841_3.py:121:14: F841 Local variable `x` is assigned to but never used
|
119 | def f(provided: int) -> int:
120 | match provided:
121 | case x:
| ^ F841
122 | pass
|
= help: Remove assignment to unused variable `x`

F841_3.py:127:18: F841 Local variable `bar` is assigned to but never used
|
125 | def f(provided: int) -> int:
126 | match provided:
127 | case Foo(bar) as x:
| ^^^ F841
128 | pass
|
= help: Remove assignment to unused variable `bar`

F841_3.py:127:26: F841 Local variable `x` is assigned to but never used
|
125 | def f(provided: int) -> int:
126 | match provided:
127 | case Foo(bar) as x:
| ^ F841
128 | pass
|
= help: Remove assignment to unused variable `x`

F841_3.py:133:27: F841 Local variable `x` is assigned to but never used
|
131 | def f(provided: int) -> int:
132 | match provided:
133 | case {"foo": 0, **x}:
| ^ F841
134 | pass
|
= help: Remove assignment to unused variable `x`

F841_3.py:139:17: F841 Local variable `x` is assigned to but never used
|
137 | def f(provided: int) -> int:
138 | match provided:
139 | case {**x}:
| ^ F841
140 | pass
|
= help: Remove assignment to unused variable `x`


4 changes: 2 additions & 2 deletions crates/ruff/src/rules/pylint/rules/useless_else_on_loop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustpython_parser::ast::{self, Excepthandler, MatchCase, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers;
use ruff_python_ast::identifier;

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -102,7 +102,7 @@ pub(crate) fn useless_else_on_loop(
if !orelse.is_empty() && !loop_exits_early(body) {
checker.diagnostics.push(Diagnostic::new(
UselessElseOnLoop,
helpers::else_range(stmt, checker.locator).unwrap(),
identifier::else_(stmt, checker.locator).unwrap(),
));
}
}
73 changes: 46 additions & 27 deletions crates/ruff_python_ast/src/helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1069,30 +1069,6 @@ pub fn match_parens(start: TextSize, locator: &Locator) -> Option<TextRange> {
}
}

/// Return the `Range` of `else` in `For`, `AsyncFor`, and `While` statements.
pub fn else_range(stmt: &Stmt, locator: &Locator) -> Option<TextRange> {
match stmt {
Stmt::For(ast::StmtFor { body, orelse, .. })
| Stmt::AsyncFor(ast::StmtAsyncFor { body, orelse, .. })
| Stmt::While(ast::StmtWhile { body, orelse, .. })
if !orelse.is_empty() =>
{
let body_end = body.last().expect("Expected body to be non-empty").end();
let or_else_start = orelse
.first()
.expect("Expected orelse to be non-empty")
.start();
let contents = &locator.contents()[TextRange::new(body_end, or_else_start)];

lexer::lex_starts_at(contents, Mode::Module, body_end)
.flatten()
.find(|(kind, _)| matches!(kind, Tok::Else))
.map(|(_, range)| range)
}
_ => None,
}
}

/// Return the `Range` of the first `Tok::Colon` token in a `Range`.
pub fn first_colon_range(range: TextRange, locator: &Locator) -> Option<TextRange> {
let contents = &locator.contents()[range];
Expand Down Expand Up @@ -1521,13 +1497,14 @@ mod tests {
use std::borrow::Cow;

use anyhow::Result;
use ruff_text_size::TextSize;
use rustpython_ast::{Cmpop, Expr};
use ruff_text_size::{TextLen, TextRange, TextSize};
use rustpython_ast::{Cmpop, Expr, Stmt};
use rustpython_parser::ast::Suite;
use rustpython_parser::Parse;

use crate::helpers::{
has_trailing_content, locate_cmpops, resolve_imported_module_path, LocatedCmpop,
elif_else_range, first_colon_range, has_trailing_content, locate_cmpops,
resolve_imported_module_path, LocatedCmpop,
};
use crate::source_code::Locator;

Expand Down Expand Up @@ -1607,6 +1584,48 @@ y = 2
);
}

#[test]
fn extract_first_colon_range() {
let contents = "with a: pass";
let locator = Locator::new(contents);
let range = first_colon_range(
TextRange::new(TextSize::from(0), contents.text_len()),
&locator,
)
.unwrap();
assert_eq!(&contents[range], ":");
assert_eq!(range, TextRange::new(TextSize::from(6), TextSize::from(7)));
}

#[test]
fn extract_elif_else_range() -> Result<()> {
let contents = "if a:
...
elif b:
...
";
let stmt = Stmt::parse(contents, "<filename>")?;
let stmt = Stmt::as_if_stmt(&stmt).unwrap();
let locator = Locator::new(contents);
let range = elif_else_range(stmt, &locator).unwrap();
assert_eq!(range.start(), TextSize::from(14));
assert_eq!(range.end(), TextSize::from(18));

let contents = "if a:
...
else:
...
";
let stmt = Stmt::parse(contents, "<filename>")?;
let stmt = Stmt::as_if_stmt(&stmt).unwrap();
let locator = Locator::new(contents);
let range = elif_else_range(stmt, &locator).unwrap();
assert_eq!(range.start(), TextSize::from(14));
assert_eq!(range.end(), TextSize::from(18));

Ok(())
}

#[test]
fn extract_cmpop_location() -> Result<()> {
let contents = "x == 1";
Expand Down
Loading

0 comments on commit 9c36a04

Please sign in to comment.