Skip to content

Conversation

jeffijoe
Copy link
Owner

When parsing an identifier, the parser will treat this.constructor as a single identifier, which means it won't match the constructor token we are looking for when scanning for a constructor. However, when using this?.constructor, 2 identifier tokens would be yielded: this and constructor, which made the parser wrongfully assume it found the constructor. To fix this, the IDENT_PART_EXPR now includes ? as a valid identifier part in order to reduce the amount of false-positive constructor tokens.

However, as I was looking into this, I realized that if you do

const constructor = this?.constructor
return constructor.call(this)

Since constructor is still a valid identifier despite being a reserved keyword, JS allows this, which would similarly make the parser think it found the constructor.

To fix this, the parser will now check if the constructor token is followed by a ( token to consider it a valid constructor. The only false-positive that will get past these checks is

const constructor = something
return constructor(arg)

But I don't think that's a case that's common enough to worry about. Fixing that would require a more complex parser, which will affect performance.

Closes #390

Include `?` as part of the identifier to treat `this?.constructor` as a single identifier
which won't match a `constructor` identifier which is what we're looking for.

Closes #390
@jeffijoe jeffijoe merged commit 1aae3d4 into master Mar 14, 2025
5 checks passed
@jeffijoe jeffijoe deleted the fix/param-parser branch March 14, 2025 12:42
@coveralls
Copy link

Coverage Status

coverage: 100.0%. remained the same
when pulling 78da08d on fix/param-parser
into c10246c on master.

@coveralls
Copy link

Coverage Status

coverage: 100.0%. remained the same
when pulling 148225a on fix/param-parser
into c10246c on master.

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

Successfully merging this pull request may close these issues.

Awilix incorrectly parses the constructor of a class if the word constructor is used before the constructor definition.
2 participants