-
Notifications
You must be signed in to change notification settings - Fork 428
tsparser: usage of default exported resources #1970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
All committers have signed the CLA. |
@@ -39,7 +39,7 @@ pub const GATEWAY_PARSER: ResourceParser = ResourceParser { | |||
for r in iter_references::<Res>(&module, &names) { | |||
let r = report_and_continue!(r); | |||
|
|||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { | |||
None => None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could resolve the default export, but not sure how difficult that would be to do. If it's tricky maybe add a TODO here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how object is used, but I've given it a stab, it resolves to something, not sure exactly what we want it to resolve to?
@@ -61,7 +61,7 @@ fn parse_cron_job( | |||
pass: &mut ResourceParseContext, | |||
r: NamedClassResource<DecodedCronJobConfig>, | |||
) -> ParseResult<()> { | |||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -46,7 +46,7 @@ pub const OBJECTS_PARSER: ResourceParser = ResourceParser { | |||
let r = report_and_continue!(r); | |||
let cfg = r.config.unwrap_or_default(); | |||
|
|||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
}); | ||
} | ||
} | ||
|
||
{ | ||
for r in iter_references::<NamedStaticMethod>(&module, &names) { | ||
let r = report_and_continue!(r); | ||
let object = match &r.bind_name { | ||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -71,7 +71,7 @@ pub const SUBSCRIPTION_PARSER: ResourceParser = ResourceParser { | |||
spread.err("cannot use ... for PubSub topic reference"); | |||
continue; | |||
} | |||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -63,7 +63,7 @@ pub const TOPIC_PARSER: ResourceParser = ResourceParser { | |||
|
|||
for r in iter_references::<PubSubTopicDefinition>(&module, &names) { | |||
let r = report_and_continue!(r); | |||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -152,7 +152,7 @@ pub const SQLDB_PARSER: ResourceParser = ResourceParser { | |||
} | |||
}; | |||
|
|||
let object = match &r.bind_name { | |||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
}); | ||
} | ||
} | ||
|
||
{ | ||
for r in iter_references::<NamedStaticMethod>(&module, &names) { | ||
let r = report_and_continue!(r); | ||
let object = match &r.bind_name { | ||
let object = match r.bind_name.ident() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Some(id) => pass | ||
let object = match r.bind_name { | ||
BindName::Anonymous => None, | ||
BindName::DefaultExport(ref expr) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should be resolving this by passing in an expression to resolve_obj
.
resolve_obj
goes into self.expr(...)
which goes into a big match statement where we end up at:
ast::Expr::Ident(expr) => {
let Some(obj) = self.ident_obj(expr) else {
HANDLER.with(|handler| handler.span_err(expr.span, "unknown identifier"));
return Type::Basic(Basic::Never);
};
let named = Named::new(obj, vec![]);
Type::Named(named)
}
This goes into ident_obj(expr)
:
fn ident_obj(&self, ident: &ast::Ident) -> Option<Rc<Object>> {
// Does this represent a type parameter?
self.state.resolve_module_ident(self.module, ident)
}
}
And resolve_module_ident
checks:
// Is it a top-level object in this module?
let ast_id = AstId::from(ident);
if let Some(obj) = module.data.top_level.get(&ast_id) {
return Some(obj.clone());
}
I believe the correct way to resolve default exports is to do the same thing, but check:
if let Some(obj) = module.data.default_export {
return Some(obj.clone());
}
(sorry for the trouble)
the resolved bind of a default export will have no name, but we still need to add it or it wont be tracked