Add TextRange to Identifier

Author: charliermarsh

ast/asdl_rs.py

@@ -514,6 +514,12 @@ def visitField(self, field, parent, vis, depth, constructor=None):
         if typ == "Int":
             typ = BUILTIN_INT_NAMES.get(field.name, typ)
         name = rust_field(field.name)
+
+        # Use a String, rather than an Identifier, for the `id` field of `Expr::Name`.
+        # Names already include a range, so there's no need to duplicate the span.
+        if name == "id":
+            typ = "String"
+
         self.emit(f"{vis}{name}: {typ},", depth)
 
     def visitProduct(self, product, type, depth):

ast/src/builtin.rs

@@ -2,85 +2,86 @@
 
 use num_bigint::BigInt;
 
+use crate::Ranged;
+use rustpython_parser_core::text_size::TextRange;
+
 pub type String = std::string::String;
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
-pub struct Identifier(String);
+pub struct Identifier {
+    id: String,
+    range: TextRange,
+}
 
 impl Identifier {
     #[inline]
-    pub fn new(s: impl Into<String>) -> Self {
-        Self(s.into())
+    pub fn new(id: impl Into<String>, range: TextRange) -> Self {
+        Self {
+            id: id.into(),
+            range,
+        }
     }
 }
 
 impl Identifier {
     #[inline]
     pub fn as_str(&self) -> &str {
-        self.0.as_str()
+        self.id.as_str()
     }
 }
 
-impl std::cmp::PartialEq<str> for Identifier {
+impl PartialEq<str> for Identifier {
     #[inline]
     fn eq(&self, other: &str) -> bool {
-        self.0 == other
+        self.id == other
     }
 }
 
-impl std::cmp::PartialEq<String> for Identifier {
+impl PartialEq<String> for Identifier {
     #[inline]
     fn eq(&self, other: &String) -> bool {
-        &self.0 == other
+        &self.id == other
     }
 }
 
 impl std::ops::Deref for Identifier {
     type Target = str;
     #[inline]
     fn deref(&self) -> &Self::Target {
-        self.0.as_str()
+        self.id.as_str()
     }
 }
 
 impl AsRef<str> for Identifier {
     #[inline]
     fn as_ref(&self) -> &str {
-        self.0.as_str()
+        self.id.as_str()
     }
 }
 
 impl AsRef<String> for Identifier {
     #[inline]
     fn as_ref(&self) -> &String {
-        &self.0
+        &self.id
     }
 }
 
 impl std::fmt::Display for Identifier {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.0.fmt(f)
+        self.id.fmt(f)
     }
 }
 
 impl From<Identifier> for String {
     #[inline]
-    fn from(id: Identifier) -> String {
-        id.0
+    fn from(identifier: Identifier) -> String {
+        identifier.id
     }
 }
 
-impl From<String> for Identifier {
-    #[inline]
-    fn from(id: String) -> Self {
-        Self(id)
-    }
-}
-
-impl<'a> From<&'a str> for Identifier {
-    #[inline]
-    fn from(id: &'a str) -> Identifier {
-        id.to_owned().into()
+impl Ranged for Identifier {
+    fn range(&self) -> TextRange {
+        self.range
     }
 }
 
@@ -207,6 +208,7 @@ impl std::fmt::Display for Constant {
 #[cfg(test)]
 mod tests {
     use super::*;
+
     #[test]
     fn test_is_macro() {
         let none = Constant::None;

ast/src/gen/generic.rs

@@ -1447,7 +1447,7 @@ impl<R> From<ExprStarred<R>> for Ast<R> {
 #[derive(Clone, Debug, PartialEq)]
 pub struct ExprName<R = TextRange> {
     pub range: R,
-    pub id: Identifier,
+    pub id: String,
     pub ctx: ExprContext,
 }
 

parser/src/function.rs

@@ -21,7 +21,13 @@ type ParameterDef = (ast::Arg, Option<ast::Expr>);
 pub(crate) fn validate_arguments(
     arguments: ast::Arguments,
 ) -> Result<ast::Arguments, LexicalError> {
-    let mut all_args: Vec<&ast::Arg> = vec![];
+    let mut all_args: Vec<&ast::Arg> = Vec::with_capacity(
+        arguments.posonlyargs.len()
+            + arguments.args.len()
+            + arguments.vararg.is_some() as usize
+            + arguments.kwonlyargs.len()
+            + arguments.kwarg.is_some() as usize,
+    );
 
     all_args.extend(arguments.posonlyargs.iter());
     all_args.extend(arguments.args.iter());
@@ -36,11 +42,11 @@ pub(crate) fn validate_arguments(
         all_args.push(a);
     }
 
-    let mut all_arg_names = FxHashSet::with_hasher(Default::default());
+    let mut all_arg_names = FxHashSet::with_capacity_and_hasher(all_args.len(), Default::default());
     for arg in all_args {
         let arg_name = &arg.arg;
         // Check for duplicate arguments in the function definition.
-        if !all_arg_names.insert(arg_name) {
+        if !all_arg_names.insert(arg_name.as_str()) {
             return Err(LexicalError {
                 error: LexicalErrorType::DuplicateArgumentError(arg_name.to_string()),
                 location: arg.start(),
@@ -104,22 +110,20 @@ pub(crate) fn parse_args(func_args: Vec<FunctionArgument>) -> Result<ArgumentLis
             Some((start, end, name)) => {
                 // Check for duplicate keyword arguments in the call.
                 if let Some(keyword_name) = &name {
-                    if keyword_names.contains(keyword_name) {
+                    if !keyword_names.insert(keyword_name.to_string()) {
                         return Err(LexicalError {
                             error: LexicalErrorType::DuplicateKeywordArgumentError(
                                 keyword_name.to_string(),
                             ),
                             location: start,
                         });
                     }
-
-                    keyword_names.insert(keyword_name.clone());
                 } else {
                     double_starred = true;
                 }
 
                 keywords.push(ast::Keyword {
-                    arg: name.map(ast::Identifier::new),
+                    arg: name.map(|name| ast::Identifier::new(name, TextRange::new(start, end))),
                     value,
                     range: TextRange::new(start, end),
                 });

parser/src/python.lalrpop

@@ -267,7 +267,7 @@ ImportAsNames: Vec<ast::Alias> = {
     <location:@L> "(" <i:OneOrMore<ImportAsAlias<Identifier>>> ","? ")" <end_location:@R> => i,
     <location:@L> "*" <end_location:@R> => {
         // Star import all
-        vec![ast::Alias { name: ast::Identifier::new("*"), asname: None, range: (location..end_location).into() }]
+        vec![ast::Alias { name: ast::Identifier::new("*", (location..end_location).into()), asname: None, range: (location..end_location).into() }]
     },
 };
 
@@ -279,14 +279,14 @@ ImportAsAlias<I>: ast::Alias = {
 
 // A name like abc or abc.def.ghi
 DottedName: ast::Identifier = {
-    <n:name> => ast::Identifier::new(n),
-    <n:name> <n2: ("." Identifier)+>  => {
+    <location:@L> <n:name> <end_location:@R> => ast::Identifier::new(n, (location..end_location).into()),
+    <location:@L> <n:name> <n2: ("." Identifier)+> <end_location:@R> => {
         let mut r = n.to_string();
         for x in n2 {
             r.push('.');
             r.push_str(x.1.as_str());
         }
-        ast::Identifier::new(r)
+        ast::Identifier::new(r, (location..end_location).into())
     },
 };
 
@@ -564,8 +564,8 @@ CapturePattern: ast::Pattern = {
 }
 
 MatchName: ast::Expr = {
-    <location:@L> <name:Identifier> <end_location:@R> => ast::Expr::Name(
-        ast::ExprName { id: name, ctx: ast::ExprContext::Load, range: (location..end_location).into() },
+    <location:@L> <id:Identifier> <end_location:@R> => ast::Expr::Name(
+        ast::ExprName { id: id.into(), ctx: ast::ExprContext::Load, range: (location..end_location).into() },
     ),
 }
 
@@ -1200,7 +1200,7 @@ NamedExpression: ast::Expr = {
         ast::Expr::NamedExpr(
             ast::ExprNamedExpr {
                 target: Box::new(ast::Expr::Name(
-                    ast::ExprName { id, ctx: ast::ExprContext::Store, range: (location..end_location).into() },
+                    ast::ExprName { id: id.into(), ctx: ast::ExprContext::Store, range: (location..end_location).into() },
                 )),
                 range: (location..value.end()).into(),
                 value: Box::new(value),
@@ -1427,8 +1427,8 @@ Atom<Goal>: ast::Expr = {
     <location:@L> <value:Constant> <end_location:@R> => ast::Expr::Constant(
         ast::ExprConstant { value, kind: None, range: (location..end_location).into() }
     ),
-    <location:@L> <name:Identifier> <end_location:@R> => ast::Expr::Name(
-        ast::ExprName { id: name, ctx: ast::ExprContext::Load, range: (location..end_location).into() }
+    <location:@L> <id:Identifier> <end_location:@R> => ast::Expr::Name(
+        ast::ExprName { id: id.into(), ctx: ast::ExprContext::Load, range: (location..end_location).into() }
     ),
     <location:@L> "[" <e:ListLiteralValues?> "]"<end_location:@R>  => {
         let elts = e.unwrap_or_default();
@@ -1663,7 +1663,7 @@ Constant: ast::Constant = {
 };
 
 Identifier: ast::Identifier = {
-    <s:name> => ast::Identifier::new(s)
+    <location:@L> <s:name> <end_location:@R> => ast::Identifier::new(s, (location..end_location).into())
 };
 
 // Hook external lexer: