Skip to content
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

MethodDefinition parsing #292

Closed
jasonwilliams opened this issue Mar 31, 2020 · 6 comments · Fixed by #339
Closed

MethodDefinition parsing #292

jasonwilliams opened this issue Mar 31, 2020 · 6 comments · Fixed by #339
Assignees
Labels
good first issue Good for newcomers parser Issues surrounding the parser
Milestone

Comments

@jasonwilliams
Copy link
Member

jasonwilliams commented Mar 31, 2020

We currently don't parse getters and setters on object literals.
There needs to be a new parsing section for MethodDefinitions.

We already have readObjectLiteral, so the logic would need to be called from within there.
https://github.com/jasonwilliams/boa/blob/master/boa/src/syntax/parser/mod.rs#L1599

This would either need to be rebased on jasonwilliams#281 or on master once #281 is in.

Spec:
https://tc39.es/ecma262/#prod-MethodDefinition

Previous Rust Code (you don't have to use this)

        TODO need to revisit this
        if let TokenKind::Identifier(name) = tok.kind {
            if name == "get" || name == "set" {
                let may_identifier = self.peek_skip_lineterminator();
                if may_identifier.is_ok()
                    && matches!(may_identifier.unwrap().kind, TokenKind::Identifier(_))
                {
                    let f = self.read_function_expression()?;
                    let func_name = if let NodeBase::FunctionExpr(ref name, _, _) = f.base {
                        name.clone().unwrap()
                    } else {
                        panic!()
                    };
                    return Ok(PropertyDefinition::MethodDefinition(
                        if name == "get" {
                            MethodDefinitionKind::Get
                        } else {
                            MethodDefinitionKind::Set
                        },
                        func_name,
                        f,
                    ));
                }
            }

            return Ok(PropertyDefinition::IdentifierReference(name));
        }
@jasonwilliams jasonwilliams added the parser Issues surrounding the parser label Mar 31, 2020
@jasonwilliams jasonwilliams added the good first issue Good for newcomers label Apr 13, 2020
@muskuloes
Copy link
Contributor

I'll like to work on this.

@HalidOdat
Copy link
Member

I'll like to work on this.

Go for it. 👍

@muskuloes
Copy link
Contributor

I think the link in the description should point to a specific file but it points to a whole pull request with several files and I don't know where to look. Also, Is the code snippet in the description the part I should refactor?

@Razican
Copy link
Member

Razican commented Apr 17, 2020

I think the link in the description should point to a specific file but it points to a whole pull request with several files and I don't know where to look. Also, Is the code snippet in the description the part I should refactor?

Good catch, I updated the link in the description. It should be https://github.com/jasonwilliams/boa/blob/master/boa/src/syntax/parser/mod.rs#L1599

@muskuloes
Copy link
Contributor

if I get this correctly: I will have to define a different method to parse js methods. Right now only Ordinary methods are parsed within the read_property_definition method of the Parser, which returns a result with PropertyDefintion. So, if I am to define a new method for parsing all method definitions, I will return a result with MethodDefinition. This will mean extracting the MethodDefinition variant of the PropertyDefinition enum as its own separed struct/enum. This method will be called in read_object_literal (the link above). So a method shouldn't be parsed within property definition.

@HalidOdat
Copy link
Member

if I get this correctly: I will have to define a different method to parse js methods. Right now only Ordinary methods are parsed within the read_property_definition method of the Parser, which returns a result with PropertyDefintion. So, if I am to define a new method for parsing all method definitions, I will return a result with MethodDefinition. This will mean extracting the MethodDefinition variant of the PropertyDefinition enum as its own separed struct/enum. This method will be called in read_object_literal (the link above). So a method shouldn't be parsed within property definition.

Pretty much.

This will mean extracting the MethodDefinition variant of the PropertyDefinition enum as its own separed struct/enum.

You don't really have to extract MethodDefinition. You can have the new function return PropertyDefintion.

@Razican Razican linked a pull request Apr 21, 2020 that will close this issue
@Razican Razican added this to the v0.8.0 milestone Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers parser Issues surrounding the parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants