Skip to content

Commit

Permalink
Disallow dotted name in from ... import statement (#10903)
Browse files Browse the repository at this point in the history 
## Summary

Dotted names aren't allowed in `from ... import` statement. They're only
allowed in `import` statement.

### Alternative

Another solution would be to parse the dotted name, check if there's a
`.` in the parsed string and raise an error.

I choose not to do this because it didn't make sense to do `contains`
for every import name.

## Test Plan

Add invalid syntax test cases to verify the logic.
  • Loading branch information
@dhruvmanila
dhruvmanila authored Apr 12, 2024
1 parent 27bb09c commit cefa753
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 7 deletions.
3 changes: 3 additions & 0 deletions crates/ruff_python_parser/resources/inline/err/from_import_dotted_names.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from x import a.
from x import a.b
from x import a, b.c, d, e.f, g
29 changes: 22 additions & 7 deletions crates/ruff_python_parser/src/parser/statement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,10 @@ impl<'src> Parser<'src> {
// import ,
// import x, y,

let names = self.parse_comma_separated_list_into_vec(
RecoveryContextKind::ImportNames,
Parser::parse_alias,
);
let names = self
.parse_comma_separated_list_into_vec(RecoveryContextKind::ImportNames, |p| {
p.parse_alias(ImportStyle::Import)
});

if names.is_empty() {
// test_err import_stmt_empty
Expand Down Expand Up @@ -528,7 +528,11 @@ impl<'src> Parser<'src> {
self.parse_comma_separated_list(
RecoveryContextKind::ImportFromAsNames(parenthesized),
|parser| {
let alias = parser.parse_alias();
// test_err from_import_dotted_names
// from x import a.
// from x import a.b
// from x import a, b.c, d, e.f, g
let alias = parser.parse_alias(ImportStyle::ImportFrom);
seen_star_import |= alias.name.id == "*";
names.push(alias);
},
Expand Down Expand Up @@ -576,7 +580,7 @@ impl<'src> Parser<'src> {
/// See:
/// - <https://docs.python.org/3/reference/simple_stmts.html#the-import-statement>
/// - <https://docs.python.org/3/library/ast.html#ast.alias>
fn parse_alias(&mut self) -> ast::Alias {
fn parse_alias(&mut self, style: ImportStyle) -> ast::Alias {
let start = self.node_start();
if self.eat(TokenKind::Star) {
let range = self.node_range(start);
Expand All @@ -590,7 +594,10 @@ impl<'src> Parser<'src> {
};
}

let name = self.parse_dotted_name();
let name = match style {
ImportStyle::Import => self.parse_dotted_name(),
ImportStyle::ImportFrom => self.parse_identifier(),
};

let asname = if self.eat(TokenKind::As) {
if self.at(TokenKind::Name) {
Expand Down Expand Up @@ -3433,3 +3440,11 @@ enum AllowStarAnnotation {
Yes,
No,
}

#[derive(Debug, Copy, Clone)]
enum ImportStyle {
/// E.g., `import foo, bar`
Import,
/// E.g., `from foo import bar, baz`
ImportFrom,
}
175 changes: 175 additions & 0 deletions crates/ruff_python_parser/tests/snapshots/invalid_syntax@from_import_dotted_names.py.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/from_import_dotted_names.py
---
## AST

```
Module(
ModModule {
range: 0..67,
body: [
ImportFrom(
StmtImportFrom {
range: 0..16,
module: Some(
Identifier {
id: "x",
range: 5..6,
},
),
names: [
Alias {
range: 14..15,
name: Identifier {
id: "a",
range: 14..15,
},
asname: None,
},
],
level: Some(
0,
),
},
),
ImportFrom(
StmtImportFrom {
range: 17..34,
module: Some(
Identifier {
id: "x",
range: 22..23,
},
),
names: [
Alias {
range: 31..32,
name: Identifier {
id: "a",
range: 31..32,
},
asname: None,
},
Alias {
range: 33..34,
name: Identifier {
id: "b",
range: 33..34,
},
asname: None,
},
],
level: Some(
0,
),
},
),
ImportFrom(
StmtImportFrom {
range: 35..66,
module: Some(
Identifier {
id: "x",
range: 40..41,
},
),
names: [
Alias {
range: 49..50,
name: Identifier {
id: "a",
range: 49..50,
},
asname: None,
},
Alias {
range: 52..53,
name: Identifier {
id: "b",
range: 52..53,
},
asname: None,
},
Alias {
range: 54..55,
name: Identifier {
id: "c",
range: 54..55,
},
asname: None,
},
Alias {
range: 57..58,
name: Identifier {
id: "d",
range: 57..58,
},
asname: None,
},
Alias {
range: 60..61,
name: Identifier {
id: "e",
range: 60..61,
},
asname: None,
},
Alias {
range: 62..63,
name: Identifier {
id: "f",
range: 62..63,
},
asname: None,
},
Alias {
range: 65..66,
name: Identifier {
id: "g",
range: 65..66,
},
asname: None,
},
],
level: Some(
0,
),
},
),
],
},
)
```
## Errors

|
1 | from x import a.
| ^ Syntax Error: Expected ',', found '.'
2 | from x import a.b
3 | from x import a, b.c, d, e.f, g
|


|
1 | from x import a.
2 | from x import a.b
| ^ Syntax Error: Expected ',', found '.'
3 | from x import a, b.c, d, e.f, g
|


|
1 | from x import a.
2 | from x import a.b
3 | from x import a, b.c, d, e.f, g
| ^ Syntax Error: Expected ',', found '.'
|


|
1 | from x import a.
2 | from x import a.b
3 | from x import a, b.c, d, e.f, g
| ^ Syntax Error: Expected ',', found '.'
|

0 comments on commit cefa753

Please sign in to comment.