Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## 0.3.2 - 2021-12-17

### Added

- Nothing.

### Changed

- Nothing.

### Deprecated

- Nothing.

### Removed

- Nothing.

### Fixed

- Check the contents of the file before parsing, to see if any of the formatting functions exist; if not, skip parsing the file

## 0.3.1 - 2021-12-17

### Added
Expand Down
3 changes: 2 additions & 1 deletion src/Console/Command/ExtractCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

use function array_filter;
use function array_map;
use function array_merge;
use function array_unique;
Expand Down Expand Up @@ -275,7 +276,7 @@ private function buildOptions(InputInterface $input): MessageExtractorOptions

/** @var string $inputFunctionNames */
$inputFunctionNames = $input->getOption('addl-func') ?? '';
$additionalFunctionNames = array_map('trim', explode(',', $inputFunctionNames));
$additionalFunctionNames = array_filter(array_map('trim', explode(',', $inputFunctionNames)));
$options->functionNames = array_unique(array_merge($options->functionNames, $additionalFunctionNames));

return $options;
Expand Down
23 changes: 22 additions & 1 deletion src/Extractor/Parser/Descriptor/PhpParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use FormatPHP\Extractor\MessageExtractorOptions;
use FormatPHP\Extractor\Parser\DescriptorParserInterface;
use FormatPHP\Extractor\Parser\ParserErrorCollection;
use FormatPHP\Icu\MessageFormat\Parser;
use FormatPHP\Util\FileSystemHelper;
use LogicException;
use PhpParser\Lexer;
Expand All @@ -43,6 +44,7 @@
use function assert;
use function count;
use function in_array;
use function mb_strpos;
use function pathinfo;

use const PATHINFO_EXTENSION;
Expand Down Expand Up @@ -88,9 +90,14 @@ public function __invoke(
return new DescriptorCollection();
}

$fileContents = $this->fileSystemHelper->getContents($filePath);
if (!$this->hasFormattingFunctions($fileContents, $options->functionNames)) {
return new DescriptorCollection();
}

$lexer = new Emulative(self::LEXER_OPTIONS);
$parser = new Php7Parser($lexer);
$statements = $parser->parse($this->fileSystemHelper->getContents($filePath));
$statements = $parser->parse($fileContents);

$descriptorCollector = new DescriptorCollectorVisitor(
$filePath,
Expand Down Expand Up @@ -199,4 +206,18 @@ private function isPhpFile(string $filePath): bool

return in_array($extension, self::PHP_PATH_EXTENSIONS);
}

/**
* @param string[] $functions
*/
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.

return true;
}
}

return false;
}
}
30 changes: 30 additions & 0 deletions tests/Extractor/Parser/Descriptor/PhpParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,36 @@ public function testParse11(): void
$this->assertCount(0, $receivedErrors);
}

/**
* This test covers situations where the formatting functions are not
* present in the source code being analyzed, so we should short-circuit
* and skip parsing the file.
*/
public function testParse12(): void
{
$fileSystemHelper = $this->mockery(FileSystemHelper::class);

$fileSystemHelper
->expects()
->getContents(__DIR__ . '/fixtures/php-parser-12.php')
->andReturn(file_get_contents(__DIR__ . '/fixtures/php-parser-12.php'));

$fileSystemHelper->shouldNotReceive('writeContents');

$errors = new ParserErrorCollection();

$options = new MessageExtractorOptions();
$options->functionNames = ['formatMessage', 'translate', 't'];

$parser = new PhpParser($fileSystemHelper);
$descriptors = $parser(__DIR__ . '/fixtures/php-parser-12.php', $options, $errors);
$receivedErrors = $this->compileErrors($errors);

$this->assertContainsOnlyInstancesOf(DescriptorInterface::class, $descriptors);
$this->assertCount(0, $descriptors);
$this->assertCount(0, $receivedErrors);
}

/**
* @return string[]
*/
Expand Down
8 changes: 8 additions & 0 deletions tests/Extractor/Parser/Descriptor/fixtures/php-parser-12.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

/**
* This PHP script has no FormatPHP functions in it, on purpose.
*/
foreach (['foo', 'bar', 'baz'] as $item) {
// Do nothing.
}