Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like ZikulaPhpFileExtractor 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 ZikulaPhpFileExtractor, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 40 | class ZikulaPhpFileExtractor implements LoggerAwareInterface, FileVisitorInterface, NodeVisitor |
||
| 41 | { |
||
| 42 | /** |
||
| 43 | * @var string |
||
| 44 | */ |
||
| 45 | private $domain = ''; |
||
| 46 | |||
| 47 | /** |
||
| 48 | * @var NodeTraverser |
||
| 49 | */ |
||
| 50 | private $traverser; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * @var MessageCatalogue |
||
| 54 | */ |
||
| 55 | private $catalogue; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @var \SplFileInfo |
||
| 59 | */ |
||
| 60 | private $file; |
||
| 61 | |||
| 62 | /** |
||
| 63 | * @var DocParser |
||
| 64 | */ |
||
| 65 | private $docParser; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * @var LoggerInterface |
||
| 69 | */ |
||
| 70 | private $logger; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * @var Node |
||
| 74 | */ |
||
| 75 | private $previousNode; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * @var array |
||
| 79 | */ |
||
| 80 | private $bundles; |
||
| 81 | |||
| 82 | /** |
||
| 83 | * @var ZikulaHttpKernelInterface |
||
| 84 | */ |
||
| 85 | private $kernel; |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Possible Zikula-style translation method names |
||
| 89 | * |
||
| 90 | * @var array |
||
| 91 | */ |
||
| 92 | private $methodNames = [ |
||
| 93 | 1 => '__', |
||
| 94 | 2 => '__f', |
||
| 95 | 3 => '_n', |
||
| 96 | 4 => '_fn' |
||
| 97 | ]; |
||
| 98 | |||
| 99 | /** |
||
| 100 | * ZikulaPhpFileExtractor constructor. |
||
| 101 | * @param DocParser $docParser |
||
| 102 | * @param ZikulaHttpKernelInterface $kernel |
||
| 103 | */ |
||
| 104 | public function __construct(DocParser $docParser, ZikulaHttpKernelInterface $kernel) |
||
| 116 | |||
| 117 | /** |
||
| 118 | * @param LoggerInterface $logger |
||
| 119 | */ |
||
| 120 | public function setLogger(LoggerInterface $logger) |
||
| 124 | |||
| 125 | public function enterNode(Node $node) |
||
| 126 | { |
||
| 127 | /** |
||
| 128 | * determine domain from namespace of files. |
||
| 129 | * Finder appears to start with root level files so Namespace is correct for remaining files |
||
| 130 | */ |
||
| 131 | if ($node instanceof Namespace_) { |
||
|
1 ignored issue
–
show
|
|||
| 132 | if (isset($node->name)) { |
||
| 133 | $bundle = $this->getBundleFromNodeNamespace($node->name->toString()); |
||
| 134 | if (isset($bundle) && $bundle instanceof AbstractBundle) { |
||
| 135 | $this->domain = $bundle->getTranslationDomain(); |
||
| 136 | } else { |
||
| 137 | $this->domain = 'zikula'; |
||
| 138 | } |
||
| 139 | |||
| 140 | return; |
||
| 141 | } else { |
||
| 142 | foreach ($node->stmts as $node) { |
||
| 143 | $this->enterNode($node); |
||
| 144 | } |
||
| 145 | |||
| 146 | return; |
||
| 147 | } |
||
| 148 | } |
||
| 149 | if (!$node instanceof MethodCall |
||
|
1 ignored issue
–
show
|
|||
| 150 | || !is_string($node->name) |
||
| 151 | || !in_array(strtolower($node->name), $this->methodNames) |
||
| 152 | ) { |
||
| 153 | $this->previousNode = $node; |
||
| 154 | |||
| 155 | return; |
||
| 156 | } |
||
| 157 | |||
| 158 | $ignore = false; |
||
| 159 | $desc = $meaning = null; |
||
| 160 | if (null !== $docComment = $this->getDocCommentForNode($node)) { |
||
| 161 | foreach ($this->docParser->parse($docComment, 'file ' . $this->file . ' near line ' . $node->getLine()) as $annot) { |
||
| 162 | if ($annot instanceof Ignore) { |
||
|
1 ignored issue
–
show
|
|||
| 163 | $ignore = true; |
||
| 164 | } elseif ($annot instanceof Desc) { |
||
|
1 ignored issue
–
show
|
|||
| 165 | $desc = $annot->text; |
||
| 166 | } elseif ($annot instanceof Meaning) { |
||
|
1 ignored issue
–
show
|
|||
| 167 | $meaning = $annot->text; |
||
| 168 | } |
||
| 169 | } |
||
| 170 | } |
||
| 171 | |||
| 172 | View Code Duplication | if (!$node->args[0]->value instanceof String_) { |
|
|
1 ignored issue
–
show
|
|||
| 173 | if ($ignore) { |
||
| 174 | return; |
||
| 175 | } |
||
| 176 | |||
| 177 | $message = sprintf('Can only extract the translation id from a scalar string, but got "%s". Please refactor your code to make it extractable, or add the doc comment /** @Ignore */ to this code element (in %s on line %d).', get_class($node->args[0]->value), $this->file, $node->args[0]->value->getLine()); |
||
| 178 | |||
| 179 | if ($this->logger) { |
||
| 180 | $this->logger->error($message); |
||
| 181 | |||
| 182 | return; |
||
| 183 | } |
||
| 184 | |||
| 185 | throw new RuntimeException($message); |
||
| 186 | } |
||
| 187 | |||
| 188 | $id = $node->args[0]->value->value; |
||
| 189 | if (in_array(strtolower($node->name), ['_n', '_fn'], true)) { |
||
| 190 | // concatenate pluralized strings from zikula functions |
||
| 191 | $id = $node->args[0]->value->value . '|' . $node->args[1]->value->value; |
||
| 192 | } |
||
| 193 | |||
| 194 | // determine location of domain |
||
| 195 | $domainIndex = array_search(strtolower($node->name), $this->methodNames); |
||
| 196 | |||
| 197 | if (isset($node->args[$domainIndex])) { |
||
| 198 | View Code Duplication | if (!$node->args[$domainIndex]->value instanceof String_) { |
|
|
1 ignored issue
–
show
|
|||
| 199 | if ($ignore) { |
||
| 200 | return; |
||
| 201 | } |
||
| 202 | |||
| 203 | $message = sprintf('Can only extract the translation domain from a scalar string, but got "%s". Please refactor your code to make it extractable, or add the doc comment /** @Ignore */ to this code element (in %s on line %d).', get_class($node->args[0]->value), $this->file, $node->args[0]->value->getLine()); |
||
| 204 | |||
| 205 | if ($this->logger) { |
||
| 206 | $this->logger->error($message); |
||
| 207 | |||
| 208 | return; |
||
| 209 | } |
||
| 210 | |||
| 211 | throw new RuntimeException($message); |
||
| 212 | } |
||
| 213 | |||
| 214 | $domain = $node->args[$domainIndex]->value->value; |
||
| 215 | } else { |
||
| 216 | $domain = !empty($this->domain) ? $this->domain : 'zikula'; |
||
| 217 | } |
||
| 218 | |||
| 219 | $message = new Message($id, $domain); |
||
| 220 | $message->setDesc($desc); |
||
| 221 | $message->setMeaning($meaning); |
||
| 222 | $message->addSource(new FileSource((string)$this->file, $node->getLine())); |
||
| 223 | |||
| 224 | $this->catalogue->add($message); |
||
| 225 | } |
||
| 226 | |||
| 227 | public function visitPhpFile(\SplFileInfo $file, MessageCatalogue $catalogue, array $ast) |
||
| 233 | |||
| 234 | public function beforeTraverse(array $nodes) |
||
| 237 | |||
| 238 | public function leaveNode(Node $node) |
||
| 241 | |||
| 242 | public function afterTraverse(array $nodes) |
||
| 245 | |||
| 246 | public function visitFile(\SplFileInfo $file, MessageCatalogue $catalogue) |
||
| 249 | |||
| 250 | public function visitTwigFile(\SplFileInfo $file, MessageCatalogue $catalogue, \Twig_Node $ast) |
||
| 253 | |||
| 254 | private function getDocCommentForNode(Node $node) |
||
| 276 | |||
| 277 | /** |
||
| 278 | * Search namespaces for match and return BundleObject |
||
| 279 | * @param $nodeNamespace |
||
| 280 | * @return BundleInterface|null |
||
| 281 | */ |
||
| 282 | private function getBundleFromNodeNamespace($nodeNamespace) |
||
| 294 | } |
||
| 295 |
This error could be the result of:
1. Missing dependencies
PHP Analyzer uses your
composer.jsonfile (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects thecomposer.jsonto be in the root folder of your repository.Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the
requireorrequire-devsection?2. Missing use statement
PHP does not complain about undefined classes in
ìnstanceofchecks. For example, the following PHP code will work perfectly fine:If you have not tested against this specific condition, such errors might go unnoticed.