Skip to content

Commit 0903322

Browse files
micheleorsellimicheleorselli
authored
Check Command: cleanup output messages and make them more consistent (#486)
1 parent d3f15d3 commit 0903322

File tree

7 files changed

+93
-64
lines changed

7 files changed

+93
-64
lines changed

‎src/Analyzer/FileParser.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@
88
use Arkitect\Rules\ParsingError;
99
use PhpParser\ErrorHandler\Collecting;
1010
use PhpParser\NodeTraverser;
11+
use PhpParser\Parser as PhpParser;
1112
use PhpParser\ParserFactory;
1213
use PhpParser\PhpVersion;
1314

1415
class FileParser implements Parser
1516
{
16-
private \PhpParser\Parser $parser;
17+
private PhpParser $parser;
1718

1819
private NodeTraverser $traverser;
1920

@@ -31,7 +32,7 @@ public function __construct(
3132
$this->fileVisitor = $fileVisitor;
3233
$this->parsingErrors = [];
3334

34-
$this->parser = (new ParserFactory())->createForVersion(PhpVersion::fromString($targetPhpVersion->get() ?? phpversion()));
35+
$this->parser = (new ParserFactory())->createForVersion(PhpVersion::fromString($targetPhpVersion->get()));
3536
$this->traverser = $traverser;
3637
$this->traverser->addVisitor($nameResolver);
3738
$this->traverser->addVisitor($this->fileVisitor);

‎src/CLI/AnalysisResult.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Arkitect\CLI;
5+
6+
use Arkitect\Rules\ParsingErrors;
7+
use Arkitect\Rules\Violations;
8+
9+
class AnalysisResult
10+
{
11+
private Violations $violations;
12+
13+
private ParsingErrors $parsingErrors;
14+
15+
public function __construct(Violations $violations, ParsingErrors $parsingErrors)
16+
{
17+
$this->violations = $violations;
18+
$this->parsingErrors = $parsingErrors;
19+
}
20+
21+
public function getViolations(): Violations
22+
{
23+
return $this->violations;
24+
}
25+
26+
public function getParsingErrors(): ParsingErrors
27+
{
28+
return $this->parsingErrors;
29+
}
30+
31+
public function hasErrors(): bool
32+
{
33+
return $this->hasViolations() || $this->hasParsingErrors();
34+
}
35+
36+
public function hasViolations(): bool
37+
{
38+
return $this->violations->count() > 0;
39+
}
40+
41+
public function hasParsingErrors(): bool
42+
{
43+
return $this->parsingErrors->count() > 0;
44+
}
45+
}

‎src/CLI/Command/Check.php

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Arkitect\CLI\Command;
66

77
use Arkitect\CLI\Config;
8+
use Arkitect\CLI\Printer\PrinterFactory;
89
use Arkitect\CLI\Progress\DebugProgress;
910
use Arkitect\CLI\Progress\ProgressBarProgress;
1011
use Arkitect\CLI\Runner;
@@ -109,6 +110,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
109110
$useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM);
110111
$skipBaseline = (bool) $input->getOption(self::SKIP_BASELINE_PARAM);
111112
$ignoreBaselineLinenumbers = (bool) $input->getOption(self::IGNORE_BASELINE_LINENUMBERS_PARAM);
113+
$generateBaseline = $input->getOption(self::GENERATE_BASELINE_PARAM);
112114
$phpVersion = $input->getOption('target-php-version');
113115
$format = $input->getOption(self::FORMAT_PARAM);
114116

@@ -117,49 +119,44 @@ protected function execute(InputInterface $input, OutputInterface $output): int
117119
$stdOut = $output;
118120
$output = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
119121

120-
/** @var string|null $phpVersion */
121122
$targetPhpVersion = TargetPhpVersion::create($phpVersion);
122123

123124
$progress = $verbose ? new DebugProgress($output) : new ProgressBarProgress($output);
124125

126+
$this->printHeadingLine($output);
127+
125128
if (true !== $skipBaseline && !$useBaseline && file_exists(self::DEFAULT_BASELINE_FILENAME)) {
126129
$useBaseline = self::DEFAULT_BASELINE_FILENAME;
127130
}
128131

129132
if ($useBaseline && !file_exists($useBaseline)) {
130-
$output->writeln('<error>Baseline file not found.</error>');
133+
$output->writeln("❌ Baseline file '$useBaseline' not found.");
131134

132135
return self::ERROR_CODE;
133136
}
134137

135-
$output->writeln('<info>Baseline found: '.$useBaseline.'</info>');
136-
137-
$generateBaseline = $input->getOption(self::GENERATE_BASELINE_PARAM);
138-
139-
$this->printHeadingLine($output);
138+
$output->writeln("Baseline file '$useBaseline' found");
140139

141140
$rulesFilename = $this->getConfigFilename($input);
142141

143-
$output->writeln(\sprintf("Config file: %s\n", $rulesFilename));
142+
$output->writeln("Config file '$rulesFilename' found\n");
144143

145144
$config = new Config();
146145

147146
$this->readRules($config, $rulesFilename);
148147

149148
$runner = new Runner($stopOnFailure);
150-
$runner->run($config, $progress, $targetPhpVersion);
149+
$result = $runner->run($config, $progress, $targetPhpVersion);
151150

152-
$violations = $runner->getViolations();
153-
$violations->sort();
151+
$violations = $result->getViolations();
154152

155153
if (false !== $generateBaseline) {
156154
if (null === $generateBaseline) {
157155
$generateBaseline = self::DEFAULT_BASELINE_FILENAME;
158156
}
159157
$this->saveBaseline($generateBaseline, $violations);
160158

161-
$output->writeln('<info>Baseline file \''.$generateBaseline.'\'created!</info>');
162-
$this->printExecutionTime($output, $startTime);
159+
$output->writeln("ℹ️ Baseline file '$generateBaseline' created!");
163160

164161
return self::SUCCESS_CODE;
165162
}
@@ -170,37 +167,30 @@ protected function execute(InputInterface $input, OutputInterface $output): int
170167
$violations->remove($baseline, $ignoreBaselineLinenumbers);
171168
}
172169

170+
$printer = (new PrinterFactory())->create($format);
171+
173172
// we always print this so we do not have to do additional ifs later
174-
$stdOut->writeln($violations->toString($format));
173+
$stdOut->writeln($printer->print($violations->groupedByFqcn()));
175174

176175
if ($violations->count() > 0) {
177-
$output->writeln(\sprintf('<error>⚠️ %s violations detected!</error>', \count($violations)));
178-
$this->printExecutionTime($output, $startTime);
179-
180-
return self::ERROR_CODE;
176+
$output->writeln(\sprintf('⚠️ %s violations detected!', \count($violations)));
181177
}
182178

183-
$parsedErrors = $runner->getParsingErrors();
179+
if ($result->hasParsingErrors()) {
180+
$output->writeln('❌ could not parse these files:');
181+
$output->writeln($result->getParsingErrors()->toString());
182+
}
184183

185-
if ($parsedErrors->count() > 0) {
186-
$output->writeln('<error>❌ could not parse these files:</error>');
187-
$output->writeln($parsedErrors->toString());
188-
$this->printExecutionTime($output, $startTime);
184+
!$result->hasErrors() && $output->writeln('✅ No violations detected');
189185

190-
return self::ERROR_CODE;
191-
}
186+
return $result->hasErrors() ? self::ERROR_CODE : self::SUCCESS_CODE;
192187
} catch (\Throwable $e) {
193-
$output->writeln($e->getMessage());
194-
$this->printExecutionTime($output, $startTime);
188+
$output->writeln("❌ {$e->getMessage()}");
195189

196190
return self::ERROR_CODE;
191+
} finally {
192+
$this->printExecutionTime($output, $startTime);
197193
}
198-
199-
$output->writeln('<info>✅ No violations detected</info>');
200-
201-
$this->printExecutionTime($output, $startTime);
202-
203-
return self::SUCCESS_CODE;
204194
}
205195

206196
protected function readRules(Config $ruleChecker, string $rulesFilename): void
@@ -229,7 +219,7 @@ protected function printExecutionTime(OutputInterface $output, float $startTime)
229219
$endTime = microtime(true);
230220
$executionTime = number_format($endTime - $startTime, 2);
231221

232-
$output->writeln('<info>Execution time: '.$executionTime."s</info>\n");
222+
$output->writeln("⏱️ Execution time: $executionTime\n");
233223
}
234224

235225
private function loadBaseline(string $filename): Violations
@@ -250,7 +240,7 @@ private function getConfigFilename(InputInterface $input): string
250240
$filename = self::DEFAULT_RULES_FILENAME;
251241
}
252242

253-
Assert::file($filename, 'Config file not found');
243+
Assert::file($filename, "Config file '$filename' not found");
254244

255245
return $filename;
256246
}

‎src/CLI/Config.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99

1010
class Config
1111
{
12-
/** @var array */
13-
private $classSetRules;
14-
/** @var bool */
15-
private $runOnlyARule;
16-
/** @var bool */
17-
private $parseCustomAnnotations;
12+
/** @var array<ClassSetRules> */
13+
private array $classSetRules;
14+
15+
private bool $runOnlyARule;
16+
17+
private bool $parseCustomAnnotations;
1818

1919
public function __construct()
2020
{

‎src/CLI/Runner.php

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
namespace Arkitect\CLI;
66

77
use Arkitect\Analyzer\ClassDescription;
8-
use Arkitect\Analyzer\FileParser;
98
use Arkitect\Analyzer\FileParserFactory;
109
use Arkitect\Analyzer\Parser;
1110
use Arkitect\ClassSetRules;
@@ -30,9 +29,8 @@ public function __construct(bool $stopOnFailure = false)
3029
$this->parsingErrors = new ParsingErrors();
3130
}
3231

33-
public function run(Config $config, Progress $progress, TargetPhpVersion $targetPhpVersion): void
32+
public function run(Config $config, Progress $progress, TargetPhpVersion $targetPhpVersion): AnalysisResult
3433
{
35-
/** @var FileParser $fileParser */
3634
$fileParser = FileParserFactory::createFileParser($targetPhpVersion, $config->isParseCustomAnnotationsEnabled());
3735

3836
/** @var ClassSetRules $classSetRule */
@@ -42,11 +40,18 @@ public function run(Config $config, Progress $progress, TargetPhpVersion $target
4240
try {
4341
$this->check($classSetRule, $progress, $fileParser, $this->violations, $this->parsingErrors);
4442
} catch (FailOnFirstViolationException $e) {
45-
return;
43+
break;
44+
} finally {
45+
$progress->endFileSetAnalysis($classSetRule->getClassSet());
4646
}
47-
48-
$progress->endFileSetAnalysis($classSetRule->getClassSet());
4947
}
48+
49+
$this->violations->sort();
50+
51+
return new AnalysisResult(
52+
$this->violations,
53+
$this->parsingErrors,
54+
);
5055
}
5156

5257
public function check(
@@ -87,14 +92,4 @@ public function check(
8792
$progress->endParsingFile($file->getRelativePathname());
8893
}
8994
}
90-
91-
public function getViolations(): Violations
92-
{
93-
return $this->violations;
94-
}
95-
96-
public function getParsingErrors(): ParsingErrors
97-
{
98-
return $this->parsingErrors;
99-
}
10095
}

‎src/CLI/TargetPhpVersion.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ class TargetPhpVersion
1717
'8.4',
1818
];
1919

20-
/** @var string|null */
21-
private $version;
20+
private string $version;
2221

2322
private function __construct(string $version)
2423
{
@@ -39,7 +38,7 @@ public static function create(?string $version): self
3938
return new self($version ?? phpversion());
4039
}
4140

42-
public function get(): ?string
41+
public function get(): string
4342
{
4443
return $this->version;
4544
}

‎tests/Unit/Analyzer/FileVisitorTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use Arkitect\Analyzer\FileParser;
1010
use Arkitect\Analyzer\FileParserFactory;
1111
use Arkitect\Analyzer\FullyQualifiedClassName;
12-
use Arkitect\CLI\Printer\Printer;
1312
use Arkitect\CLI\TargetPhpVersion;
1413
use Arkitect\Expression\ForClasses\DependsOnlyOnTheseNamespaces;
1514
use Arkitect\Expression\ForClasses\Implement;
@@ -865,7 +864,7 @@ class Test implements Order
865864
$implement = new Implement('Foo\Order');
866865
$implement->evaluate($cd, $violations, 'we want to add this rule for our software');
867866

868-
self::assertCount(0, $violations, $violations->toString(Printer::FORMAT_TEXT));
867+
self::assertCount(0, $violations);
869868
}
870869

871870
public function test_it_parse_dependencies_in_docblocks_with_alias(): void

0 commit comments

Comments
 (0)