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

ContainsArguments incorrectly descends into the Initializer of a FieldDefinition #2438

Open
rbuckton opened this issue Jun 17, 2021 · 0 comments

Comments

@rbuckton
Copy link
Contributor

Contains gets around this by shunting to ComputedPropertyContains for a ClassTail. It looks like this was missed in ContainsArguments, which will incorrectly look for arguments in a FieldDefinition's initializer. In general this isn't a problem because that would be a static error anyways, but it does mean that ContainsArguments incorrectly and unnecessarily recurs through nested FieldDefinition declarations:

class A {
  static B = class {
    static C = class {
      static D = arguments; // static error, but for `A`
    }
  }
}

In this case, ContainsArguments would descend all the way to D when checking the static semantics of A, B, and C, when only ContainsArguments for C really needs to do that descent. Instead, ContainsArguments should only check a ComputedPropertyName of the FieldDefinition. In the case where there's a static error, per the spec it would be incorrectly attributed to A and not C above.

If there's no static error, then ContainsArguments needlessly descends though the AST when it encounters the Initializer of a FieldDefinition:

class A {
  static B = class {
    static C = class {
      static D = 0;
    }
  }
}

In this case, ContainsArguments descends all the way to D for A, B, and C, which would be a waste of processing time in a literal interpretation of the specification.

I suggest that ContainsArguments be amended to include this rule:

FieldDefinition : ClassElementName Initializer?
  1. Return ContainsArguments of ClassElementName.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant