recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Classify changesets as "generated" at creation time, in addition to display time

Summary:
Ref T13130. See PHI251. Currently, changesets are marked as "generated" (i.e., the file contains generated code and does not normally need to be reviewed) at display time.

An install would like support for having Owners rules ignore generated files. Additionally, future changes anticipate making "generated" and some other similar behaviors more flexible and more general.

To support these, move toward a world where:

- Changesets have "attributes": today, generated. In the future, perhaps: third-party, highlight-as, encoding, enormous-text-file, etc.
- Attributes are either "trusted" (usually: the server assigned the attribute) or "untrusted" (usually: the client assigned the attribute). For attributes like "highlight-as", this isn't relevant, but I'd like to provide tools so that you can't make `arc` mark every file as "generated" and sneak past review rules in the future.

Here, the `differential.generated-paths` config can mark a file as "generated" with a trusted attribute. The `@generated`-in-content rule can mark a file as "generated" with an untrusted attribute.

Putting these attributes on changesets at creation time instead of display time will let Owners interact with changesets cheaply: it won't have to render an entire changeset just to figure out if it's generated or not.

Test Plan:
- Created a revision touching several files, some generated and some not.
- Saw the generated files get marked properly with attribute metadata in the database, and show/fold as "Generated" in the UI.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13130

Differential Revision: https://secure.phabricator.com/D19425

epriestley af295341 5784e3d3

+141 -1
+6
src/applications/differential/parser/DifferentialChangesetParser.php
··· 542 542 PhutilEventEngine::dispatchEvent($event); 543 543 544 544 $generated = $event->getValue('is_generated'); 545 + 546 + $attribute = $this->changeset->isGeneratedChangeset(); 547 + if ($attribute) { 548 + $generated = true; 549 + } 550 + 545 551 $this->specialAttributes[self::ATTR_GENERATED] = $generated; 546 552 } 547 553
+90 -1
src/applications/differential/storage/DifferentialChangeset.php
··· 12 12 protected $awayPaths; 13 13 protected $changeType; 14 14 protected $fileType; 15 - protected $metadata; 15 + protected $metadata = array(); 16 16 protected $oldProperties; 17 17 protected $newProperties; 18 18 protected $addLines; ··· 23 23 private $diff = self::ATTACHABLE; 24 24 25 25 const TABLE_CACHE = 'differential_changeset_parse_cache'; 26 + 27 + const METADATA_TRUSTED_ATTRIBUTES = 'attributes.trusted'; 28 + const METADATA_UNTRUSTED_ATTRIBUTES = 'attributes.untrusted'; 29 + 30 + const ATTRIBUTE_GENERATED = 'generated'; 26 31 27 32 protected function getConfiguration() { 28 33 return array( ··· 264 269 } 265 270 266 271 return null; 272 + } 273 + 274 + public function setChangesetMetadata($key, $value) { 275 + if (!is_array($this->metadata)) { 276 + $this->metadata = array(); 277 + } 278 + 279 + $this->metadata[$key] = $value; 280 + 281 + return $this; 282 + } 283 + 284 + public function getChangesetMetadata($key, $default = null) { 285 + if (!is_array($this->metadata)) { 286 + return $default; 287 + } 288 + 289 + return idx($this->metadata, $key, $default); 290 + } 291 + 292 + private function setInternalChangesetAttribute($trusted, $key, $value) { 293 + if ($trusted) { 294 + $meta_key = self::METADATA_TRUSTED_ATTRIBUTES; 295 + } else { 296 + $meta_key = self::METADATA_UNTRUSTED_ATTRIBUTES; 297 + } 298 + 299 + $attributes = $this->getChangesetMetadata($meta_key, array()); 300 + $attributes[$key] = $value; 301 + $this->setChangesetMetadata($meta_key, $attributes); 302 + 303 + return $this; 304 + } 305 + 306 + private function getInternalChangesetAttributes($trusted) { 307 + if ($trusted) { 308 + $meta_key = self::METADATA_TRUSTED_ATTRIBUTES; 309 + } else { 310 + $meta_key = self::METADATA_UNTRUSTED_ATTRIBUTES; 311 + } 312 + 313 + return $this->getChangesetMetadata($meta_key, array()); 314 + } 315 + 316 + public function setTrustedChangesetAttribute($key, $value) { 317 + return $this->setInternalChangesetAttribute(true, $key, $value); 318 + } 319 + 320 + public function getTrustedChangesetAttributes() { 321 + return $this->getInternalChangesetAttributes(true); 322 + } 323 + 324 + public function getTrustedChangesetAttribute($key, $default = null) { 325 + $map = $this->getTrustedChangesetAttributes(); 326 + return idx($map, $key, $default); 327 + } 328 + 329 + public function setUntrustedChangesetAttribute($key, $value) { 330 + return $this->setInternalChangesetAttribute(false, $key, $value); 331 + } 332 + 333 + public function getUntrustedChangesetAttributes() { 334 + return $this->getInternalChangesetAttributes(false); 335 + } 336 + 337 + public function getUntrustedChangesetAttribute($key, $default = null) { 338 + $map = $this->getUntrustedChangesetAttributes(); 339 + return idx($map, $key, $default); 340 + } 341 + 342 + public function getChangesetAttributes() { 343 + // Prefer trusted values over untrusted values when both exist. 344 + return 345 + $this->getTrustedChangesetAttributes() + 346 + $this->getUntrustedChangesetAttributes(); 347 + } 348 + 349 + public function getChangesetAttribute($key, $default = null) { 350 + $map = $this->getChangesetAttributes(); 351 + return idx($map, $key, $default); 352 + } 353 + 354 + public function isGeneratedChangeset() { 355 + return $this->getChangesetAttribute(self::ATTRIBUTE_GENERATED); 267 356 } 268 357 269 358
+45
src/applications/differential/storage/DifferentialDiff.php
··· 226 226 $changeset->setAddLines($add_lines); 227 227 $changeset->setDelLines($del_lines); 228 228 229 + self::detectGeneratedCode($changeset); 230 + 229 231 $diff->addUnsavedChangeset($changeset); 230 232 } 231 233 $diff->setLineCount($lines); ··· 821 823 ); 822 824 } 823 825 826 + private static function detectGeneratedCode( 827 + DifferentialChangeset $changeset) { 828 + 829 + $is_generated_trusted = self::isTrustedGeneratedCode($changeset); 830 + 831 + $changeset->setTrustedChangesetAttribute( 832 + DifferentialChangeset::ATTRIBUTE_GENERATED, 833 + $is_generated_trusted); 834 + 835 + $is_generated_untrusted = self::isUntrustedGeneratedCode($changeset); 836 + 837 + $changeset->setUntrustedChangesetAttribute( 838 + DifferentialChangeset::ATTRIBUTE_GENERATED, 839 + $is_generated_untrusted); 840 + } 841 + 842 + private static function isTrustedGeneratedCode( 843 + DifferentialChangeset $changeset) { 844 + 845 + $filename = $changeset->getFilename(); 846 + 847 + $paths = PhabricatorEnv::getEnvConfig('differential.generated-paths'); 848 + foreach ($paths as $regexp) { 849 + if (preg_match($regexp, $filename)) { 850 + return true; 851 + } 852 + } 853 + 854 + return false; 855 + } 856 + 857 + private static function isUntrustedGeneratedCode( 858 + DifferentialChangeset $changeset) { 859 + 860 + if ($changeset->getHunks()) { 861 + $new_data = $changeset->makeNewFile(); 862 + if (strpos($new_data, '@'.'generated') !== false) { 863 + return true; 864 + } 865 + } 866 + 867 + return false; 868 + } 824 869 825 870 }