Skip to content

fix: skip files not having extraction functions to improve speed#32

Merged
ramsey merged 1 commit intomainfrom
fix/improve-extraction-speed
Dec 17, 2021
Merged

fix: skip files not having extraction functions to improve speed#32
ramsey merged 1 commit intomainfrom
fix/improve-extraction-speed

Conversation

@ramsey
Copy link
Copy Markdown
Contributor

@ramsey ramsey commented Dec 17, 2021

Description

Before fully parsing the code to get the parameters of the string extraction functions, we'll first check the file contents to see if any of the functions exist (using string comparison). If we don't find them in the source code, we can skip parsing the contents.

This saves on some processing time for large code bases.

Product requirements and context

How has this been tested?

PR Checklist

  • I have added tests to cover my changes.

@ramsey ramsey requested review from a team, jmauerhan, jrode and xiian December 17, 2021 22:00
@ramsey ramsey force-pushed the fix/improve-extraction-speed branch from c3e11ea to 582e2ec Compare December 17, 2021 22:02
@ramsey ramsey force-pushed the fix/improve-extraction-speed branch from 582e2ec to 057735b Compare December 17, 2021 22:03
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 057735b and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 96.9% (0.0% change).

View more on Code Climate.

private function hasFormattingFunctions(string $code, array $functions): bool
{
foreach ($functions as $function) {
if (mb_strpos($code, $function, 0, Parser::ENCODING) !== false) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have some false positives too.... if we're looking for function foo but there is only a fooBar defined, it'd still match, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. There will be false positives. We're currently looking only for formatMessage, so if formatMessage appears only in a comment, this will match. That's because we haven't yet parsed the file to find out the context of the strings, so we don't know whether they're real function calls, etc.

We might be able to iterate on this to make it better at matching.

@ramsey ramsey merged commit b61ad44 into main Dec 17, 2021
@ramsey ramsey deleted the fix/improve-extraction-speed branch December 17, 2021 22:12
Copy link
Copy Markdown

@jrode jrode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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.

3 participants