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

File::getDeclarationName(): prevent incorrect result during live coding #816

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Feb 12, 2025

Description

GetDeclarationNameTest: move parse error test to separate file

File::getDeclarationName(): prevent incorrect result during live coding

The function keyword for a closure is normally tokenized as T_CLOSURE, but will be tokenized as T_FUNCTION in case of an unfinished closure declaration (no scope opener/closer).

For an unfinished closure with typed parameters, the File::getDeclarationName() method would incorrectly return the contents of the first T_STRING in the type declaration as if it were the name of the function.

Fixed now.

Includes test.

Suggested changelog entry

File::getDeclarationName(): prevent incorrect result for unfinished closures during live coding

Related issues/external references

Loosely related to #152

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

The `function` keyword for a closure is normally tokenized as `T_CLOSURE`, but will be tokenized as `T_FUNCTION` in case of an unfinished closure declaration (no scope opener/closer).

For an unfinished closure with typed parameters, the `File::getDeclarationName()` method would incorrectly return the contents of the first `T_STRING` in the type declaration as if it were the name of the function.

Fixed now.

Includes test.
@jrfnl jrfnl added this to the 3.12.0 milestone Feb 12, 2025
@jrfnl jrfnl requested a review from fredden February 12, 2025 23:58
@jrfnl
Copy link
Member Author

jrfnl commented Feb 13, 2025

FYI: Build failure is unrelated to this PR, but has to do with an outage at Coveralls. See lemurheavy/coveralls-public#1791 and https://status.coveralls.io/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant