Skip to content

Commit 4e5c7ef

Browse files
author
Dan Cryer
committed
Adding method signature checking.
1 parent cc5c914 commit 4e5c7ef

File tree

7 files changed

+756
-123
lines changed

7 files changed

+756
-123
lines changed

‎PhpDocblockChecker/CheckerApplication.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace PhpDocblockChecker;
1111

1212
use Symfony\Component\Console\Application;
13+
use Symfony\Component\Console\Input\InputDefinition;
1314
use Symfony\Component\Console\Input\InputInterface;
1415

1516
/**
@@ -23,7 +24,7 @@ class CheckerApplication extends Application
2324
* @param InputInterface $input
2425
* @return string
2526
*/
26-
protected function getCommandName(InputInterface $input)
27+
protected function getCommandName(InputInterface $input) : string
2728
{
2829
return 'check';
2930
}

‎PhpDocblockChecker/CheckerCommand.php‎

Lines changed: 151 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
namespace PhpDocblockChecker;
1111

1212
use DirectoryIterator;
13-
use PHP_Token_Stream;
1413
use Symfony\Component\Console\Command\Command;
1514
use Symfony\Component\Console\Input\InputInterface;
1615
use Symfony\Component\Console\Input\InputOption;
@@ -35,12 +34,17 @@ class CheckerCommand extends Command
3534
/**
3635
* @var array
3736
*/
38-
protected $report = array();
37+
protected $errors = [];
3938

4039
/**
4140
* @var array
4241
*/
43-
protected $exclude = array();
42+
protected $warnings = [];
43+
44+
/**
45+
* @var array
46+
*/
47+
protected $exclude = [];
4448

4549
/**
4650
* @var bool
@@ -52,6 +56,11 @@ class CheckerCommand extends Command
5256
*/
5357
protected $skipMethods = false;
5458

59+
/**
60+
* @var bool
61+
*/
62+
protected $skipSignatures = false;
63+
5564
/**
5665
* @var OutputInterface
5766
*/
@@ -72,9 +81,10 @@ protected function configure()
7281
->addOption('directory', 'd', InputOption::VALUE_REQUIRED, 'Directory to scan.', './')
7382
->addOption('skip-classes', null, InputOption::VALUE_NONE, 'Don\'t check classes for docblocks.')
7483
->addOption('skip-methods', null, InputOption::VALUE_NONE, 'Don\'t check methods for docblocks.')
75-
->addOption('skip-anonymous-functions', null, InputOption::VALUE_NONE, 'Don\'t check anonymous functions for docblocks.')
84+
->addOption('skip-signatures', null, InputOption::VALUE_NONE, 'Don\'t check docblocks against method signatures.')
7685
->addOption('json', 'j', InputOption::VALUE_NONE, 'Output JSON instead of a log.')
7786
->addOption('files-per-line', 'l', InputOption::VALUE_REQUIRED, 'Number of files per line in progress', 50)
87+
->addOption('fail-on-warnings', 'w', InputOption::VALUE_NONE, 'Consider the check failed if any warnings are produced.')
7888
->addOption('info-only', 'i', InputOption::VALUE_NONE, 'Information-only mode, just show summary.');
7989
}
8090

@@ -94,6 +104,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
94104
$this->output = $output;
95105
$this->skipClasses = $input->getOption('skip-classes');
96106
$this->skipMethods = $input->getOption('skip-methods');
107+
$this->skipSignatures = $input->getOption('skip-signatures');
108+
$failOnWarnings = $input->getOption('fail-on-warnings');
109+
$startTime = microtime(true);
97110

98111
// Set up excludes:
99112
if (!is_null($exclude)) {
@@ -124,10 +137,14 @@ protected function execute(InputInterface $input, OutputInterface $output)
124137
$processed++;
125138
$file = array_shift($chunk);
126139

127-
if ($this->processFile($file)) {
128-
$this->output->write('<info>.</info>');
129-
} else {
140+
list($errors, $warnings) = $this->processFile($file);
141+
142+
if ($errors) {
130143
$this->output->write('<fg=red>F</>');
144+
} elseif ($warnings) {
145+
$this->output->write('<fg=yellow>W</>');
146+
} else {
147+
$this->output->write('<info>.</info>');
131148
}
132149
}
133150

@@ -136,46 +153,74 @@ protected function execute(InputInterface $input, OutputInterface $output)
136153
}
137154

138155
if ($this->verbose) {
156+
$time = round(microtime(true) - $startTime, 2);
139157
$this->output->writeln('');
140158
$this->output->writeln('');
141-
$this->output->writeln($totalFiles . ' Files Checked.');
142-
$this->output->writeln('<info>' . $this->passed . ' Passed</info> / <fg=red>'.count($this->report).' Errors</>');
159+
$this->output->writeln('Checked ' . number_format($totalFiles) . ' files in ' . $time . ' seconds.');
160+
$this->output->write('<info>' . number_format($this->passed) . ' Passed</info>');
161+
$this->output->write(' / <fg=red>'.number_format(count($this->errors)).' Errors</>');
162+
$this->output->write(' / <fg=yellow>'.number_format(count($this->warnings)).' Warnings</>');
163+
164+
$this->output->writeln('');
143165

144-
if (count($this->report) && !$input->getOption('info-only')) {
166+
if (count($this->errors) && !$input->getOption('info-only')) {
145167
$this->output->writeln('');
146168
$this->output->writeln('');
147169

148-
foreach ($this->report as $error) {
149-
$this->output->write('<error>' . $error['file'] . ':' . $error['line'] . '</error> - ');
170+
foreach ($this->errors as $error) {
171+
$this->output->write('<fg=red>ERROR </> ' . $error['file'] . ':' . $error['line'] . ' - ');
150172

151173
if ($error['type'] == 'class') {
152174
$this->output->write('Class <info>' . $error['class'] . '</info> is missing a docblock.');
153175
}
154176

155177
if ($error['type'] == 'method') {
156-
$this->output->write('Method <info>' . $error['class'] . '::' . $error['method'] . '</info> is missing a docblock.');
178+
$this->output->write('Method <info>' . $error['method'] . '</info> is missing a docblock.');
157179
}
158180

159181
$this->output->writeln('');
160182
}
161183
}
162184

163-
$this->output->writeln('');
164-
}
185+
if (count($this->warnings) && !$input->getOption('info-only')) {
186+
foreach ($this->warnings as $error) {
187+
$this->output->write('<fg=yellow>WARNING </> ');
188+
189+
if ($error['type'] == 'param-missing') {
190+
$this->output->write('<info>' . $error['method'] . '</info> - @param <fg=blue>'.$error['param'] . '</> missing.');
191+
}
165192

193+
if ($error['type'] == 'param-mismatch') {
194+
$this->output->write('<info>' . $error['method'] . '</info> - @param <fg=blue>'.$error['param'] . '</> ('.$error['doc-type'].') does not match method signature ('.$error['param-type'].').');
195+
}
196+
197+
if ($error['type'] == 'return-missing') {
198+
$this->output->write('<info>' . $error['method'] . '</info> - @return missing.');
199+
}
200+
201+
if ($error['type'] == 'return-mismatch') {
202+
$this->output->write('<info>' . $error['method'] . '</info> - @return <fg=blue>'.$error['doc-type'] . '</> does not match method signature ('.$error['return-type'].').');
203+
}
166204

205+
$this->output->writeln('');
206+
}
207+
}
208+
209+
$this->output->writeln('');
210+
}
167211

168212
// Output JSON if requested:
169213
if ($json) {
170-
print json_encode($this->report);
214+
print json_encode(array_merge($this->errors, $this->warnings));
171215
}
172216

173-
return count($this->report) ? 1 : 0;
217+
return count($this->errors) || ($failOnWarnings && count($this->warnings)) ? 1 : 0;
174218
}
175219

176220
/**
177221
* Iterate through a directory and check all of the PHP files within it.
178222
* @param string $path
223+
* @param string[] $worklist
179224
*/
180225
protected function processDirectory($path = '', array &$worklist = [])
181226
{
@@ -204,40 +249,104 @@ protected function processDirectory($path = '', array &$worklist = [])
204249

205250
/**
206251
* Check a specific PHP file for errors.
207-
* @param $file
208-
* @return bool
252+
* @param string $file
253+
* @return array
209254
*/
210255
protected function processFile($file)
211256
{
212257
$errors = false;
213-
$stream = new PHP_Token_Stream($this->basePath . $file);
214-
215-
foreach($stream->getClasses() as $name => $class) {
216-
if (!$this->skipClasses && is_null($class['docblock'])) {
217-
$errors = true;
218-
$this->report[] = array(
219-
'type' => 'class',
220-
'file' => $file,
221-
'class' => $name,
222-
'line' => $class['startLine'],
223-
);
258+
$warnings = false;
259+
$processor = new FileProcessor($this->basePath . $file);
260+
261+
if (!$this->skipClasses) {
262+
foreach ($processor->getClasses() as $name => $class) {
263+
if (is_null($class['docblock'])) {
264+
$errors = true;
265+
$this->errors[] = [
266+
'type' => 'class',
267+
'file' => $file,
268+
'class' => $name,
269+
'line' => $class['line'],
270+
];
271+
}
224272
}
273+
}
225274

226-
if (!$this->skipMethods) {
227-
foreach ($class['methods'] as $methodName => $method) {
228-
if ($methodName == 'anonymous function') {
229-
continue;
275+
if (!$this->skipMethods) {
276+
foreach ($processor->getMethods() as $name => $method) {
277+
if (is_null($method['docblock'])) {
278+
$errors = true;
279+
$this->errors[] = [
280+
'type' => 'method',
281+
'file' => $file,
282+
'class' => $name,
283+
'method' => $name,
284+
'line' => $method['line'],
285+
];
286+
}
287+
}
288+
}
289+
290+
if (!$this->skipSignatures) {
291+
foreach ($processor->getMethods() as $name => $method) {
292+
if (count($method['params'])) {
293+
foreach ($method['params'] as $param => $type) {
294+
if (!isset($method['docblock']['params'][$param])) {
295+
$warnings = true;
296+
$this->warnings[] = [
297+
'type' => 'param-missing',
298+
'file' => $file,
299+
'class' => $name,
300+
'method' => $name,
301+
'line' => $method['line'],
302+
'param' => $param,
303+
];
304+
} elseif (!empty($type) && $method['docblock']['params'][$param] != $type) {
305+
if ($type == 'array' && substr($method['docblock']['params'][$param], -2) == '[]') {
306+
// Do nothing because this is fine.
307+
} else {
308+
$warnings = true;
309+
$this->warnings[] = [
310+
'type' => 'param-mismatch',
311+
'file' => $file,
312+
'class' => $name,
313+
'method' => $name,
314+
'line' => $method['line'],
315+
'param' => $param,
316+
'param-type' => $type,
317+
'doc-type' => $method['docblock']['params'][$param],
318+
];
319+
}
320+
}
230321
}
322+
}
323+
231324

232-
if (is_null($method['docblock'])) {
233-
$errors = true;
234-
$this->report[] = array(
235-
'type' => 'method',
325+
if (!empty($method['return'])) {
326+
if (empty($method['docblock']['return'])) {
327+
$warnings = true;
328+
$this->warnings[] = [
329+
'type' => 'return-missing',
236330
'file' => $file,
237331
'class' => $name,
238-
'method' => $methodName,
239-
'line' => $method['startLine'],
240-
);
332+
'method' => $name,
333+
'line' => $method['line'],
334+
];
335+
} elseif ($method['docblock']['return'] != $method['return']) {
336+
if ($method['return'] == 'array' && substr($method['docblock']['return'], -2) == '[]') {
337+
// Do nothing because this is fine.
338+
} else {
339+
$warnings = true;
340+
$this->warnings[] = [
341+
'type' => 'return-mismatch',
342+
'file' => $file,
343+
'class' => $name,
344+
'method' => $name,
345+
'line' => $method['line'],
346+
'return-type' => $method['return'],
347+
'doc-type' => $method['docblock']['return'],
348+
];
349+
}
241350
}
242351
}
243352
}
@@ -247,6 +356,6 @@ protected function processFile($file)
247356
$this->passed += 1;
248357
}
249358

250-
return !$errors;
359+
return [$errors, $warnings];
251360
}
252361
}

0 commit comments

Comments
 (0)