Skip to content

ext/phar: changing few methods returning true to void ones. #10804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ext/phar: changing few methods returning true to void ones. #10804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ PHP 8.3 UPGRADE NOTES
Underscores must be encoded as "=5F" in such MIME encoded words.
(Alex Dowad)

- Phar:
. Phar::delete, Phar::decompress, Phar::decompressFiles, Phar::copy,
Phar::delMetadata, Phar::setStub, Phar::unlinkArchive,
PharFileInfo::compress, PharFileInfo::decompress, PharFileInfo::delMetadata
no longer return true on success.

- Standard:
. E_NOTICEs emitted by unserialized() have been promoted to E_WARNING.
RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling
Expand Down
34 changes: 8 additions & 26 deletions ext/phar/phar_object.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,6 @@ PHP_METHOD(Phar, unlinkArchive)
phar_archive_delref(phar);
unlink(fname);
efree(fname);
RETURN_TRUE;
}
/* }}} */

Expand Down Expand Up @@ -2609,7 +2608,7 @@ PHP_METHOD(Phar, delete)
if (NULL != (entry = zend_hash_str_find_ptr(&phar_obj->archive->manifest, fname, (uint32_t) fname_len))) {
if (entry->is_deleted) {
/* entry is deleted, but has not been flushed to disk yet */
RETURN_TRUE;
return;
} else {
entry->is_deleted = 1;
entry->is_modified = 1;
Expand All @@ -2626,8 +2625,6 @@ PHP_METHOD(Phar, delete)
efree(error);
RETURN_THROWS();
}

RETURN_TRUE;
}
/* }}} */

Expand Down Expand Up @@ -2872,7 +2869,7 @@ PHP_METHOD(Phar, setStub)
zend_throw_exception_ex(phar_ce_PharException, 0, "%s", error);
efree(error);
}
RETURN_TRUE;
return;
} else {
zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0,
"Cannot change stub, unable to read from input stream");
Expand All @@ -2890,7 +2887,7 @@ PHP_METHOD(Phar, setStub)
RETURN_THROWS();
}

RETURN_TRUE;
return;
}

RETURN_THROWS();
Expand Down Expand Up @@ -3362,7 +3359,7 @@ PHP_METHOD(Phar, decompressFiles)
}

if (phar_obj->archive->is_tar) {
RETURN_TRUE;
return;
} else {
if (phar_obj->archive->is_persistent && FAILURE == phar_copy_on_write(&(phar_obj->archive))) {
zend_throw_exception_ex(phar_ce_PharException, 0, "phar \"%s\" is persistent, unable to copy on write", phar_obj->archive->fname);
Expand All @@ -3378,8 +3375,6 @@ PHP_METHOD(Phar, decompressFiles)
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "%s", error);
efree(error);
}

RETURN_TRUE;
}
/* }}} */

Expand Down Expand Up @@ -3473,8 +3468,6 @@ PHP_METHOD(Phar, copy)
zend_throw_exception_ex(phar_ce_PharException, 0, "%s", error);
efree(error);
}

RETURN_TRUE;
}
/* }}} */

Expand Down Expand Up @@ -4062,7 +4055,7 @@ PHP_METHOD(Phar, delMetadata)
}

if (!phar_metadata_tracker_has_data(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent)) {
RETURN_TRUE;
return;
}

phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent);
Expand All @@ -4073,8 +4066,6 @@ PHP_METHOD(Phar, delMetadata)
zend_throw_exception_ex(phar_ce_PharException, 0, "%s", error);
efree(error);
RETURN_THROWS();
} else {
RETURN_TRUE;
}
}
/* }}} */
Expand Down Expand Up @@ -4771,12 +4762,7 @@ PHP_METHOD(PharFileInfo, delMetadata)
zend_throw_exception_ex(phar_ce_PharException, 0, "%s", error);
efree(error);
RETURN_THROWS();
} else {
RETURN_TRUE;
}

} else {
RETURN_TRUE;
}
}
/* }}} */
Expand Down Expand Up @@ -4879,7 +4865,7 @@ PHP_METHOD(PharFileInfo, compress)
switch (method) {
case PHAR_ENT_COMPRESSED_GZ:
if (entry_obj->entry->flags & PHAR_ENT_COMPRESSED_GZ) {
RETURN_TRUE;
return;
}

if ((entry_obj->entry->flags & PHAR_ENT_COMPRESSED_BZ2) != 0) {
Expand Down Expand Up @@ -4910,7 +4896,7 @@ PHP_METHOD(PharFileInfo, compress)
break;
case PHAR_ENT_COMPRESSED_BZ2:
if (entry_obj->entry->flags & PHAR_ENT_COMPRESSED_BZ2) {
RETURN_TRUE;
return;
}

if ((entry_obj->entry->flags & PHAR_ENT_COMPRESSED_GZ) != 0) {
Expand Down Expand Up @@ -4952,8 +4938,6 @@ PHP_METHOD(PharFileInfo, compress)
efree(error);
RETURN_THROWS();
}

RETURN_TRUE;
}
/* }}} */

Expand All @@ -4976,7 +4960,7 @@ PHP_METHOD(PharFileInfo, decompress)
}

if ((entry_obj->entry->flags & PHAR_ENT_COMPRESSION_MASK) == 0) {
RETURN_TRUE;
return;
}

if (PHAR_G(readonly) && !entry_obj->entry->phar->is_data) {
Expand Down Expand Up @@ -5044,8 +5028,6 @@ PHP_METHOD(PharFileInfo, decompress)
efree(error);
RETURN_THROWS();
}

RETURN_TRUE;
}
/* }}} */

Expand Down
30 changes: 15 additions & 15 deletions ext/phar/phar_object.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public function buildFromIterator(Traversable $iterator, ?string $baseDirectory
public function compressFiles(int $compression): void {}

/** @return bool */
public function decompressFiles() {} // TODO make return type void
public function decompressFiles(): void {}
Copy link
Member

@kocsismate kocsismate Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO these shouldn't be made native type declarations first so that the type declarations users already have can be fixed easier.


/** @tentative-return-type */
public function compress(int $compression, ?string $extension = null): ?Phar {}
Expand All @@ -127,16 +127,16 @@ public function convertToExecutable(?int $format = null, ?int $compression = nul
public function convertToData(?int $format = null, ?int $compression = null, ?string $extension = null): ?PharData {}

/** @return bool */
public function copy(string $from, string $to) {} // TODO make return type void
public function copy(string $from, string $to): void {}

/** @tentative-return-type */
public function count(int $mode = COUNT_NORMAL): int {}

/** @return bool */
public function delete(string $localName) {} // TODO make return type void
public function delete(string $localName): void {}

/** @return bool */
public function delMetadata() {} // TODO make return type void
public function delMetadata(): void {}

/** @tentative-return-type */
public function extractTo(string $directory, array|string|null $files = null, bool $overwrite = false): bool {}
Expand Down Expand Up @@ -218,7 +218,7 @@ public function setSignatureAlgorithm(int $algo, ?string $privateKey = null): vo
* @param resource|string $stub
* @return bool
*/
public function setStub($stub, int $length = UNKNOWN) {} // TODO make return type void
public function setStub($stub, int $length = UNKNOWN): void {}

/** @tentative-return-type */
public function startBuffering(): void {}
Expand Down Expand Up @@ -252,7 +252,7 @@ final public static function mount(string $pharPath, string $externalPath): void

final public static function mungServer(array $variables): void {}

final public static function unlinkArchive(string $filename): bool {} // TODO make return type void
final public static function unlinkArchive(string $filename): void {}

final public static function webPhar(
?string $alias = null, ?string $index = null, ?string $fileNotFoundScript = null,
Expand Down Expand Up @@ -310,7 +310,7 @@ public function compressFiles(int $compression): void {}
* @return bool
* @implementation-alias Phar::decompressFiles
*/
public function decompressFiles() {} // TODO make return type void
public function decompressFiles(): void {}

/**
* @tentative-return-type
Expand Down Expand Up @@ -342,7 +342,7 @@ public function convertToData(?int $format = null, ?int $compression = null, ?st
* @return bool
* @implementation-alias Phar::copy
*/
public function copy(string $from, string $to) {} // TODO make return type void
public function copy(string $from, string $to): void {}

/**
* @tentative-return-type
Expand All @@ -354,13 +354,13 @@ public function count(int $mode = COUNT_NORMAL): int {}
* @return bool
* @implementation-alias Phar::delete
*/
public function delete(string $localName) {} // TODO make return type void
public function delete(string $localName): void {}

/**
* @return bool
* @implementation-alias Phar::delMetadata
*/
public function delMetadata() {} // TODO make return type void
public function delMetadata(): void {}

/**
* @tentative-return-type
Expand Down Expand Up @@ -498,7 +498,7 @@ public function setSignatureAlgorithm(int $algo, ?string $privateKey = null): vo
* @return bool
* @implementation-alias Phar::setStub
*/
public function setStub($stub, int $length = UNKNOWN) {} // TODO make return type void
public function setStub($stub, int $length = UNKNOWN): void {}

/**
* @tentative-return-type
Expand Down Expand Up @@ -552,7 +552,7 @@ final public static function mount(string $pharPath, string $externalPath): void
final public static function mungServer(array $variables): void {}

/** @implementation-alias Phar::unlinkArchive */
final public static function unlinkArchive(string $filename): bool {} // TODO make return type void
final public static function unlinkArchive(string $filename): void {}

/** @implementation-alias Phar::webPhar */
final public static function webPhar(
Expand All @@ -570,13 +570,13 @@ public function __destruct() {}
public function chmod(int $perms): void {}

/** @return bool */
public function compress(int $compression) {} // TODO make return type void
public function compress(int $compression): void {}

/** @return bool */
public function decompress() {} // TODO make return type void
public function decompress(): void {}

/** @return bool */
public function delMetadata() {} // TODO make return type void
public function delMetadata(): void {}

/** @tentative-return-type */
public function getCompressedSize(): int {}
Expand Down
30 changes: 15 additions & 15 deletions ext/phar/phar_object_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ext/phar/tests/bug76584.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ var_dump($phar->extractTo(__DIR__ . '/76584'));
echo file_get_contents(__DIR__ . '/76584/76584.txt');
?>
--EXPECT--
NULL
bool(true)
bool(true)
bool(true)
NULL
bool(false)
bool(true)
This is a test file.
Expand Down
Loading