r96416 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r96415‎ | r96416 | r96417 >
Date:12:00, 7 September 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Documentation

Add explicit member variables

Trim trailing whitespace
Modified paths:
  • /trunk/phase3/includes/filerepo/ArchivedFile.php (modified) (history)
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)
  • /trunk/phase3/includes/filerepo/FileRepo.php (modified) (history)
  • /trunk/phase3/includes/filerepo/ForeignAPIFile.php (modified) (history)
  • /trunk/phase3/includes/media/Generic.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/FileRepo.php
——@@ -164,6 +164,8 @@
165165 * $findItem = array( 'title' => $title, 'private' => true );
166166 * $findBatch = array( $findItem );
167167 * $repo->findFiles( $findBatch );
 168+ *
 169+ * @return array
168170 */
169171 function findFiles( $items ) {
170172 $result = array();
——@@ -259,6 +261,8 @@
260262
261263 /**
262264 * Returns true if the repository can transform files via a 404 handler
 265+ *
 266+ * @return bool
263267 */
264268 function canTransformVia404() {
265269 return $this->transformVia404;
——@@ -281,6 +285,11 @@
282286 return $name;
283287 }
284288
 289+ /**
 290+ * @param $name
 291+ * @param $levels
 292+ * @return string
 293+ */
285294 static function getHashPathForLevel( $name, $levels ) {
286295 if ( $levels == 0 ) {
287296 return '';
——@@ -297,6 +306,10 @@
298307 /**
299308 * Get a relative path including trailing slash, e.g. f/fa/
300309 * If the repo is not hashed, returns an empty string
 310+ *
 311+ * @param $name string
 312+ *
 313+ * @return string
301314 */
302315 function getHashPath( $name ) {
303316 return self::getHashPathForLevel( $name, $this->hashLevels );
——@@ -497,6 +510,11 @@
498511 */
499512 abstract function publishBatch( $triplets, $flags = 0 );
500513
 514+ /**
 515+ * @param $file
 516+ * @param int $flags
 517+ * @return bool
 518+ */
501519 function fileExists( $file, $flags = 0 ) {
502520 $result = $this->fileExistsBatch( array( $file ), $flags );
503521 return $result[0];
——@@ -547,6 +565,8 @@
548566 * Get properties of a file with a given virtual URL
549567 * The virtual URL must refer to this repo
550568 * Properties should ultimately be obtained via File::getPropsFromPath()
 569+ *
 570+ * @param $virtualUrl string
551571 */
552572 abstract function getFileProps( $virtualUrl );
553573
——@@ -561,6 +581,10 @@
562582
563583 /**
564584 * Determine if a relative path is valid, i.e. not blank or involving directory traveral
 585+ *
 586+ * @param $filename string
 587+ *
 588+ * @return bool
565589 */
566590 function validateFilename( $filename ) {
567591 if ( strval( $filename ) == '' ) {
——@@ -573,11 +597,11 @@
574598 * Use the same traversal protection as Title::secureAndSplit()
575599 */
576600 if ( strpos( $filename, '.' ) !== false &&
577 - ( $filename === '.' || $filename === '..' ||
578 - strpos( $filename, './' ) === 0 ||
579 - strpos( $filename, '../' ) === 0 ||
580 - strpos( $filename, '/./' ) !== false ||
581 - strpos( $filename, '/../' ) !== false ) )
 601+ ( $filename === '.' || $filename === '..' ||
 602+ strpos( $filename, './' ) === 0 ||
 603+ strpos( $filename, '../' ) === 0 ||
 604+ strpos( $filename, '/./' ) !== false ||
 605+ strpos( $filename, '/../' ) !== false ) )
582606 {
583607 return false;
584608 } else {
——@@ -589,6 +613,11 @@
590614 * Path disclosure protection functions
591615 */
592616 function paranoidClean( $param ) { return '[hidden]'; }
 617+
 618+ /**
 619+ * @param $param
 620+ * @return
 621+ */
593622 function passThrough( $param ) { return $param; }
594623
595624 /**
——@@ -617,6 +646,8 @@
618647
619648 /**
620649 * Create a new good result
 650+ *
 651+ * @return FileRepoStatus
621652 */
622653 function newGood( $value = null ) {
623654 return FileRepoStatus::newGood( $this, $value );
——@@ -681,7 +712,6 @@
682713 return $this->getName() == 'local';
683714 }
684715
685 -
686716 /**
687717 * Get a key on the primary cache for this repository.
688718 * Returns false if the repository's cache is not accessible at this site.
Index: trunk/phase3/includes/filerepo/File.php
——@@ -70,8 +70,18 @@
7171 */
7272 protected $handler;
7373
 74+ protected $url, $extension, $name, $path, $hashPath, $pageCount, $transformScript;
 75+
7476 /**
 77+ * @var bool
 78+ */
 79+ protected $canRender, $isSafeFile;
 80+
 81+ /**
7582 * Call this constructor from child classes
 83+ *
 84+ * @param $title
 85+ * @param $repo
7686 */
7787 function __construct( $title, $repo ) {
7888 $this->title = $title;
——@@ -117,7 +127,7 @@
118128 *
119129 * @param $old File Old file
120130 * @param $new string New name
121 - *
 131+ *
122132 * @return bool|null
123133 */
124134 static function checkExtensionCompatibility( File $old, $new ) {
——@@ -217,7 +227,10 @@
218228 public function getFullUrl() {
219229 return wfExpandUrl( $this->getUrl(), PROTO_RELATIVE );
220230 }
221 -
 231+
 232+ /**
 233+ * @return string
 234+ */
222235 public function getCanonicalUrl() {
223236 return wfExpandUrl( $this->getUrl(), PROTO_CANONICAL );
224237 }
——@@ -292,6 +305,8 @@
293306 * STUB
294307 * Overridden by LocalFile, UnregisteredLocalFile
295308 *
 309+ * @param $page int
 310+ *
296311 * @return false|number
297312 */
298313 public function getHeight( $page = 1 ) {
——@@ -475,7 +490,7 @@
476491
477492 /**
478493 * Accessor for __get()
479 - *
 494+ *
480495 * @return bool
481496 */
482497 protected function getIsSafeFile() {
——@@ -891,10 +906,10 @@
892907
893908 /**
894909 * Get the relative path for an archived file
895 - *
 910+ *
896911 * @param $suffix bool|string if not false, the name of an archived thumbnail file
897912 *
898 - * @return string
 913+ * @return string
899914 */
900915 function getArchiveRel( $suffix = false ) {
901916 $path = 'archive/' . $this->getHashPath();
——@@ -909,9 +924,11 @@
910925 /**
911926 * Get the relative path for an archived file's thumbs directory
912927 * or a specific thumb if the $suffix is given.
913 - *
 928+ *
914929 * @param $archiveName string the timestamped name of an archived image
915930 * @param $suffix bool|string if not false, the name of a thumbnail file
 931+ *
 932+ * @return string
916933 */
917934 function getArchiveThumbRel( $archiveName, $suffix = false ) {
918935 $path = 'archive/' . $this->getHashPath() . $archiveName . "/";
Index: trunk/phase3/includes/filerepo/ForeignAPIFile.php
——@@ -13,13 +13,13 @@
1414 * @ingroup FileRepo
1515 */
1616 class ForeignAPIFile extends File {
17 -
 17+
1818 private $mExists;
1919
2020 /**
21 - * @param $title
22 - * @param $repo ForeignApiRepo
23 - * @param $info
 21+ * @param $title
 22+ * @param $repo ForeignApiRepo
 23+ * @param $info
2424 * @param bool $exists
2525 */
2626 function __construct( $title, $repo, $info, $exists = false ) {
——@@ -29,8 +29,8 @@
3030 }
3131
3232 /**
33 - * @param $title Title
34 - * @param $repo ForeignApiRepo
 33+ * @param $title Title
 34+ * @param $repo ForeignApiRepo
3535 * @return ForeignAPIFile|null
3636 */
3737 static function newFromTitle( $title, $repo ) {
——@@ -61,19 +61,19 @@
6262 return null;
6363 }
6464 }
65 -
 65+
6666 /**
6767 * Get the property string for iiprop and aiprop
6868 */
6969 static function getProps() {
7070 return 'timestamp|user|comment|url|size|sha1|metadata|mime';
7171 }
72 -
 72+
7373 // Dummy functions...
7474 public function exists() {
7575 return $this->mExists;
7676 }
77 -
 77+
7878 public function getPath() {
7979 return false;
8080 }
——@@ -100,18 +100,22 @@
101101 public function getWidth( $page = 1 ) {
102102 return isset( $this->mInfo['width'] ) ? intval( $this->mInfo['width'] ) : 0;
103103 }
104 -
 104+
 105+ /**
 106+ * @param $page int
 107+ * @return int
 108+ */
105109 public function getHeight( $page = 1 ) {
106110 return isset( $this->mInfo['height'] ) ? intval( $this->mInfo['height'] ) : 0;
107111 }
108 -
 112+
109113 public function getMetadata() {
110114 if ( isset( $this->mInfo['metadata'] ) ) {
111115 return serialize( self::parseMetadata( $this->mInfo['metadata'] ) );
112116 }
113117 return null;
114118 }
115 -
 119+
116120 public static function parseMetadata( $metadata ) {
117121 if( !is_array( $metadata ) ) {
118122 return $metadata;
——@@ -122,11 +126,11 @@
123127 }
124128 return $ret;
125129 }
126 -
 130+
127131 public function getSize() {
128132 return isset( $this->mInfo['size'] ) ? intval( $this->mInfo['size'] ) : null;
129133 }
130 -
 134+
131135 public function getUrl() {
132136 return isset( $this->mInfo['url'] ) ? strval( $this->mInfo['url'] ) : null;
133137 }
——@@ -134,25 +138,25 @@
135139 public function getUser( $method='text' ) {
136140 return isset( $this->mInfo['user'] ) ? strval( $this->mInfo['user'] ) : null;
137141 }
138 -
 142+
139143 public function getDescription() {
140144 return isset( $this->mInfo['comment'] ) ? strval( $this->mInfo['comment'] ) : null;
141145 }
142146
143147 function getSha1() {
144 - return isset( $this->mInfo['sha1'] ) ?
145 - wfBaseConvert( strval( $this->mInfo['sha1'] ), 16, 36, 31 ) :
 148+ return isset( $this->mInfo['sha1'] ) ?
 149+ wfBaseConvert( strval( $this->mInfo['sha1'] ), 16, 36, 31 ) :
146150 null;
147151 }
148 -
 152+
149153 function getTimestamp() {
150 - return wfTimestamp( TS_MW,
 154+ return wfTimestamp( TS_MW,
151155 isset( $this->mInfo['timestamp'] ) ?
152 - strval( $this->mInfo['timestamp'] ) :
 156+ strval( $this->mInfo['timestamp'] ) :
153157 null
154158 );
155159 }
156 -
 160+
157161 function getMimeType() {
158162 if( !isset( $this->mInfo['mime'] ) ) {
159163 $magic = MimeMagic::singleton();
——@@ -160,19 +164,19 @@
161165 }
162166 return $this->mInfo['mime'];
163167 }
164 -
 168+
165169 /// @todo FIXME: May guess wrong on file types that can be eg audio or video
166170 function getMediaType() {
167171 $magic = MimeMagic::singleton();
168172 return $magic->getMediaType( null, $this->getMimeType() );
169173 }
170 -
 174+
171175 function getDescriptionUrl() {
172176 return isset( $this->mInfo['descriptionurl'] )
173177 ? $this->mInfo['descriptionurl']
174178 : false;
175179 }
176 -
 180+
177181 /**
178182 * Only useful if we're locally caching thumbs anyway...
179183 */
——@@ -187,7 +191,7 @@
188192 return null;
189193 }
190194 }
191 -
 195+
192196 function getThumbnails() {
193197 $files = array();
194198 $dir = $this->getThumbPath( $this->getName() );
——@@ -204,19 +208,19 @@
205209 }
206210 return $files;
207211 }
208 -
 212+
209213 function purgeCache() {
210214 $this->purgeThumbnails();
211215 $this->purgeDescriptionPage();
212216 }
213 -
 217+
214218 function purgeDescriptionPage() {
215219 global $wgMemc, $wgContLang;
216220 $url = $this->repo->getDescriptionRenderUrl( $this->getName(), $wgContLang->getCode() );
217221 $key = $this->repo->getLocalCacheKey( 'RemoteFileDescription', 'url', md5($url) );
218222 $wgMemc->delete( $key );
219223 }
220 -
 224+
221225 function purgeThumbnails() {
222226 global $wgMemc;
223227 $key = $this->repo->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $this->getName() );
Index: trunk/phase3/includes/filerepo/ArchivedFile.php
——@@ -177,6 +177,9 @@
178178
179179 /**
180180 * Loads a file object from the filearchive table
 181+ *
 182+ * @param $row
 183+ *
181184 * @return ArchivedFile
182185 */
183186 public static function newFromRow( $row ) {
——@@ -205,6 +208,8 @@
206209
207210 /**
208211 * Return the associated title object
 212+ *
 213+ * @return Title
209214 */
210215 public function getTitle() {
211216 return $this->title;
——@@ -212,16 +217,24 @@
213218
214219 /**
215220 * Return the file name
 221+ *
 222+ * @return string
216223 */
217224 public function getName() {
218225 return $this->name;
219226 }
220227
 228+ /**
 229+ * @return int
 230+ */
221231 public function getID() {
222232 $this->load();
223233 return $this->id;
224234 }
225235
 236+ /**
 237+ * @return bool
 238+ */
226239 public function exists() {
227240 $this->load();
228241 return $this->exists;
——@@ -229,6 +242,7 @@
230243
231244 /**
232245 * Return the FileStore key
 246+ * @return string
233247 */
234248 public function getKey() {
235249 $this->load();
——@@ -237,6 +251,7 @@
238252
239253 /**
240254 * Return the FileStore key (overriding base File class)
 255+ * @return string
241256 */
242257 public function getStorageKey() {
243258 return $this->getKey();
——@@ -244,6 +259,7 @@
245260
246261 /**
247262 * Return the FileStore storage group
 263+ * @return string
248264 */
249265 public function getGroup() {
250266 return $this->group;
——@@ -251,6 +267,7 @@
252268
253269 /**
254270 * Return the width of the image
 271+ * @return int
255272 */
256273 public function getWidth() {
257274 $this->load();
——@@ -259,6 +276,7 @@
260277
261278 /**
262279 * Return the height of the image
 280+ * @return int
263281 */
264282 public function getHeight() {
265283 $this->load();
——@@ -267,6 +285,7 @@
268286
269287 /**
270288 * Get handler-specific metadata
 289+ * @return string
271290 */
272291 public function getMetadata() {
273292 $this->load();
——@@ -275,6 +294,7 @@
276295
277296 /**
278297 * Return the size of the image file, in bytes
 298+ * @return int
279299 */
280300 public function getSize() {
281301 $this->load();
——@@ -283,6 +303,7 @@
284304
285305 /**
286306 * Return the bits of the image file, in bytes
 307+ * @return int
287308 */
288309 public function getBits() {
289310 $this->load();
——@@ -291,6 +312,7 @@
292313
293314 /**
294315 * Returns the mime type of the file.
 316+ * @return string
295317 */
296318 public function getMimeType() {
297319 $this->load();
——@@ -326,6 +348,7 @@
327349 /**
328350 * Return the type of the media in the file.
329351 * Use the value returned by this function with the MEDIATYPE_xxx constants.
 352+ * @return string
330353 */
331354 public function getMediaType() {
332355 $this->load();
——@@ -334,6 +357,8 @@
335358
336359 /**
337360 * Return upload timestamp.
 361+ *
 362+ * @return string
338363 */
339364 public function getTimestamp() {
340365 $this->load();
——@@ -342,6 +367,8 @@
343368
344369 /**
345370 * Return the user ID of the uploader.
 371+ *
 372+ * @return int
346373 */
347374 public function getUser() {
348375 $this->load();
——@@ -354,6 +381,8 @@
355382
356383 /**
357384 * Return the user name of the uploader.
 385+ *
 386+ * @return string
358387 */
359388 public function getUserText() {
360389 $this->load();
——@@ -366,6 +395,8 @@
367396
368397 /**
369398 * Return upload description.
 399+ *
 400+ * @return string
370401 */
371402 public function getDescription() {
372403 $this->load();
——@@ -378,6 +409,8 @@
379410
380411 /**
381412 * Return the user ID of the uploader.
 413+ *
 414+ * @return int
382415 */
383416 public function getRawUser() {
384417 $this->load();
——@@ -386,6 +419,8 @@
387420
388421 /**
389422 * Return the user name of the uploader.
 423+ *
 424+ * @return string
390425 */
391426 public function getRawUserText() {
392427 $this->load();
——@@ -394,6 +429,8 @@
395430
396431 /**
397432 * Return upload description.
 433+ *
 434+ * @return string
398435 */
399436 public function getRawDescription() {
400437 $this->load();
Index: trunk/phase3/includes/media/Generic.php
——@@ -200,6 +200,8 @@
201201 * @param $dstUrl String: destination URL to use in output HTML
202202 * @param $params Array: arbitrary set of parameters validated by $this->validateParam()
203203 * @param $flags Integer: a bitfield, may contain self::TRANSFORM_LATER
 204+ *
 205+ * @return MediaTransformOutput
204206 */
205207 abstract function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 );
206208

Comments

#Comment by Nikerabbit (talk | contribs)   14:31, 7 September 2011

Is there IDE that takes advantages of these annotations?

#Comment by 😂 (talk | contribs)   14:33, 7 September 2011

NetBeans does, and I'm sure it's not alone. Plus, it makes the doxygen docs more complete :)

#Comment by Reedy (talk | contribs)   14:33, 7 September 2011

Yeah, part of the reason for adding them. Objects of known types then can have autocomplete

Status & tagging log