Skip to content

Commit

Permalink
Code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
pdohogne-magento committed Feb 26, 2020
1 parent d546194 commit 348ec7d
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/Analyzer/AbstractCodeAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ protected function getFileName($context, $nodeName, $isBefore = true)
* @param string $context
* @param string $fileBefore
* @param string $fileAfter
* @return array
* @return AbstractCodeAnalyzer[]
*/
protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
{
Expand Down
4 changes: 1 addition & 3 deletions src/Analyzer/ClassAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
$classAfter = $afterNameMap[$key];

if ($classBefore !== $classAfter) {
/** @var AnalyzerInterface[] $analyzers */
$analyzers = $this->getContentAnalyzers(static::CONTEXT, $fileBefore, $fileAfter);

foreach ($analyzers as $analyzer) {
Expand All @@ -125,7 +124,7 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
* @param string $context
* @param string $fileBefore
* @param string $fileAfter
* @return array
* @return AbstractCodeAnalyzer[]
*/
protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
{
Expand All @@ -140,7 +139,6 @@ protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
];
}


/**
* Get the class node registry
*
Expand Down
5 changes: 1 addition & 4 deletions src/Analyzer/InterfaceAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
$interfaceAfter = $afterNameMap[$key];

if ($interfaceBefore !== $interfaceAfter) {
/** @var AnalyzerInterface[] $analyzers */
$analyzers = $this->getContentAnalyzers(static::CONTEXT, $fileBefore, $fileAfter);

foreach ($analyzers as $analyzer) {
Expand All @@ -135,7 +134,7 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
* @param string $context
* @param string $fileBefore
* @param string $fileAfter
* @return array
* @return AbstractCodeAnalyzer[]
*/
protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
{
Expand All @@ -146,6 +145,4 @@ protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
new InterfaceExtendsAnalyzer($context, $fileBefore, $fileAfter)
];
}


}
6 changes: 1 addition & 5 deletions src/Analyzer/TraitAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
if ($traitBefore !== $traitAfter) {
$fileBefore = $this->getFileName($registryBefore, $traitName);
$fileAfter = $this->getFileName($registryAfter, $traitName);

/** @var AbstractCodeAnalyzer[] $analyzers */
$analyzers = $this->getContentAnalyzers(static::CONTEXT, $fileBefore, $fileAfter);

foreach ($analyzers as $analyzer) {
Expand All @@ -133,12 +131,10 @@ protected function reportChanged($report, $registryBefore, $registryAfter, $toVe
* @param string $context
* @param string $fileBefore
* @param string $fileAfter
* @return array
* @return AbstractCodeAnalyzer[]
*/
protected function getContentAnalyzers($context, $fileBefore, $fileAfter)
{
return [new ClassMethodAnalyzer($context, $fileBefore, $fileAfter)];
}


}
22 changes: 6 additions & 16 deletions src/DbSchemaReport.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,18 @@

class DbSchemaReport extends ReportAlias
{
public static $contexts = [
'class',
'function',
'interface',
'trait',
'database',
'di',
'layout',
'system',
'xsd',
'less'
];

/**
* Report constructor.
*/
public function __construct()
{
$levels = array_fill_keys(Level::asList(), []);
parent::__construct();
foreach (static::$contexts as $context) {
$this->differences[$context] = $levels;
}
$this->differences['database'] = $levels;
$this->differences['di'] = $levels;
$this->differences['layout'] = $levels;
$this->differences['system'] = $levels;
$this->differences['xsd'] = $levels;
$this->differences['less'] = $levels;
}
}
33 changes: 33 additions & 0 deletions src/MergedReport.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
// @codingStandardsIgnoreFile
namespace Magento\SemanticVersionChecker;

use PHPSemVerChecker\Report\Report;

class MergedReport extends Report
{
/**
* Merges with the given report including any non-standard contexts
*
* @param Report $report
* @return $this
*/
public function merge(Report $report)
{
foreach ($report->differences as $context => $levels) {
if (!key_exists($context, $this->differences)) {
$this->differences[$context] = $levels;
} else {
foreach ($levels as $level => $differences) {
$this->differences[$context][$level] = array_merge($this->differences[$context][$level], $differences);
}
}
}

return $this;
}
}
12 changes: 6 additions & 6 deletions src/ReportBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ class ReportBuilder

/**
* Define analyzer factory list for the different report types.
* @var array
* @var string[]
*/
private $analyzerList = [
private $analyzerFactoryClasses = [
ReportTypes::API => AnalyzerFactory::class,
ReportTypes::ALL => NonApiAnalyzerFactory::class,
ReportTypes::DB_SCHEMA => DbSchemaAnalyzerFactory::class,
Expand Down Expand Up @@ -121,11 +121,11 @@ protected function makeVersionReport()
/**
* Get the mapping of report type -> analyzer factory
*
* @return array
* @return string[]
*/
protected function getAnalyzerFactories()
protected function getAnalyzerFactoryClasses()
{
return $this->analyzerList;
return $this->analyzerFactoryClasses;
}

/**
Expand Down Expand Up @@ -171,7 +171,7 @@ protected function buildReport()
/**
* @var AnalyzerFactoryInterface $factory
*/
foreach ($this->getAnalyzerFactories() as $reportType => $factory) {
foreach ($this->getAnalyzerFactoryClasses() as $reportType => $factory) {
/** @var AnalyzerInterface $analyzer */
$analyzer = (new $factory())->create($dependencyMap);
$tmpReport = $analyzer->analyze(
Expand Down
8 changes: 4 additions & 4 deletions src/Reporter/BreakingChangeTableReporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
// @codingStandardsIgnoreFile
namespace Magento\SemanticVersionChecker\Reporter;

use Magento\SemanticVersionChecker\DbSchemaReport;
use PHPSemVerChecker\Report\Report;
use PHPSemVerChecker\SemanticVersioning\Level;
use Symfony\Component\Console\Output\OutputInterface;
Expand Down Expand Up @@ -52,14 +51,15 @@ public function __construct($apiChangeReport, $apiMembershipReport, $targetFile)
*/
public function output(OutputInterface $output)
{
$contexts = DbSchemaReport::$contexts;
$reportContexts = array_keys($this->report->getDifferences());
$membershipContexts = array_keys($this->membershipReport->getDifferences());

foreach ($contexts as $context) {
foreach ($reportContexts as $context) {
$header = static::formatSectionHeader($this->targetFile, $context, 'breaking-change');
$this->outputChangeReport($output, $this->report, $context, $header);
}

foreach ($contexts as $context) {
foreach ($membershipContexts as $context) {
$header = static::formatSectionHeader($this->targetFile, $context, 'api-membership');
$this->outputChangeReport($output, $this->membershipReport, $context, $header);
}
Expand Down
96 changes: 96 additions & 0 deletions tests/Unit/MergedReportTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\SemanticVersionChecker\Test\Unit;

use Magento\SemanticVersionChecker\InjectableReport;
use Magento\SemanticVersionChecker\MergedReport;
use Magento\SemanticVersionChecker\Operation\ClassConstantRemoved;
use Magento\SemanticVersionChecker\Operation\ClassConstructorOptionalParameterAdded;
use Magento\SemanticVersionChecker\Operation\ClassExtendsAdded;
use Magento\SemanticVersionChecker\Operation\ClassImplementsAdded;
use Magento\SemanticVersionChecker\Operation\ClassMethodOptionalParameterAdded;
use Magento\SemanticVersionChecker\Operation\ClassTraitAdded;
use Magento\SemanticVersionChecker\Operation\DropForeignKey;
use Magento\SemanticVersionChecker\Operation\SystemXml\FieldAdded;
use PHPSemVerChecker\Report\Report;
use PHPSemVerChecker\SemanticVersioning\Level;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

class MergedReportTest extends TestCase
{
/**
* Verify that reports are properly merged including non-standard contexts
*
* @return void
*/
public function testMergedChanges()
{
$diffsWithDatabase = [];
$diffsWithDatabase['class'][Level::PATCH] = [$this->createMock(ClassConstructorOptionalParameterAdded::class)];
$diffsWithDatabase['class'][Level::MINOR] = [
$this->createMock(ClassExtendsAdded::class),
$this->createMock(ClassMethodOptionalParameterAdded::class)
];
$diffsWithDatabase['database'][Level::MAJOR] = [$this->createMock(DropForeignKey::class)];
$databaseReport = new InjectableReport($diffsWithDatabase);

$diffsWithXml = [];
$diffsWithXml['class'][Level::MAJOR] = [$this->createMock(ClassConstantRemoved::class)];
$diffsWithXml['class'][Level::MINOR] = [$this->createMock(ClassImplementsAdded::class)];
$diffsWithXml['xml'][Level::MINOR] = [$this->createMock(FieldAdded::class)];
$xmlReport = new InjectableReport($diffsWithXml);

/** @var MockObject|ClassTraitAdded $preMergeOperation */
$preMergeOperation = $this->createMock(ClassTraitAdded::class);
$preMergeOperation->expects($this->any())->method('getLevel')->willReturn(Level::MINOR);

$mergedReport = new MergedReport();
$mergedReport->addClass($preMergeOperation);
$mergedReport->merge($databaseReport);
$mergedReport->merge($xmlReport);

$mergedDiffs = $mergedReport->getDifferences();
$this->assertTrue(key_exists('database', $mergedDiffs));
$this->assertTrue(key_exists('xml', $mergedDiffs));
$this->assertTrue($this->hasAllDifferences($mergedReport, $databaseReport));
$this->assertTrue($this->hasAllDifferences($mergedReport, $xmlReport));
$this->assertTrue(array_search($preMergeOperation, $mergedDiffs['class'][Level::MINOR]) !== false);
}

/**
* Checks if a merged report contains all differences in another report
* @param Report $mergedReport
* @param Report $inputReport
* @return bool
*/
private function hasAllDifferences($mergedReport, $inputReport)
{
$mergedDiffs = $mergedReport->getDifferences();
$inputDiffs = $inputReport->getDifferences();
foreach ($inputDiffs as $context => $levels) {
if (!key_exists($context, $mergedDiffs)) {
return false;
}
$mergedLevels = $mergedDiffs[$context];
foreach ($levels as $level => $operations) {
if (!key_exists($level, $mergedLevels)) {
return false;
}
$mergedOperations = $mergedLevels[$level];
foreach ($operations as $operation) {
if (array_search($operation, $mergedOperations) === false) {
return false;
}
}
}
}
return true;
}
}

0 comments on commit 348ec7d

Please sign in to comment.