Skip to content

Commit

Permalink
Merge pull request #240 from totten/master-misc
Browse files Browse the repository at this point in the history
Upgrade - Fix cleanup logic for a few funny edge-cases
  • Loading branch information
totten committed May 13, 2022
2 parents e08f2a4 + b5ef6b1 commit b226a60
Show file tree
Hide file tree
Showing 6 changed files with 407 additions and 9 deletions.
59 changes: 50 additions & 9 deletions src/CRM/CivixBundle/Upgrader.php
Expand Up @@ -234,7 +234,17 @@ public function removeHookDelegation(array $names): void {
if (preg_match("|^(\s*)//(\s*)($nameQuoted\(.*)|", $line, $m)) {
// ok, already disabled
}
elseif (preg_match("|^(\s*)($nameQuoted\([^;]*;\s*)$|", $line, $m)) {
elseif (preg_match("|^(\s*)return ($nameQuoted\([^;]*;\s*)$|", $line, $m)) {
// Easy case - we can disable it.
$this->io->writeln(sprintf(
"<info>Found reference to obsolete function </info>%s()<info> at </info>%s:%d<info>.</info>\n",
$name, $mainPhp, 1 + $lineNum
));
$this->showLine($oldLines, $lineNum);
$this->io->writeln(sprintf("<info>Redacting call in </info>%s:%d<info></info>\n", $mainPhp, 1 + $lineNum));
$line = $m[1] . 'return;';
}
elseif (preg_match("|^(\s*)?($nameQuoted\([^;]*;\s*)$|", $line, $m)) {
// Easy case - we can disable it.
$this->io->writeln(sprintf(
"<info>Found reference to obsolete function </info>%s()<info> at </info>%s:%d<info>.</info>\n",
Expand Down Expand Up @@ -295,9 +305,23 @@ public function cleanEmptyHooks() {
$comment = "/\*\*\n( \*.*\n)* \*/";
$funcName = $infoXml->getFile() . "_civicrm_[a-zA-Z0-9_]+";
$funcArgs = "\([^\)]*\)";
$emptyBody = "\{\s*\}";
$content = preg_replace_callback("|({$comment})?\s*function ({$funcName})({$funcArgs})\s*{$emptyBody}\n*|m", function ($m) {
$startBody = "\{[^\}]*\}"; /* For empty functions, this grabs everything. For non-empty functions, this may just grab the opening segment. */
$content = preg_replace_callback(";({$comment})?\n\s*function ({$funcName})({$funcArgs})\s*({$startBody})\n*;m", function ($m) {
$func = $m[3];

// Is our start-body basically empty (notwithstanding silly things - like `{}`, `//Comment`, and `return;`)?
$mStartBody = explode("\n", $m[5]);
$mStartBody = preg_replace(';^\s*;', '', $mStartBody);
$mStartBody = preg_grep(';^\/\/;', $mStartBody, PREG_GREP_INVERT);
$mStartBody = preg_grep('/^$/', $mStartBody, PREG_GREP_INVERT);
$mStartBody = preg_grep('/^[\{\}]$/', $mStartBody, PREG_GREP_INVERT);
$mStartBody = preg_grep('/^return;$/', $mStartBody, PREG_GREP_INVERT);
$mStartBody = preg_grep('/^\{\s*\}$/', $mStartBody, PREG_GREP_INVERT);
if (!empty($mStartBody)) {
// There is some kind of substance in here...
return $m[0];
}

$this->io->note("The function \"{$func}()\" now appears to be empty.");
$this->showCode(explode("\n", $m[0]));
if ($this->io->confirm("Delete the empty function \"{$func}()\"?")) {
Expand Down Expand Up @@ -355,18 +379,35 @@ public function reloadInfo(): array {
* @param array $lines
* @param int $focusLine
*/
protected function showLine(array $lines, int $focusLine): void {
public function showLine(array $lines, int $focusLine): void {
$low = max(0, $focusLine - 2);
$high = min(count($lines), $focusLine + 2);
$this->showCode($lines, $low, $high, $focusLine);
$this->showCode($lines, $low, $high, $focusLine, $focusLine);
}

protected function showCode(array $lines, ?int $low = NULL, ?int $high = NULL, ?int $focusLine = NULL): void {
$low = $low ?: 0;
$high = $high ?: (count($lines) - 1);
/**
* Show a chunk of code.
*
* @param array $lines
* @param int|null $low
* The first line to show
* @param int|null $high
* The last line to show
* @param int|null $focusStart
* The first line to highlight (within the overall code)
* @param int|null $focusEnd
* The last line to highlight (within the overall code).
*/
public function showCode(array $lines, ?int $low = NULL, ?int $high = NULL, ?int $focusStart = NULL, ?int $focusEnd = NULL): void {
if ($low === NULL || $low < 0) {
$low = 0;
}
if ($high === NULL || $high >= count($lines)) {
$high = count($lines) - 1;
}
for ($i = $low; $i <= $high; $i++) {
$fmt = sprintf('% 5d', 1 + $i);
if ($i === $focusLine) {
if ($focusStart !== NULL && $focusEnd !== NULL && $i >= $focusStart && $i <= $focusEnd) {
$this->io->write("<error>*{$fmt}: ");
$this->io->write($lines[$i], SymfonyStyle::OUTPUT_RAW);
$this->io->write("</error>");
Expand Down
96 changes: 96 additions & 0 deletions src/CRM/CivixBundle/Utils/EvilEx.php
@@ -0,0 +1,96 @@
<?php

namespace CRM\CivixBundle\Utils;

/**
* Evil Expressions: What happens when you don't import nikic/php-parser.
*/
class EvilEx {

/**
* Find a function-body, and run it through a filter.
*
* Limitation: This only matches top-level functions, where the `function x()` begins in the leftmost,
* and where the closing `}` is als in the leftmost, and where the body is indented.
*
* @param string $body
* Full-text file.
* @param string $function
* Name of the function to filter.
* @param callable $filter
* Filter the function-body; return new body.
* function(string $body): string
* @return string
* Updated body.
*/
public static function rewriteFunction(string $body, string $function, callable $filter): string {
$pattern = "/(\nfunction " . $function . "\([^{]+\)\s*{\n)((\n| [^\n]*\n)*)(}\n)/m";
return preg_replace_callback($pattern,
function ($m) use ($filter) {
$body = $m[2];
$newBody = $filter($body);
return $m[1] . $newBody . $m[4];
},
$body
);
}

/**
* Searches for a chunk of code - and replaces it.
*
* When matching the chunk of code, it specifically ignores whitespace and blank lines.
*
* @param string $body
* @param array $matchChunk
* A series of lines which should be found somewhere inside $body (modulo whitespace).
* @param callable $filterChunk
* Filter the matching lines; return new lines.
* function(array $matchLines): array
* @return string
* Updated body.
*/
public static function rewriteMultilineChunk(string $body, array $matchChunk, callable $filterChunk) {
$expectLines = EvilEx::digestLines($matchChunk);
$actualLines = EvilEx::digestLines(explode("\n", $body));
foreach (array_keys($actualLines) as $startOffset) {
$endOffset = NULL;
$expectLineNum = 0;
for ($actualLineNum = $startOffset; $actualLineNum < count($actualLines); $actualLineNum++) {
if (empty($actualLines[$actualLineNum]['dig'])) {
continue;
}
if ($expectLines[$expectLineNum]['dig'] !== $actualLines[$actualLineNum]['dig']) {
continue 2;
}
$expectLineNum++;
if ($expectLineNum >= count($expectLines)) {
$endOffset = $actualLineNum;
break 2;
}
}
}

if ($endOffset === NULL) {
return $body;
}

$rawActualLines = array_column($actualLines, 'raw');
$matchLines = array_slice($rawActualLines, $startOffset, $endOffset - $startOffset + 1, TRUE);
$newLines = $filterChunk($matchLines);
array_splice($rawActualLines, $startOffset, $endOffset - $startOffset + 1, $newLines);
return implode("\n", $rawActualLines);
}

public static function digestLine(string $line): string {
return mb_strtolower(preg_replace('/\s+/', '', $line));
}

public static function digestLines($lines): array {
$result = [];
foreach ($lines as $line) {
$result[] = ['raw' => $line, 'dig' => static::digestLine($line)];
}
return $result;
}

}
92 changes: 92 additions & 0 deletions tests/e2e/CleanEmptyTest.in.txt
@@ -0,0 +1,92 @@
<?php
// [[ EX - Keep pure comment ]]

// function civixcleanempty_civicrm_keepPureComment() {
// foo
// }

// [[ EX - Removable 1 ]]

function civixcleanempty_civicrm_removeOne() {
}

// [[ EX - Keepable 1 ]]

function civixcleanempty_civicrm_keepOne() {
stuff();
}

// [[ EX - Removable 2 ]]

/**
*
*/
function civixcleanempty_civicrm_removeTwo() {

}

// [[ EX - Keepable 2 ]]

/**
* @param $x
*/
function civixcleanempty_civicrm_keepTwo($x) {
stuff($x);
}

// [[ EX - Removable 3 ]]

/**
*
*/
function civixcleanempty_civicrm_remove_three($x) {
// Do nothing
}

// [[ EX - Keepable 3 ]]

function civixcleanempty_civicrm_keepThree() {
stuff(); // Comment
}

// [[ EX - Removable 4 ]]

function civixcleanempty_civicrm_removeFour() {}

// [[ EX - Keepable 4 ]]

function civixcleanempty_civicrm_keepFour() {
foreach ($a as $b) {} x();
}

// [[ EX - Removable 5 ]]

function civixcleanempty_civicrm_removeFive() { }

// [[ EX - Keepable 5 ]]

function civixcleanempty_civicrm_keepFive() {
if (FOO) {
x();
}
y();
}

// [[ EX - Removable 6 ]]

function civixcleanempty_civicrm_removeSix() {
return;
}

// [[ EX - Keepable 6 ]]

function civixcleanempty_civicrm_keepSix() {
return;
$x++;
}

// [[ EX - Keepable 6a ]]

function civixcleanempty_civicrm_keepSixA() {
return 1;
}
71 changes: 71 additions & 0 deletions tests/e2e/CleanEmptyTest.out.txt
@@ -0,0 +1,71 @@
<?php
// [[ EX - Keep pure comment ]]

// function civixcleanempty_civicrm_keepPureComment() {
// foo
// }

// [[ EX - Removable 1 ]]

// [[ EX - Keepable 1 ]]

function civixcleanempty_civicrm_keepOne() {
stuff();
}

// [[ EX - Removable 2 ]]



// [[ EX - Keepable 2 ]]

/**
* @param $x
*/
function civixcleanempty_civicrm_keepTwo($x) {
stuff($x);
}

// [[ EX - Removable 3 ]]



// [[ EX - Keepable 3 ]]

function civixcleanempty_civicrm_keepThree() {
stuff(); // Comment
}

// [[ EX - Removable 4 ]]

// [[ EX - Keepable 4 ]]

function civixcleanempty_civicrm_keepFour() {
foreach ($a as $b) {} x();
}

// [[ EX - Removable 5 ]]

// [[ EX - Keepable 5 ]]

function civixcleanempty_civicrm_keepFive() {
if (FOO) {
x();
}
y();
}

// [[ EX - Removable 6 ]]

// [[ EX - Keepable 6 ]]

function civixcleanempty_civicrm_keepSix() {
return;
$x++;
}

// [[ EX - Keepable 6a ]]

function civixcleanempty_civicrm_keepSixA() {
return 1;
}
27 changes: 27 additions & 0 deletions tests/e2e/CleanEmptyTest.php
@@ -0,0 +1,27 @@
<?php

namespace E2E;

class CleanEmptyTest extends \PHPUnit\Framework\TestCase {

use CivixProjectTestTrait;

public static $key = 'civixcleanempty';

public function setUp(): void {
chdir(static::getWorkspacePath());
static::cleanDir(static::getKey());
$this->civixGenerateModule(static::getKey());
chdir(static::getKey());

$this->assertFileExists('info.xml');
}

public function testCleanup(): void {
$mainPhp = static::getKey() . '.php';
copy(__DIR__ . '/CleanEmptyTest.in.txt', $mainPhp);
$this->assertEquals(0, $this->civix('upgrade')->execute([]));
$this->assertFileEquals(__DIR__ . '/CleanEmptyTest.out.txt', $mainPhp);
}

}

0 comments on commit b226a60

Please sign in to comment.