Skip to content

Commit 76a9d40

Browse files
stepanchegfacebook-github-bot
stepancheg
authored and
facebook-github-bot
committed
LoadArgP
Summary: Struct with two fields instead of tuple. Mechanical refactoring. Reviewed By: IanChilds Differential Revision: D48940698 fbshipit-source-id: 2db5d498dab25b849b58a589ce791099287d863f
1 parent 15021ff commit 76a9d40

File tree

16 files changed

+99
-54
lines changed

16 files changed

+99
-54
lines changed

‎starlark/src/analysis/incompatible.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use starlark_syntax::syntax::ast::AstStmt;
2727
use starlark_syntax::syntax::ast::BinOp;
2828
use starlark_syntax::syntax::ast::DefP;
2929
use starlark_syntax::syntax::ast::Expr;
30+
use starlark_syntax::syntax::ast::LoadArgP;
3031
use starlark_syntax::syntax::ast::Stmt;
3132
use thiserror::Error;
3233

@@ -181,8 +182,8 @@ fn duplicate_top_level_assignment(module: &AstModule, res: &mut Vec<LintT<Incomp
181182
}
182183
Stmt::Def(DefP { name, .. }) => ident(name, false, codemap, defined, res),
183184
Stmt::Load(load) => {
184-
for (name, _) in &load.args {
185-
ident(name, true, codemap, defined, res)
185+
for LoadArgP { local, .. } in &load.args {
186+
ident(local, true, codemap, defined, res)
186187
}
187188
}
188189
// Visit statements, but don't descend under def - only top-level statements are interesting

‎starlark/src/analysis/names.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use starlark_syntax::syntax::ast::Clause;
4141
use starlark_syntax::syntax::ast::Expr;
4242
use starlark_syntax::syntax::ast::ForClause;
4343
use starlark_syntax::syntax::ast::ForP;
44+
use starlark_syntax::syntax::ast::LoadArgP;
4445
use starlark_syntax::syntax::ast::Stmt;
4546
use thiserror::Error;
4647

@@ -520,8 +521,8 @@ impl<'a> State<'a> {
520521
}
521522
// These were handled by collecting the scopes
522523
Stmt::Load(x) => {
523-
for (ident, _) in &x.args {
524-
self.set_ident(ident, Kind::Load)
524+
for LoadArgP { local, .. } in &x.args {
525+
self.set_ident(local, Kind::Load)
525526
}
526527
}
527528
// These control flow operators can be ignored - either the code after is fine (no problem)

‎starlark/src/analysis/underscore.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ fn use_ignored(codemap: &CodeMap, x: &AstStmt, res: &mut Vec<LintT<UnderscoreWar
119119
}
120120
Stmt::Load(xs) => {
121121
for x in &xs.args {
122-
res.insert(x.0.ident.as_str());
122+
res.insert(x.local.ident.as_str());
123123
}
124124
}
125125
_ => x.visit_stmt(|x| root_definitions(x, res)),

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,19 @@ impl<'v> Compiler<'v, '_, '_> {
7373
Some(loader) => expr_throw(loader.load(name), span, self.eval)?,
7474
};
7575

76-
for (our_name, their_name) in &load.node.args {
76+
for load_arg in &load.node.args {
7777
let (slot, _captured) = self
7878
.scope_data
79-
.get_assign_ident_slot(our_name, &self.codemap);
79+
.get_assign_ident_slot(&load_arg.local, &self.codemap);
8080
let slot = match slot {
8181
Slot::Local(..) => unreachable!("symbol need to be resolved to module"),
8282
Slot::Module(slot) => slot,
8383
};
8484
let value = expr_throw(
85-
self.eval.module_env.load_symbol(&loadenv, &their_name.node),
86-
FrameSpan::new(FrozenFileSpan::new(
87-
self.codemap,
88-
our_name.span.merge(their_name.span),
89-
)),
85+
self.eval
86+
.module_env
87+
.load_symbol(&loadenv, &load_arg.their.node),
88+
FrameSpan::new(FrozenFileSpan::new(self.codemap, load_arg.span())),
9089
self.eval,
9190
)?;
9291
self.eval.set_slot_module(slot, value)

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use starlark_syntax::syntax::ast::ExprP;
3939
use starlark_syntax::syntax::ast::ForClauseP;
4040
use starlark_syntax::syntax::ast::ForP;
4141
use starlark_syntax::syntax::ast::LambdaP;
42+
use starlark_syntax::syntax::ast::LoadArgP;
4243
use starlark_syntax::syntax::ast::Stmt;
4344
use starlark_syntax::syntax::ast::StmtP;
4445
use starlark_syntax::syntax::ast::Visibility;
@@ -887,13 +888,13 @@ impl StmtCollectDefines for Stmt {
887888
true => Visibility::Public,
888889
false => Visibility::Private,
889890
};
890-
for (name, _) in &mut load.args {
891+
for LoadArgP { local, .. } in &mut load.args {
891892
let mut vis = vis;
892-
if Module::default_visibility(&name.ident) == Visibility::Private {
893+
if Module::default_visibility(&local.ident) == Visibility::Private {
893894
vis = Visibility::Private;
894895
}
895896
AssignIdent::collect_assign_ident(
896-
name,
897+
local,
897898
in_loop,
898899
vis,
899900
scope_data,

‎starlark/src/typing/fill_types_for_lint.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use starlark_syntax::syntax::ast::BinOp;
2929
use starlark_syntax::syntax::ast::DefP;
3030
use starlark_syntax::syntax::ast::ExprP;
3131
use starlark_syntax::syntax::ast::ForP;
32+
use starlark_syntax::syntax::ast::LoadArgP;
3233
use starlark_syntax::syntax::ast::LoadP;
3334
use starlark_syntax::syntax::ast::StmtP;
3435
use starlark_syntax::syntax::def::DefParamKind;
@@ -290,9 +291,9 @@ impl<'a, 'v> GlobalTypesBuilder<'a, 'v> {
290291
}
291292

292293
fn load(&mut self, load: &LoadP<CstPayload>) -> Result<(), InternalError> {
293-
for (var, source) in &load.args {
294-
let ty = load.payload.get(source).cloned().unwrap_or_else(Ty::any);
295-
self.assign_ident_value(var, GlobalValue::ty(ty))?;
294+
for LoadArgP { local, their } in &load.args {
295+
let ty = load.payload.get(their).cloned().unwrap_or_else(Ty::any);
296+
self.assign_ident_value(local, GlobalValue::ty(ty))?;
296297
}
297298
Ok(())
298299
}

‎starlark_lsp/src/bind.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,9 +326,9 @@ fn stmt(x: &AstStmt, res: &mut Vec<Bind>) {
326326
res.push(Bind::Set(
327327
Assigner::Load {
328328
path: load.module.clone(),
329-
name: x.1.clone(),
329+
name: x.their.clone(),
330330
},
331-
x.0.clone(),
331+
x.local.clone(),
332332
))
333333
}
334334
}

‎starlark_lsp/src/definition.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ use starlark_syntax::syntax::ast::AstNoPayload;
3232
use starlark_syntax::syntax::ast::AstString;
3333
use starlark_syntax::syntax::ast::Expr;
3434
use starlark_syntax::syntax::ast::ExprP;
35+
use starlark_syntax::syntax::ast::LoadArgP;
3536
use starlark_syntax::syntax::ast::Stmt;
3637
use starlark_syntax::syntax::ast::StmtP;
3738
use starlark_syntax::syntax::top_level_stmts::top_level_stmts;
@@ -572,13 +573,13 @@ impl LspModule {
572573
path: load.module.node.to_owned(),
573574
})
574575
} else {
575-
load.args.iter().find_map(|(assign, name)| {
576-
if assign.span.contains(pos) || name.span.contains(pos) {
576+
load.args.iter().find_map(|LoadArgP { local, their }| {
577+
if local.span.contains(pos) || their.span.contains(pos) {
577578
Some(IdentifierDefinition::LoadedLocation {
578-
source: codemap.resolve_span(name.span),
579-
destination: codemap.resolve_span(name.span),
579+
source: codemap.resolve_span(their.span),
580+
destination: codemap.resolve_span(their.span),
580581
path: load.module.node.to_owned(),
581-
name: name.node.to_owned(),
582+
name: their.node.to_owned(),
582583
})
583584
} else {
584585
None

‎starlark_lsp/src/inspect.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use starlark_syntax::syntax::ast::AstLiteral;
2828
use starlark_syntax::syntax::ast::AstNoPayload;
2929
use starlark_syntax::syntax::ast::AstStmtP;
3030
use starlark_syntax::syntax::ast::ExprP;
31+
use starlark_syntax::syntax::ast::LoadArgP;
3132
use starlark_syntax::syntax::ast::ParameterP;
3233
use starlark_syntax::syntax::ast::StmtP;
3334
use starlark_syntax::syntax::uniplate::Visit;
@@ -150,16 +151,16 @@ impl AstModuleInspect for AstModule {
150151
});
151152
}
152153

153-
for (name, _) in &load.args {
154-
if name.span.contains(position) {
154+
for LoadArgP { local, .. } in &load.args {
155+
if local.span.contains(position) {
155156
return Some(AutocompleteType::LoadSymbol {
156157
path: load.module.to_string(),
157-
current_span: string_span_without_quotes(codemap, name.span),
158+
current_span: string_span_without_quotes(codemap, local.span),
158159
previously_loaded: load
159160
.args
160161
.iter()
161-
.filter(|(n, _)| n != name)
162-
.map(|(n, _)| n.to_string())
162+
.filter(|load_arg| &load_arg.local != local)
163+
.map(|LoadArgP { local, .. }| local.to_string())
163164
.collect(),
164165
});
165166
}

‎starlark_lsp/src/loaded.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl AstModuleLoadedSymbols for AstModule {
4646
})
4747
.flat_map(|l| {
4848
l.args.iter().map(|symbol| LoadedSymbol {
49-
name: &symbol.1,
49+
name: &symbol.their,
5050
loaded_from: &l.module,
5151
})
5252
})

0 commit comments

Comments
 (0)