lechimp-p /
dicto.php
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:
| 1 | <?php |
||
| 25 | class ContainText extends Schema { |
||
| 26 | /** |
||
| 27 | * @inheritdoc |
||
| 28 | */ |
||
| 29 | 52 | public function name() { |
|
| 32 | |||
| 33 | /** |
||
| 34 | * @inheritdoc |
||
| 35 | */ |
||
| 36 | 7 | public function fetch_arguments(ArgumentParser $parser) { |
|
| 40 | |||
| 41 | /** |
||
| 42 | * @inheritdoc |
||
| 43 | */ |
||
| 44 | 20 | public function arguments_are_valid(array &$arguments) { |
|
| 45 | 20 | if (count($arguments) != 1 || !($arguments[0] instanceof Regexp)) { |
|
| 46 | return false; |
||
| 47 | } |
||
| 48 | 20 | return true; |
|
| 49 | } |
||
| 50 | |||
| 51 | /** |
||
| 52 | * @inheritdoc |
||
| 53 | */ |
||
| 54 | 12 | public function compile(Index $index, Rule $rule) { |
|
| 55 | 12 | $mode = $rule->mode(); |
|
| 56 | 12 | $pred_factory = $index->query()->predicate_factory(); |
|
| 57 | 12 | $filter = $rule->checked_on()->compile($pred_factory); |
|
| 58 | // ContainText needs to have to kinds of queries, one for files, one |
||
| 59 | // for none-files. To make the queries faster, we separate the filters |
||
| 60 | // to the different types. |
||
| 61 | 12 | $filter_non_files = $pred_factory->_and |
|
| 62 | 12 | ([$pred_factory->_not($pred_factory->_type_is("file")) |
|
| 63 | 12 | , $filter |
|
| 64 | 12 | ]); |
|
| 65 | 12 | $filter_files = $pred_factory->_and |
|
| 66 | 12 | ([$pred_factory->_type_is("file") |
|
| 67 | 12 | , $filter |
|
| 68 | 12 | ]); |
|
| 69 | 12 | $regexp = $rule->argument(0); |
|
| 70 | // TODO: This behaviour might better be encoded as a method on regexp. |
||
| 71 | // This is needed in get_source_location to find the number of |
||
| 72 | // the line the regexp was found. |
||
| 73 | 12 | $loc_regexp = new Regexp(".*".$rule->argument(0)->raw()); |
|
| 74 | |||
| 75 | 12 | if ($mode == Rule::MODE_CANNOT || $mode == Rule::MODE_ONLY_CAN) { |
|
| 76 | return |
||
| 77 | 9 | [ $index->query() |
|
| 78 | 9 | ->filter($filter_non_files) |
|
|
|
|||
| 79 | 9 | ->expand_relations(["defined in"]) |
|
| 80 | 9 | ->filter($this->regexp_source_filter($pred_factory, $regexp, false)) |
|
| 81 | ->extract(function($e,&$r) use ($regexp, $loc_regexp) { |
||
| 82 | 1 | $file = $e->target(); |
|
| 83 | 1 | $found_at_line = $this->get_source_location($e, $loc_regexp); |
|
| 84 | 1 | $start_line = $e->property("start_line"); |
|
| 85 | 1 | $line = $start_line + $found_at_line - 1; |
|
| 86 | // -1 is for the line where the class starts, this would |
||
| 87 | // count double otherwise. |
||
| 88 | 1 | $r["file"] = $file->property("path"); |
|
| 89 | 1 | $r["line"] = $line; |
|
| 90 | 1 | $r["source"] = $file->property("source")[$line-1]; |
|
| 91 | 9 | }) |
|
| 92 | 9 | , $index->query() |
|
| 93 | 9 | ->filter($filter_files) |
|
| 94 | 9 | ->filter($this->regexp_source_filter($pred_factory, $regexp, false)) |
|
| 95 | ->extract(function($e,&$r) use ($regexp, $loc_regexp) { |
||
| 96 | 2 | $line = $this->get_source_location($e, $loc_regexp); |
|
| 97 | 2 | $r["file"] = $e->property("path"); |
|
| 98 | 2 | $r["line"] = $line; |
|
| 99 | 2 | $r["source"] = $e->property("source")[$line-1]; |
|
| 100 | 9 | }) |
|
| 101 | 9 | ]; |
|
| 102 | } |
||
| 103 | 3 | if ($mode == Rule::MODE_MUST) { |
|
| 104 | return |
||
| 105 | 3 | [ $index->query() |
|
| 106 | 3 | ->filter($filter_non_files) |
|
| 107 | 4 | ->expand_relations(["defined in"]) |
|
| 108 | 3 | ->filter($this->regexp_source_filter($pred_factory, $regexp, true)) |
|
| 109 | View Code Duplication | ->extract(function($e,&$r) use ($rule) { |
|
| 110 | 1 | $file = $e->target(); |
|
| 111 | 1 | $r["file"] = $file->property("path"); |
|
| 112 | 1 | $line = $e->property("start_line"); |
|
| 113 | 1 | $r["line"] = $line; |
|
| 114 | 1 | $r["source"] = $file->property("source")[$line - 1]; |
|
| 115 | 3 | }) |
|
| 116 | // TODO: add implementation for files here. |
||
| 117 | 3 | ]; |
|
| 118 | } |
||
| 119 | throw new \LogicException("Unknown rule mode: '$mode'"); |
||
| 120 | } |
||
| 121 | |||
| 122 | // Helpers for compile |
||
| 123 | |||
| 124 | 9 | protected function get_source_for(Graph\Entity $e) { |
|
| 125 | 9 | if ($e->type() == "defined in") { |
|
| 126 | 5 | return $this->get_source_for_defined_in($e); |
|
| 127 | } |
||
| 128 | 5 | if ($e->type() == "file") { |
|
| 129 | 4 | return $this->get_source_for_file($e); |
|
| 130 | } |
||
| 131 | throw new \LogicException( |
||
| 132 | "Can't get source for entity with type '".$e->type()."'"); |
||
| 133 | } |
||
| 134 | |||
| 135 | 5 | protected function get_source_for_defined_in(Graph\Relation $r) { |
|
| 136 | 5 | assert('$r->type() == "defined in"'); |
|
| 137 | 5 | $start_line = $r->property("start_line"); |
|
| 138 | 5 | $end_line = $r->property("end_line"); |
|
| 139 | return implode |
||
| 140 | 5 | ( "\n" |
|
| 141 | 5 | , array_slice |
|
| 142 | 5 | ( $r->target()->property("source") |
|
| 143 | 5 | , $start_line - 1 |
|
| 144 | 5 | , $end_line - $start_line |
|
| 145 | 5 | ) |
|
| 146 | 5 | ); |
|
| 147 | } |
||
| 148 | |||
| 149 | 4 | protected function get_source_for_file(Graph\Node $f) { |
|
| 150 | 4 | assert('$f->type() == "file"'); |
|
| 151 | return implode |
||
| 152 | 4 | ( "\n" |
|
| 153 | 4 | , $f->property("source") |
|
| 154 | 4 | ); |
|
| 155 | } |
||
| 156 | |||
| 157 | 12 | protected function regexp_source_filter(PredicateFactory $f, Regexp $regexp, $negate) { |
|
| 158 | 12 | assert('is_bool($negate)'); |
|
| 159 | 12 | return $f->_custom(function(Graph\Entity $e) use ($regexp, $negate) { |
|
| 160 | 9 | $source = $this->get_source_for($e); |
|
| 161 | 9 | if(!$negate) { |
|
| 162 | 7 | return $regexp->search($source); |
|
| 163 | } |
||
| 164 | else { |
||
| 165 | 2 | return !$regexp->search($source); |
|
| 166 | } |
||
| 167 | 12 | }); |
|
| 168 | } |
||
| 169 | |||
| 170 | 3 | protected function get_source_location(Graph\Entity $e, Regexp $regexp) { |
|
| 176 | |||
| 177 | /** |
||
| 178 | * @inheritdoc |
||
| 179 | */ |
||
| 180 | 12 | public function pprint(Rule $rule) { |
|
| 183 | |||
| 184 | /** |
||
| 185 | * No listeners required for contains text. |
||
| 186 | * |
||
| 187 | * @inheritdoc |
||
| 188 | */ |
||
| 189 | 11 | public function register_listeners(ListenerRegistry $registry) { |
|
| 191 | } |
||
| 192 |
Loading history...
Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: