Fixing constant indexer #2249
Merged
Fixing constant indexer #2249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dantleech
reviewed
May 1, 2023
| $record->setFilePath($document->uri()->path()); | ||
| $index->write($record); | ||
| $firstArgument = $expression; | ||
| break; |
There was a problem hiding this comment.
i think we just needed to add the break and the original code was ok? doing it like this means dealing with NULL and adding a temporary var on line 85. if we want to make the code "cleaner" we can extract a method, but I think just adding break is enough 🙂
|
nice find! if you agree can you update the code as per the suggestion and update the CHANGELOG? 🙇 |
|
Yeah that implementation would be much easier. Fixed it now. |
mamazu
force-pushed
the
fixing_constant_indexer
branch
from
5c93805
to
bf8f7df
Compare
mamazu
force-pushed
the
fixing_constant_indexer
branch
from
bf8f7df
to
848e80f
Compare
dantleech
approved these changes
May 1, 2023
|
Merci 🙇 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2168
What was the problem?
The problem was that in the old code phpactor created a constant for every (string literal) argument in a define call. This isn't correct since the first argument in the call is the name and the second argument is the value of the constant. The multilineness of the value was what tricked the cache. But this isn't a problem since the second argument shouldn't be a constant in the first place.