phpmd /
phpmd
Complex classes like AbstractTest often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use AbstractTest, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 46 | abstract class AbstractTest extends AbstractStaticTest |
||
| 47 | { |
||
| 48 | /** @var int At least one violation is expected */ |
||
| 49 | const AL_LEAST_ONE_VIOLATION = -1; |
||
| 50 | |||
| 51 | /** @var int No violation is expected */ |
||
| 52 | const NO_VIOLATION = 0; |
||
| 53 | |||
| 54 | /** @var int One violation is expected */ |
||
| 55 | const ONE_VIOLATION = 1; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * Get a list of files that should trigger a rule violation. |
||
| 59 | * |
||
| 60 | * By default, files named like "testRuleAppliesTo*", but it can be overridden in sub-classes. |
||
| 61 | * |
||
| 62 | * @return string[] |
||
| 63 | */ |
||
| 64 | public function getApplyingFiles() |
||
| 65 | { |
||
| 66 | return $this->getFilesForCalledClass('testRuleAppliesTo*'); |
||
| 67 | } |
||
| 68 | |||
| 69 | /** |
||
| 70 | * Get a list of files that should not trigger a rule violation. |
||
| 71 | * |
||
| 72 | * By default, files named like "testRuleDoesNotApplyTo*", but it can be overridden in sub-classes. |
||
| 73 | * |
||
| 74 | * @return string[] |
||
| 75 | */ |
||
| 76 | public function getNotApplyingFiles() |
||
| 77 | { |
||
| 78 | return $this->getFilesForCalledClass('testRuleDoesNotApplyTo*'); |
||
| 79 | } |
||
| 80 | |||
| 81 | /** |
||
| 82 | * Get a list of test files specified by getApplyingFiles() as an array of 1-length arguments lists. |
||
| 83 | * |
||
| 84 | * @return string[][] |
||
| 85 | */ |
||
| 86 | public function getApplyingCases() |
||
| 87 | { |
||
| 88 | return static::getValuesAsArrays($this->getApplyingFiles()); |
||
| 89 | } |
||
| 90 | |||
| 91 | /** |
||
| 92 | * Get a list of test files specified by getNotApplyingFiles() as an array of 1-length arguments lists. |
||
| 93 | * |
||
| 94 | * @return string[][] |
||
| 95 | */ |
||
| 96 | public function getNotApplyingCases() |
||
| 97 | { |
||
| 98 | return static::getValuesAsArrays($this->getNotApplyingFiles()); |
||
| 99 | } |
||
| 100 | |||
| 101 | /** |
||
| 102 | * Resets a changed working directory. |
||
| 103 | * |
||
| 104 | * @return void |
||
| 105 | */ |
||
| 106 | protected function tearDown() |
||
| 107 | { |
||
| 108 | static::returnToOriginalWorkingDirectory(); |
||
| 109 | static::cleanupTempFiles(); |
||
| 110 | |||
| 111 | parent::tearDown(); |
||
| 112 | } |
||
| 113 | |||
| 114 | /** |
||
| 115 | * Returns the first class found in a source file related to the calling |
||
| 116 | * test method. |
||
| 117 | * |
||
| 118 | * @return ClassNode |
||
| 119 | */ |
||
| 120 | protected function getClass() |
||
| 121 | { |
||
| 122 | return new ClassNode( |
||
| 123 | $this->getNodeForCallingTestCase( |
||
| 124 | $this->parseTestCaseSource()->getClasses() |
||
| 125 | ) |
||
| 126 | ); |
||
| 127 | } |
||
| 128 | |||
| 129 | /** |
||
| 130 | * Returns the first interface found in a source file related to the calling |
||
| 131 | * test method. |
||
| 132 | * |
||
| 133 | * @return InterfaceNode |
||
| 134 | */ |
||
| 135 | protected function getInterface() |
||
| 136 | { |
||
| 137 | return new InterfaceNode( |
||
| 138 | $this->getNodeForCallingTestCase( |
||
| 139 | $this->parseTestCaseSource()->getInterfaces() |
||
| 140 | ) |
||
| 141 | ); |
||
| 142 | } |
||
| 143 | |||
| 144 | /** |
||
| 145 | * @return TraitNode |
||
| 146 | */ |
||
| 147 | protected function getTrait() |
||
| 148 | { |
||
| 149 | return new TraitNode( |
||
| 150 | $this->getNodeForCallingTestCase( |
||
| 151 | $this->parseTestCaseSource()->getTraits() |
||
| 152 | ) |
||
| 153 | ); |
||
| 154 | } |
||
| 155 | |||
| 156 | /** |
||
| 157 | * Returns the first method found in a source file related to the calling |
||
| 158 | * test method. |
||
| 159 | * |
||
| 160 | * @return MethodNode |
||
| 161 | */ |
||
| 162 | protected function getMethod() |
||
| 163 | { |
||
| 164 | return new MethodNode( |
||
| 165 | $this->getNodeForCallingTestCase( |
||
| 166 | $this->parseTestCaseSource() |
||
| 167 | ->getTypes() |
||
| 168 | ->current() |
||
| 169 | ->getMethods() |
||
| 170 | ) |
||
| 171 | ); |
||
| 172 | } |
||
| 173 | |||
| 174 | /** |
||
| 175 | * Returns the first function found in a source files related to the calling |
||
| 176 | * test method. |
||
| 177 | * |
||
| 178 | * @return FunctionNode |
||
| 179 | */ |
||
| 180 | protected function getFunction() |
||
| 181 | { |
||
| 182 | return new FunctionNode( |
||
| 183 | $this->getNodeForCallingTestCase( |
||
| 184 | $this->parseTestCaseSource()->getFunctions() |
||
| 185 | ) |
||
| 186 | ); |
||
| 187 | } |
||
| 188 | |||
| 189 | /** |
||
| 190 | * Returns the first class found for a given test file. |
||
| 191 | * |
||
| 192 | * @return ClassNode |
||
| 193 | */ |
||
| 194 | protected function getClassNodeForTestFile($file) |
||
| 202 | |||
| 203 | /** |
||
| 204 | * Returns the first method or function node for a given test file. |
||
| 205 | * |
||
| 206 | * @param string $file |
||
| 207 | * @return MethodNode|FunctionNode |
||
| 208 | * @since 2.8.3 |
||
| 209 | */ |
||
| 210 | protected function getNodeForTestFile($file) |
||
| 211 | { |
||
| 212 | $source = $this->parseSource($file); |
||
| 232 | |||
| 233 | /** |
||
| 234 | * Assert that a given file trigger N times the given rule. |
||
| 235 | * |
||
| 236 | * Rethrows the PHPUnit ExpectationFailedException with the base name |
||
| 237 | * of the file for better readability. |
||
| 238 | * |
||
| 239 | * @param Rule $rule Rule to test. |
||
| 240 | * @param int $expectedInvokes Count of expected invocations. |
||
| 241 | * @param string $file Test file containing a method with the same name to be tested. |
||
| 242 | * @return void |
||
| 243 | * @throws PHPUnit_Framework_ExpectationFailedException |
||
| 244 | */ |
||
| 245 | protected function expectRuleHasViolationsForFile(Rule $rule, $expectedInvokes, $file) |
||
| 264 | |||
| 265 | /** |
||
| 266 | * Return a human-friendly failure message for a given list of violations and the actual/expected counts. |
||
| 267 | * |
||
| 268 | * @param string $file |
||
| 269 | * @param int $expectedInvokes |
||
| 270 | * @param int $actualInvokes |
||
| 271 | * @param array|iterable|Traversable $violations |
||
| 272 | * |
||
| 273 | * @return string |
||
| 274 | */ |
||
| 275 | protected function getViolationFailureMessage($file, $expectedInvokes, $actualInvokes, $violations) |
||
| 285 | |||
| 286 | /** |
||
| 287 | * Return a human-friendly summary for a list of violations. |
||
| 288 | * |
||
| 289 | * @param array|iterable|Traversable $violations |
||
| 290 | * @return string |
||
| 291 | */ |
||
| 292 | protected function getViolationsSummary($violations) |
||
| 313 | |||
| 314 | /** |
||
| 315 | * Returns the absolute path for a test resource for the current test. |
||
| 316 | * |
||
| 317 | * @return string |
||
| 318 | * @since 1.1.0 |
||
| 319 | */ |
||
| 320 | protected static function createCodeResourceUriForTest() |
||
| 326 | |||
| 327 | /** |
||
| 328 | * Returns the absolute path for a test resource for the current test. |
||
| 329 | * |
||
| 330 | * @param string $localPath The local/relative file location |
||
| 331 | * @return string |
||
| 332 | * @since 1.1.0 |
||
| 333 | */ |
||
| 334 | protected static function createResourceUriForTest($localPath) |
||
| 340 | |||
| 341 | /** |
||
| 342 | * Return URI for a given pattern with directory based on the current called class name. |
||
| 343 | * |
||
| 344 | * @param string $pattern |
||
| 345 | * @return string |
||
| 346 | */ |
||
| 347 | protected function createResourceUriForCalledClass($pattern) |
||
| 351 | |||
| 352 | /** |
||
| 353 | * Return list of files matching a given pattern with directory based on the current called class name. |
||
| 354 | * |
||
| 355 | * @param string $pattern |
||
| 356 | * @return string[] |
||
| 357 | */ |
||
| 358 | protected function getFilesForCalledClass($pattern = '*') |
||
| 362 | |||
| 363 | /** |
||
| 364 | * Creates a mocked class node instance. |
||
| 365 | * |
||
| 366 | * @param string $metric |
||
| 367 | * @param integer $value |
||
| 368 | * @return ClassNode |
||
| 369 | */ |
||
| 370 | protected function getClassMock($metric = null, $value = null) |
||
| 386 | |||
| 387 | /** |
||
| 388 | * Creates a mocked method node instance. |
||
| 389 | * |
||
| 390 | * @param string $metric |
||
| 391 | * @param integer $value |
||
| 392 | * @return MethodNode |
||
| 393 | */ |
||
| 394 | protected function getMethodMock($metric = null, $value = null) |
||
| 398 | |||
| 399 | /** |
||
| 400 | * Creates a mocked function node instance. |
||
| 401 | * |
||
| 402 | * @param string $metric The metric acronym used by PHP_Depend. |
||
| 403 | * @param mixed $value The expected metric return value. |
||
| 404 | * @return FunctionNode |
||
| 405 | */ |
||
| 406 | protected function createFunctionMock($metric = null, $value = null) |
||
| 415 | |||
| 416 | /** |
||
| 417 | * Initializes the getMetric() method of the given function or method node. |
||
| 418 | * |
||
| 419 | * @param FunctionNode|MethodNode|PHPUnit_Framework_MockObject_MockObject $mock |
||
| 420 | * @param string $metric |
||
| 421 | * @param mixed $value |
||
| 422 | * @return FunctionNode|MethodNode |
||
| 423 | */ |
||
| 424 | protected function initFunctionOrMethod($mock, $metric, $value) |
||
| 437 | |||
| 438 | /** |
||
| 439 | * Creates a mocked report instance. |
||
| 440 | * |
||
| 441 | * @param integer $expectedInvokes Number of expected invokes. |
||
| 442 | * @return Report|PHPUnit_Framework_MockObject_MockObject |
||
| 443 | */ |
||
| 444 | protected function getReportMock($expectedInvokes = -1) |
||
| 462 | |||
| 463 | /** |
||
| 464 | * Get a mocked report with one violation |
||
| 465 | * |
||
| 466 | * @return Report |
||
| 467 | */ |
||
| 468 | public function getReportWithOneViolation() |
||
| 472 | |||
| 473 | /** |
||
| 474 | * Get a mocked report with no violation |
||
| 475 | * |
||
| 476 | * @return Report |
||
| 477 | */ |
||
| 478 | public function getReportWithNoViolation() |
||
| 482 | |||
| 483 | /** |
||
| 484 | * Get a mocked report with at least one violation |
||
| 485 | * |
||
| 486 | * @return Report |
||
| 487 | */ |
||
| 488 | public function getReportWithAtLeastOneViolation() |
||
| 492 | |||
| 493 | protected function getMockFromBuilder(PHPUnit_Framework_MockObject_MockBuilder $builder) |
||
| 501 | |||
| 502 | /** |
||
| 503 | * Creates a mocked {@link \PHPMD\AbstractRule} instance. |
||
| 504 | * |
||
| 505 | * @return AbstractRule|PHPUnit_Framework_MockObject_MockObject |
||
| 506 | */ |
||
| 507 | protected function getRuleMock() |
||
| 515 | |||
| 516 | /** |
||
| 517 | * Creates a mocked rule-set instance. |
||
| 518 | * |
||
| 519 | * @param string $expectedClass Optional class name for apply() expected at least once. |
||
| 520 | * @param int|string $count How often should apply() be called? |
||
| 521 | * @return RuleSet|PHPUnit_Framework_MockObject_MockObject |
||
| 522 | */ |
||
| 523 | protected function getRuleSetMock($expectedClass = null, $count = '*') |
||
| 544 | |||
| 545 | /** |
||
| 546 | * Creates a mocked rule violation instance. |
||
| 547 | * |
||
| 548 | * @param string $fileName The filename to use. |
||
| 549 | * @param integer $beginLine The begin of violation line number to use. |
||
| 550 | * @param integer $endLine The end of violation line number to use. |
||
| 551 | * @param null|object $rule The rule object to use. |
||
| 552 | * @param null|string $description The violation description to use. |
||
| 553 | * @return PHPUnit_Framework_MockObject_MockObject |
||
| 554 | */ |
||
| 555 | protected function getRuleViolationMock( |
||
| 596 | |||
| 597 | /** |
||
| 598 | * Creates a mocked rul violation instance. |
||
| 599 | * |
||
| 600 | * @param string $file |
||
| 601 | * @param string $message |
||
| 602 | * @return ProcessingError|PHPUnit_Framework_MockObject_MockObject |
||
| 603 | */ |
||
| 604 | protected function getErrorMock( |
||
| 624 | |||
| 625 | /** |
||
| 626 | * Parses the source code for the calling test method and returns the first |
||
| 627 | * package node found in the parsed file. |
||
| 628 | * |
||
| 629 | * @return ASTNamespace |
||
| 630 | */ |
||
| 631 | private function parseTestCaseSource() |
||
| 635 | |||
| 636 | /** |
||
| 637 | * @param string $mockBuilder |
||
| 638 | * @param ASTFunction|ASTMethod $mock |
||
| 639 | * @param string $metric The metric acronym used by PHP_Depend. |
||
| 640 | * @param mixed $value The expected metric return value. |
||
| 641 | * @return FunctionNode|MethodNode |
||
| 642 | */ |
||
| 643 | private function createFunctionOrMethodMock($mockBuilder, $mock, $metric = null, $value = null) |
||
| 654 | |||
| 655 | /** |
||
| 656 | * Returns the PHP_Depend node having the given name. |
||
| 657 | * |
||
| 658 | * @param Iterator $nodes |
||
| 659 | * @return PHP_Depend_Code_AbstractItem |
||
| 660 | * @throws ErrorException |
||
| 661 | */ |
||
| 662 | private function getNodeByName(Iterator $nodes, $name) |
||
| 671 | |||
| 672 | /** |
||
| 673 | * Returns the PHP_Depend node for the calling test case. |
||
| 674 | * |
||
| 675 | * @param Iterator $nodes |
||
| 676 | * @return PHP_Depend_Code_AbstractItem |
||
| 677 | * @throws ErrorException |
||
| 678 | */ |
||
| 679 | private function getNodeForCallingTestCase(Iterator $nodes) |
||
| 685 | |||
| 686 | /** |
||
| 687 | * Parses the source of the given file and returns the first package found |
||
| 688 | * in that file. |
||
| 689 | * |
||
| 690 | * @param string $sourceFile |
||
| 691 | * @return ASTNamespace |
||
| 692 | * @throws ErrorException |
||
| 693 | */ |
||
| 694 | private function parseSource($sourceFile) |
||
| 714 | } |
||
| 715 |
Loading history...
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.