Skip to content

Commit 3886dae

Browse files
stepanchegfacebook-github-bot
stepancheg
authored and
facebook-github-bot
committed
Make IdentP fields named
Summary: More readable. Reviewed By: IanChilds Differential Revision: D48940191 fbshipit-source-id: 23043a2bf371cf5871d08f383b6d4ca97bfbd4bd
1 parent 8eb2f12 commit 3886dae

29 files changed

+177
-135
lines changed

‎starlark/src/analysis/dubious.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ fn duplicate_dictionary_key(module: &AstModule, res: &mut Vec<LintT<Dubious>>) {
8686
AstLiteral::String(x) => Some((Key::String(&x.node), x.span)),
8787
AstLiteral::Ellipsis => None,
8888
},
89-
Expr::Identifier(x) => Some((Key::Identifier(&x.node.0), x.span)),
89+
Expr::Identifier(x) => Some((Key::Identifier(&x.node.ident), x.span)),
9090
_ => None,
9191
}
9292
}
@@ -124,7 +124,7 @@ fn identifier_as_statement(module: &AstModule, res: &mut Vec<LintT<Dubious>>) {
124124
Expr::Identifier(x) => res.push(LintT::new(
125125
codemap,
126126
x.span,
127-
Dubious::IdentifierAsStatement(x.node.0.clone()),
127+
Dubious::IdentifierAsStatement(x.node.ident.clone()),
128128
)),
129129
_ => {}
130130
},

‎starlark/src/analysis/flow.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ fn returns(x: &AstStmt) -> Vec<(Span, Option<&AstExpr>)> {
9292
fn is_fail(x: &AstExpr) -> bool {
9393
match &**x {
9494
Expr::Call(x, _) => match &***x {
95-
Expr::Identifier(name) => name.node.0 == "fail",
95+
Expr::Identifier(name) => name.node.ident == "fail",
9696
_ => false,
9797
},
9898
_ => false,
@@ -135,7 +135,7 @@ fn require_return_expression(ret_type: &Option<Box<AstTypeExpr>>) -> Option<Span
135135
match ret_type {
136136
None => None,
137137
Some(x) => match &x.node.expr.node {
138-
Expr::Identifier(x) if x.node.0 == "None" => None,
138+
Expr::Identifier(x) if x.node.ident == "None" => None,
139139
_ => Some(x.span),
140140
},
141141
}
@@ -162,7 +162,7 @@ fn check_stmt(codemap: &CodeMap, x: &AstStmt, res: &mut Vec<LintT<FlowIssue>>) {
162162
x.span,
163163
FlowIssue::MissingReturn(
164164
// Statements often end with \n, so remove that to fit nicely
165-
name.node.0.trim_end().to_owned(),
165+
name.node.ident.trim_end().to_owned(),
166166
codemap.file_span(reason).resolve(),
167167
),
168168
));
@@ -173,7 +173,7 @@ fn check_stmt(codemap: &CodeMap, x: &AstStmt, res: &mut Vec<LintT<FlowIssue>>) {
173173
codemap,
174174
span,
175175
FlowIssue::MissingReturnExpression(
176-
name.0.clone(),
176+
name.ident.clone(),
177177
codemap.file_span(x.span).resolve(),
178178
codemap.file_span(reason).resolve(),
179179
),

‎starlark/src/analysis/incompatible.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ fn match_bad_type_equality(
7777
) {
7878
fn lookup_type<'a>(x: &AstExpr, types: &HashMap<&str, &'a str>) -> Option<&'a str> {
7979
match &**x {
80-
Expr::Identifier(name) => types.get(name.node.0.as_str()).copied(),
80+
Expr::Identifier(name) => types.get(name.node.ident.as_str()).copied(),
8181
_ => None,
8282
}
8383
}
@@ -86,7 +86,7 @@ fn match_bad_type_equality(
8686
fn is_type_call(x: &AstExpr) -> bool {
8787
match &**x {
8888
Expr::Call(fun, args) if args.len() == 1 => match &***fun {
89-
Expr::Identifier(x) => x.node.0 == "type",
89+
Expr::Identifier(x) => x.node.ident == "type",
9090
_ => false,
9191
},
9292
_ => false,
@@ -143,14 +143,14 @@ fn duplicate_top_level_assignment(module: &AstModule, res: &mut Vec<LintT<Incomp
143143
defined: &mut HashMap<&'a str, (Span, bool)>,
144144
res: &mut Vec<LintT<Incompatibility>>,
145145
) {
146-
if let Some((old, _)) = defined.get(x.0.as_str()) {
146+
if let Some((old, _)) = defined.get(x.ident.as_str()) {
147147
res.push(LintT::new(
148148
codemap,
149149
x.span,
150-
Incompatibility::DuplicateTopLevelAssign(x.0.clone(), codemap.file_span(*old)),
150+
Incompatibility::DuplicateTopLevelAssign(x.ident.clone(), codemap.file_span(*old)),
151151
));
152152
} else {
153-
defined.insert(&x.0, (x.span, is_load));
153+
defined.insert(&x.ident, (x.span, is_load));
154154
}
155155
}
156156

@@ -164,13 +164,13 @@ fn duplicate_top_level_assignment(module: &AstModule, res: &mut Vec<LintT<Incomp
164164
match &**x {
165165
Stmt::Assign(assign) => match (&assign.lhs.node, &assign.rhs.node) {
166166
(AssignTarget::Identifier(x), Expr::Identifier(y))
167-
if x.node.0 == y.node.0
168-
&& defined.get(x.node.0.as_str()).map_or(false, |x| x.1)
169-
&& !exported.contains(x.node.0.as_str()) =>
167+
if x.node.ident == y.node.ident
168+
&& defined.get(x.node.ident.as_str()).map_or(false, |x| x.1)
169+
&& !exported.contains(x.node.ident.as_str()) =>
170170
{
171171
// Normally this would be an error, but if we load()'d it, this is how we'd reexport through Starlark.
172172
// But only allow one export
173-
exported.insert(x.node.0.as_str());
173+
exported.insert(x.node.ident.as_str());
174174
}
175175
_ => assign
176176
.lhs

‎starlark/src/analysis/names.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,11 @@ impl<'a> AstStrExt<'a> for AstStr<'a> {
125125
}
126126

127127
fn ident(x: &'a AstIdent) -> Self {
128-
Self::new(x.span, x.node.0.as_str())
128+
Self::new(x.span, x.node.ident.as_str())
129129
}
130130

131131
fn assign_ident(x: &'a AstAssignIdent) -> Self {
132-
Self::new(x.span, x.node.0.as_str())
132+
Self::new(x.span, x.node.ident.as_str())
133133
}
134134
}
135135

@@ -148,7 +148,7 @@ enum Abort {
148148
fn is_fail(x: &AstExpr) -> bool {
149149
if let Expr::Call(x, _) = &**x {
150150
if let Expr::Identifier(x) = &***x {
151-
return x.0.as_str() == "fail";
151+
return x.ident.as_str() == "fail";
152152
}
153153
}
154154
false

‎starlark/src/analysis/performance.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ fn match_dict_copy(codemap: &CodeMap, x: &AstExpr, res: &mut Vec<LintT<Performan
5858
// If we see `dict(**x)` suggest `dict(x)`
5959
match &**x {
6060
Expr::Call(fun, args) if args.len() == 1 => match (&***fun, &*args[0]) {
61-
(Expr::Identifier(f), Argument::KwArgs(arg)) if f.node.0 == "dict" => {
61+
(Expr::Identifier(f), Argument::KwArgs(arg)) if f.node.ident == "dict" => {
6262
res.push(LintT::new(
6363
codemap,
6464
x.span,
@@ -75,27 +75,27 @@ fn match_inefficient_bool_check(codemap: &CodeMap, x: &AstExpr, res: &mut Vec<Li
7575
match &**x {
7676
Expr::Call(fun, args) if args.len() == 1 => match (&***fun, &*args[0]) {
7777
(Expr::Identifier(f), Argument::Positional(arg))
78-
if f.node.0 == "any" || f.node.0 == "all" =>
78+
if f.node.ident == "any" || f.node.ident == "all" =>
7979
{
8080
match &**arg {
8181
// any([blah for blah in blahs])
8282
Expr::ListComprehension(_, _, _) | Expr::DictComprehension(_, _, _) => res
8383
.push(LintT::new(
8484
codemap,
8585
x.span,
86-
Performance::EagerAndInefficientBoolCheck(f.node.0.clone()),
86+
Performance::EagerAndInefficientBoolCheck(f.node.ident.clone()),
8787
)),
8888
// any(list(_get_some_dict()))
8989
Expr::Call(any_call, _) => match &***any_call {
9090
Expr::Identifier(any_id)
91-
if any_id.node.0 == "dict" || any_id.node.0 == "list" =>
91+
if any_id.node.ident == "dict" || any_id.node.ident == "list" =>
9292
{
9393
res.push(LintT::new(
9494
codemap,
9595
x.span,
9696
Performance::InefficientBoolCheck(
9797
x.to_string(),
98-
any_id.node.0.clone(),
98+
any_id.node.ident.clone(),
9999
),
100100
))
101101
}

‎starlark/src/analysis/underscore.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,22 +80,22 @@ fn inappropriate_underscore(
8080

8181
match &**x {
8282
Stmt::Def(DefP { name, body, .. }) => {
83-
if !top && name.0.starts_with('_') {
83+
if !top && name.ident.starts_with('_') {
8484
res.push(LintT::new(
8585
codemap,
8686
name.span,
87-
UnderscoreWarning::UnderscoreDefinition(name.0.clone()),
87+
UnderscoreWarning::UnderscoreDefinition(name.ident.clone()),
8888
))
8989
}
9090
inappropriate_underscore(codemap, body, false, res)
9191
}
9292
Stmt::Assign(assign) if !top => {
9393
if let AssignTarget::Identifier(name) = &assign.lhs.node {
94-
if name.0.starts_with('_') && !is_allowed(&assign.rhs) {
94+
if name.ident.starts_with('_') && !is_allowed(&assign.rhs) {
9595
res.push(LintT::new(
9696
codemap,
9797
name.span,
98-
UnderscoreWarning::UnderscoreDefinition(name.node.0.clone()),
98+
UnderscoreWarning::UnderscoreDefinition(name.node.ident.clone()),
9999
))
100100
}
101101
}
@@ -111,15 +111,15 @@ fn use_ignored(codemap: &CodeMap, x: &AstStmt, res: &mut Vec<LintT<UnderscoreWar
111111
match &**x {
112112
Stmt::Assign(AssignP { lhs: x, .. }) | Stmt::AssignModify(x, _, _) => {
113113
x.visit_lvalue(|x| {
114-
res.insert(x.0.as_str());
114+
res.insert(x.ident.as_str());
115115
});
116116
}
117117
Stmt::Def(x) => {
118-
res.insert(x.name.0.as_str());
118+
res.insert(x.name.ident.as_str());
119119
}
120120
Stmt::Load(xs) => {
121121
for x in &xs.args {
122-
res.insert(x.0.0.as_str());
122+
res.insert(x.0.ident.as_str());
123123
}
124124
}
125125
_ => x.visit_stmt(|x| root_definitions(x, res)),
@@ -139,11 +139,11 @@ fn use_ignored(codemap: &CodeMap, x: &AstStmt, res: &mut Vec<LintT<UnderscoreWar
139139
) {
140140
match &**x {
141141
Expr::Identifier(x) => {
142-
if is_ignored(x.0.as_str()) && !roots.contains(x.0.as_str()) {
142+
if is_ignored(x.ident.as_str()) && !roots.contains(x.ident.as_str()) {
143143
res.push(LintT::new(
144144
codemap,
145145
x.span,
146-
UnderscoreWarning::UsingIgnored(x.node.0.clone()),
146+
UnderscoreWarning::UsingIgnored(x.node.ident.clone()),
147147
));
148148
}
149149
}

‎starlark/src/eval/compiler/def.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,10 @@ pub(crate) struct DefCompiled {
387387

388388
impl Compiler<'_, '_, '_> {
389389
fn parameter_name(&mut self, ident: &CstAssignIdent) -> ParameterName {
390-
let binding_id = ident.1.expect("no binding for parameter");
390+
let binding_id = ident.payload.expect("no binding for parameter");
391391
let binding = self.scope_data.get_binding(binding_id);
392392
ParameterName {
393-
name: ident.node.0.clone(),
393+
name: ident.node.ident.clone(),
394394
captured: binding.captured,
395395
}
396396
}

‎starlark/src/eval/compiler/expr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,9 +1167,9 @@ impl<'v, 'a, 'e> Compiler<'v, 'a, 'e> {
11671167
fn expr_ident(&mut self, ident: &CstIdent) -> ExprCompiled {
11681168
let resolved_ident = ident
11691169
.node
1170-
.1
1170+
.payload
11711171
.as_ref()
1172-
.unwrap_or_else(|| panic!("variable not resolved: `{}`", ident.node.0));
1172+
.unwrap_or_else(|| panic!("variable not resolved: `{}`", ident.node.ident));
11731173
match resolved_ident {
11741174
ResolvedIdent::Slot(Slot::Local(slot), binding_id) => {
11751175
let binding = self.scope_data.get_binding(*binding_id);

‎starlark/src/eval/compiler/scope/mod.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ impl<'f> ModuleScopeBuilder<'f> {
435435
.set_param_count(params.len().try_into().unwrap());
436436
let mut locals: SmallMap<FrozenStringValue, _> = SmallMap::new();
437437
for p in params {
438-
let name = frozen_heap.alloc_str_intern(&p.0);
438+
let name = frozen_heap.alloc_str_intern(&p.ident);
439439
// Subtle invariant: the slots for the params must be ordered and at the
440440
// beginning
441441
let binding_id = scope_data
@@ -446,7 +446,7 @@ impl<'f> ModuleScopeBuilder<'f> {
446446
AssignCount::AtMostOnce,
447447
)
448448
.0;
449-
p.1 = Some(binding_id);
449+
p.payload = Some(binding_id);
450450
let old_local = locals.insert_hashed(name.get_hashed(), binding_id);
451451
assert!(old_local.is_none());
452452
}
@@ -648,13 +648,17 @@ impl<'f> ModuleScopeBuilder<'f> {
648648

649649
fn variable_not_found_err(&self, ident: &CstIdent) -> EvalException {
650650
let variants = self.current_scope_all_visible_names_for_did_you_mean();
651-
let better = did_you_mean(ident.node.0.as_str(), variants.iter().map(|s| s.as_str()));
651+
let better = did_you_mean(
652+
ident.node.ident.as_str(),
653+
variants.iter().map(|s| s.as_str()),
654+
);
652655
EvalException::new(
653656
match better {
654-
Some(better) => {
655-
ScopeError::VariableNotFoundDidYouMean(ident.node.0.clone(), better.to_owned())
656-
}
657-
None => ScopeError::VariableNotFound(ident.node.0.clone()),
657+
Some(better) => ScopeError::VariableNotFoundDidYouMean(
658+
ident.node.ident.clone(),
659+
better.to_owned(),
660+
),
661+
None => ScopeError::VariableNotFound(ident.node.ident.clone()),
658662
}
659663
.into(),
660664
ident.span,
@@ -663,11 +667,11 @@ impl<'f> ModuleScopeBuilder<'f> {
663667
}
664668

665669
fn resolve_ident(&mut self, scope: ResolveIdentScope, ident: &mut CstIdent) {
666-
assert!(ident.node.1.is_none());
667-
let resolved = match self.get_name(self.frozen_heap.alloc_str_intern(&ident.node.0)) {
670+
assert!(ident.node.payload.is_none());
671+
let resolved = match self.get_name(self.frozen_heap.alloc_str_intern(&ident.node.ident)) {
668672
None => {
669673
// Must be a global, since we know all variables
670-
match self.globals.get_frozen(&ident.node.0) {
674+
match self.globals.get_frozen(&ident.node.ident) {
671675
None => {
672676
self.errors.push(self.variable_not_found_err(ident));
673677
return;
@@ -682,7 +686,7 @@ impl<'f> ModuleScopeBuilder<'f> {
682686
ResolveIdentScope::GlobalForTypeExpression => match resolved {
683687
ResolvedIdent::Slot(Slot::Local(_), _) => {
684688
self.errors.push(EvalException::new(
685-
ScopeError::TypeExpressionGlobalOrBuiltin(ident.node.0.clone()).into(),
689+
ScopeError::TypeExpressionGlobalOrBuiltin(ident.node.ident.clone()).into(),
686690
ident.span,
687691
&self.codemap,
688692
));
@@ -692,7 +696,7 @@ impl<'f> ModuleScopeBuilder<'f> {
692696
ResolvedIdent::Global(_) => {}
693697
},
694698
}
695-
ident.node.1 = Some(resolved);
699+
ident.node.payload = Some(resolved);
696700
}
697701

698702
fn resolve_idents_in_compr(
@@ -883,7 +887,7 @@ impl StmtCollectDefines for Stmt {
883887
};
884888
for (name, _) in &mut load.args {
885889
let mut vis = vis;
886-
if Module::default_visibility(&name.0) == Visibility::Private {
890+
if Module::default_visibility(&name.ident) == Visibility::Private {
887891
vis = Visibility::Private;
888892
}
889893
AssignIdent::collect_assign_ident(
@@ -972,9 +976,9 @@ impl AssignIdentCollect for AssignIdent {
972976
};
973977
}
974978
assign_ident_impl(
975-
frozen_heap.alloc_str_intern(&assign.node.0),
979+
frozen_heap.alloc_str_intern(&assign.node.ident),
976980
assign.span,
977-
&mut assign.node.1,
981+
&mut assign.node.payload,
978982
in_loop,
979983
vis,
980984
scope_data,
@@ -1179,7 +1183,7 @@ impl<'f> ModuleScopeData<'f> {
11791183
ident: &CstAssignIdent,
11801184
codemap: &CodeMap,
11811185
) -> (Slot, Captured) {
1182-
let binding_id = ident.1.expect("binding not assigned for ident");
1186+
let binding_id = ident.payload.expect("binding not assigned for ident");
11831187
let binding = self.get_binding(binding_id);
11841188
let slot = binding.resolved_slot(codemap).unwrap();
11851189
(slot, binding.captured)

‎starlark/src/eval/compiler/scope/payload.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ pub(crate) trait CstAssignIdentExt {
132132

133133
impl CstAssignIdentExt for CstAssignIdent {
134134
fn resolved_binding_id(&self, codemap: &CodeMap) -> Result<BindingId, InternalError> {
135-
match self.1 {
135+
match self.payload {
136136
Some(binding_id) => Ok(binding_id),
137137
None => Err(InternalError::msg(
138138
"Binding id is not filled",

0 commit comments

Comments
 (0)