Skip to content

refactor: reduce duplication in FileParser test cases #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 21, 2025
Merged

refactor: reduce duplication in FileParser test cases #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2db7701
refactor: reduce duplication in FileParser test cases
fain182 Jul 19, 2025
fafad34
refactor: simplify parseCode helper method signature
fain182 Jul 20, 2025
eeddf96
refactor: remove redundant reason parameter from evaluateRule
fain182 Jul 20, 2025
d3d9723
use php 8.4 to run csfix, to avoid incompatibilities
fain182 Jul 20, 2025
0cc31bb
use php-cs-fixer on php 7.4, but show the diff
fain182 Jul 20, 2025
84f991b
Revert style fixes to keep only build.yml and test changes
fain182 Jul 20, 2025
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
14 changes: 7 additions & 7 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:

- name: Coding Standard Checks
if: ${{ matrix.php-versions == '7.4' }}
run: PHP_CS_FIXER_IGNORE_ENV=1 ./bin/php-cs-fixer fix --dry-run -v
run: PHP_CS_FIXER_IGNORE_ENV=1 ./bin/php-cs-fixer fix --dry-run --diff

- name: Static Analysis
if: ${{ matrix.php-versions == '7.4' }}
Expand Down Expand Up @@ -84,7 +84,7 @@ jobs:

- name: "Rename phar to avoid conflicts"
run: mv ./phparkitect.phar phparkitect-${{ github.sha }}.phar

- name: "Upload phar file artifact"
uses: actions/upload-artifact@v4
with:
Expand Down Expand Up @@ -127,27 +127,27 @@ jobs:
with:
# will download phar in project root
name: phar-artifact

- name: Import GPG key
uses: crazy-max/ghaction-import-gpg@v6
with:
gpg_private_key: ${{ secrets.GPG_KEY_47CD54B6398FE21B3709D0A4D9C905CED1932CA2 }}
passphrase: ${{ secrets.GPG_KEY_47CD54B6398FE21B3709D0A4D9C905CED1932CA2_PASSPHRASE }}

- name: Rename phar
run: mv ./phparkitect-${{ github.sha }}.phar phparkitect.phar
run: mv ./phparkitect-${{ github.sha }}.phar phparkitect.phar

- name: Sign the PHAR
run: |
run: |
gpg --local-user 47CD54B6398FE21B3709D0A4D9C905CED1932CA2 \
--batch \
--yes \
--passphrase="${{ secrets.GPG_KEY_47CD54B6398FE21B3709D0A4D9C905CED1932CA2_PASSPHRASE }}" \
--detach-sign \
--output ./phparkitect.phar.asc \
./phparkitect.phar
- name: Add phar to the release

- name: Add phar to the release
uses: softprops/action-gh-release@v2
with:
token: ${{ secrets.GITHUB_TOKEN }}
Expand Down
159 changes: 53 additions & 106 deletions tests/Unit/Analyzer/FileParser/CanParseClassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

namespace Arkitect\Tests\Unit\Analyzer;
namespace Arkitect\Tests\Unit\Analyzer\FileParser;

use Arkitect\Analyzer\ClassDependency;
use Arkitect\Analyzer\ClassDescription;
Expand Down Expand Up @@ -38,17 +38,14 @@ public function __construct(Request $request)
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'path/to/class.php');

$violations = new Violations();
$classDescriptions = $this->parseCode($code);

$dependsOnTheseNamespaces = new DependsOnlyOnTheseNamespaces(['Foo']);
$dependsOnTheseNamespaces->evaluate($fp->getClassDescriptions()[0], $violations, 'because');
$violations = $this->evaluateRule($dependsOnTheseNamespaces, $classDescriptions[0]);

self::assertCount(2, $violations);
self::assertEquals('path/to/class.php', $violations->get(0)->getFilePath());
self::assertEquals('path/to/class.php', $violations->get(1)->getFilePath());
self::assertEquals('relativePathName', $violations->get(0)->getFilePath());
self::assertEquals('relativePathName', $violations->get(1)->getFilePath());
}

public function test_should_parse_instanceof(): void
Expand All @@ -69,9 +66,7 @@ public function bar($a, $b)
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code);

self::assertCount(1, $cd);
self::assertCount(1, $cd[0]->getDependencies());
Expand All @@ -97,9 +92,7 @@ class Cat implements AnInterface
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code);

self::assertCount(2, $cd);
self::assertInstanceOf(ClassDescription::class, $cd[0]);
Expand Down Expand Up @@ -141,9 +134,7 @@ class Cat implements AnInterface
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code);

self::assertCount(2, $cd);
self::assertInstanceOf(ClassDescription::class, $cd[0]);
Expand Down Expand Up @@ -183,9 +174,7 @@ class Cat implements AnInterface
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code);

self::assertCount(2, $cd);
self::assertInstanceOf(ClassDescription::class, $cd[0]);
Expand Down Expand Up @@ -217,10 +206,8 @@ class Cat extends Animal
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions()[1];
$cd = $this->parseCode($code);
$cd = $cd[1];

self::assertEquals('Root\Animals\Animal', $cd->getExtends()[0]->toString());
}
Expand All @@ -245,10 +232,8 @@ public function methodWithAnonymous(): void
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions()[1];
$cd = $this->parseCode($code);
$cd = $cd[1];

self::assertEquals('Root\Animals\Animal', $cd->getExtends()[0]->toString());
}
Expand All @@ -272,14 +257,10 @@ public function __construct(Request $request)
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$dependsOnTheseNamespaces = new DependsOnlyOnTheseNamespaces(['Foo', 'Symfony', 'Doctrine']);
$dependsOnTheseNamespaces->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($dependsOnTheseNamespaces, $cd[0]);

self::assertCount(0, $violations);
}
Expand Down Expand Up @@ -308,9 +289,7 @@ public function __construct(Request $request, ?Nullable $nullable)
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code);

$expectedDependencies = [
new ClassDependency('Foo\Baz\Baz', 10),
Expand Down Expand Up @@ -340,15 +319,10 @@ public function __construct()
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$dependsOnTheseNamespaces = new DependsOnlyOnTheseNamespaces(['Foo', 'Symfony', 'Doctrine']);
$dependsOnTheseNamespaces->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($dependsOnTheseNamespaces, $cd[0]);

self::assertCount(0, $violations);
}
Expand Down Expand Up @@ -379,15 +353,10 @@ public function doSomething(self $self, static $static)
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$notHaveDependencyOutsideNamespace = new NotHaveDependencyOutsideNamespace('Root\Animals');
$notHaveDependencyOutsideNamespace->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($notHaveDependencyOutsideNamespace, $cd[0]);

self::assertCount(0, $violations);
}
Expand All @@ -411,15 +380,10 @@ public function foo()
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$dependsOnlyOnTheseNamespaces = new DependsOnlyOnTheseNamespaces();
$dependsOnlyOnTheseNamespaces->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($dependsOnlyOnTheseNamespaces, $cd[0]);

self::assertCount(1, $violations);
}
Expand Down Expand Up @@ -452,15 +416,10 @@ public function getStatic(): self
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$notHaveDependencyOutsideNamespace = new NotHaveDependencyOutsideNamespace('Root\Cars');
$notHaveDependencyOutsideNamespace->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($notHaveDependencyOutsideNamespace, $cd[0]);

self::assertCount(1, $violations);
}
Expand All @@ -484,15 +443,11 @@ class Test implements Order

EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_1);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions()[2]; // class Test

$violations = new Violations();
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_1);
$cd = $cd[2]; // class Test

$implement = new Implement('Foo\Order');
$implement->evaluate($cd, $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($implement, $cd);

self::assertCount(0, $violations);
}
Expand All @@ -512,15 +467,10 @@ public function getBookList(): QueryBuilder;
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_1);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_1);

$dependsOnTheseNamespaces = new DependsOnlyOnTheseNamespaces(['MyProject\AppBundle\Application']);
$dependsOnTheseNamespaces->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($dependsOnTheseNamespaces, $cd[0]);

self::assertCount(1, $violations);
}
Expand Down Expand Up @@ -549,10 +499,7 @@ public function foobar();
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_1);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_1);

self::assertCount(3, $cd);
self::assertEquals('MyProject\AppBundle\Application\FooAble', $cd[2]->getExtends()[0]->toString());
Expand Down Expand Up @@ -583,14 +530,10 @@ public function getRequest(): Request //the violations is reported here
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');
$cd = $fp->getClassDescriptions();

$violations = new Violations();
$cd = $this->parseCode($code);

$dependsOnTheseNamespaces = new DependsOnlyOnTheseNamespaces(['Foo', 'Symfony', 'Doctrine']);
$dependsOnTheseNamespaces->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($dependsOnTheseNamespaces, $cd[0]);

self::assertCount(0, $violations);
}
Expand All @@ -608,13 +551,9 @@ public function __construct() {
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();
$violations = new Violations();
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_4);
$isFinal = new IsFinal();
$isFinal->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($isFinal, $cd[0]);

self::assertCount(0, $violations);
}
Expand All @@ -634,13 +573,9 @@ abstract public function foo() {}
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();
$violations = new Violations();
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_4);
$isAbstract = new IsAbstract();
$isAbstract->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$violations = $this->evaluateRule($isAbstract, $cd[0]);

self::assertCount(0, $violations);
}
Expand All @@ -658,14 +593,26 @@ public function __construct() {
}
EOF;

$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_4);
$cd = $this->parseCode($code, TargetPhpVersion::PHP_8_4);
$isReadOnly = new IsReadonly();
$violations = $this->evaluateRule($isReadOnly, $cd[0]);

self::assertCount(0, $violations);
}

private function parseCode(string $code, ?string $version = null): array
{
$fp = FileParserFactory::forPhpVersion($version ?? TargetPhpVersion::PHP_7_4);
$fp->parse($code, 'relativePathName');

$cd = $fp->getClassDescriptions();
return $fp->getClassDescriptions();
}

private function evaluateRule($rule, ClassDescription $classDescription): Violations
{
$violations = new Violations();
$isReadOnly = new IsReadonly();
$isReadOnly->evaluate($cd[0], $violations, 'we want to add this rule for our software');
$rule->evaluate($classDescription, $violations, 'we want to add this rule for our software');

self::assertCount(0, $violations);
return $violations;
}
}