Skip to content

3634: Improve code quality and increase PHPStan level #30

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
martin-augment wants to merge 2 commits into
base: main
Choose a base branch
from
Open

3634: Improve code quality and increase PHPStan level #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81b06f5
improve code quality and increase phpstan level
mattiabasone Jan 23, 2026
fc83923
update exception messages and fixes check for bz2
mattiabasone Jan 26, 2026
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
14 changes: 7 additions & 7 deletions lang/php/lib/Avro.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,42 @@ class Avro
* self::GMP_BIGINTEGER_MODE on 32-bit platforms that have GMP available,
* and to self::PHP_BIGINTEGER_MODE otherwise.
*/
private static int $biginteger_mode;
private static int $bigIntegerMode;

/**
* Wrapper method to call each required check.
*/
public static function checkPlatform()
public static function checkPlatform(): void
{
self::check64Bit();
}

/**
* @returns bool true if the PHP GMP extension is used and false otherwise.
* @return bool true if the PHP GMP extension is used and false otherwise.
* @internal Requires Avro::check64Bit() (exposed via Avro::checkPlatform())
* to have been called to set Avro::$biginteger_mode.
*/
public static function usesGmp(): bool
{
return self::GMP_BIGINTEGER_MODE === self::$biginteger_mode;
return self::GMP_BIGINTEGER_MODE === self::$bigIntegerMode;
}
Comment on lines +63 to 70
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Jan 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update the docblock to reference the renamed property.

Line 65 still references Avro::$biginteger_mode but the property was renamed to $bigIntegerMode.

📝 Suggested fix 
     /**
      * `@return` bool true if the PHP GMP extension is used and false otherwise.
      * `@internal` Requires Avro::check64Bit() (exposed via Avro::checkPlatform())
-     *           to have been called to set Avro::$biginteger_mode.
+     *           to have been called to set Avro::$bigIntegerMode.
      */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* @return bool true if the PHP GMP extension is used and false otherwise.
* @internal Requires Avro::check64Bit() (exposed via Avro::checkPlatform())
* to have been called to set Avro::$biginteger_mode.
*/
public static function usesGmp(): bool
{
return self::GMP_BIGINTEGER_MODE === self::$biginteger_mode;
return self::GMP_BIGINTEGER_MODE === self::$bigIntegerMode;
}
/**
* `@return` bool true if the PHP GMP extension is used and false otherwise.
* `@internal` Requires Avro::check64Bit() (exposed via Avro::checkPlatform())
* to have been called to set Avro::$bigIntegerMode.
*/
public static function usesGmp(): bool
{
return self::GMP_BIGINTEGER_MODE === self::$bigIntegerMode;
}
🤖 Prompt for AI Agents 
In `@lang/php/lib/Avro.php` around lines 63 - 70, Update the docblock on the
usesGmp() method to reference the renamed property $bigIntegerMode (instead of
Avro::$biginteger_mode); mention the dependency on Avro::check64Bit() (or
Avro::checkPlatform()) setting Avro::$bigIntegerMode so the documentation
matches the actual property name and casing.


/**
* Determines if the host platform can encode and decode long integer data.
*
* @throws AvroException if the platform cannot handle long integers.
*/
private static function check64Bit()
private static function check64Bit(): void
{
if (8 !== PHP_INT_SIZE) {
if (extension_loaded('gmp')) {
self::$biginteger_mode = self::GMP_BIGINTEGER_MODE;
self::$bigIntegerMode = self::GMP_BIGINTEGER_MODE;
} else {
throw new AvroException('This platform cannot handle a 64-bit operations. '
.'Please install the GMP PHP extension.');
}
} else {
self::$biginteger_mode = self::PHP_BIGINTEGER_MODE;
self::$bigIntegerMode = self::PHP_BIGINTEGER_MODE;
}
}
}
60 changes: 26 additions & 34 deletions lang/php/lib/AvroDebug.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ class AvroDebug
/**
* @param string $format format string for the given arguments. Passed as is
* to <code>vprintf</code>.
* @param array $args array of arguments to pass to vsprinf.
* @param int $debug_level debug level at which to print this statement
* @returns boolean true
* @param list<string> $args array of arguments to pass to vsprinf.
* @param int $debugLevel debug level at which to print this statement
* @return bool true
*/
public static function debug($format, $args, $debug_level = self::DEBUG1)
public static function debug(string $format, array $args, int $debugLevel = self::DEBUG1): bool
{
if (self::isDebug($debug_level)) {
if (self::isDebug($debugLevel)) {
vprintf($format."\n", $args);
}

Expand All @@ -59,37 +59,34 @@ public static function debug($format, $args, $debug_level = self::DEBUG1)
* or more verbose than than the current debug level
* and false otherwise.
*/
public static function isDebug(int $debug_level = self::DEBUG1): bool
public static function isDebug(int $debugLevel = self::DEBUG1): bool
{
return self::DEBUG_LEVEL >= $debug_level;
return self::DEBUG_LEVEL >= $debugLevel;
}

/**
* @param string $str
* @param string $joiner string used to join
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for hexString, but the $str parameter is still present in the function signature. This creates a mismatch between the documentation and the actual function definition, which can lead to confusion.

     * @param string $str

* @returns string hex-represented bytes of each byte of $str
* @return string hex-represented bytes of each byte of $str
* joined by $joiner
*/
public static function hexString($str, $joiner = ' ')
public static function hexString(string $str, string $joiner = ' '): string
{
return implode($joiner, self::hexArray($str));
}

/**
* @param string $str
* @returns string[] array of hex representation of each byte of $str
* @return string[] array of hex representation of each byte of $str
*/
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for hexArray, but the $str parameter is still present in the function signature. Please ensure the PHPDoc accurately reflects the function's parameters.

     * @param string $str

public static function hexArray($str)
public static function hexArray(string $str): array
{
return self::bytesArray($str);
}

/**
* @param string $str
* @param string $format format to represent bytes
* @returns string[] array of each byte of $str formatted using $format
* @return string[] array of each byte of $str formatted using $format
*/
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for bytesArray, but the $str parameter is still present in the function signature. It's important to keep the PHPDoc synchronized with the code for clarity.

     * @param string $str

public static function bytesArray($str, $format = 'x%02x')
public static function bytesArray(string $str, string $format = 'x%02x'): array
{
$x = [];
foreach (str_split($str) as $b) {
Expand All @@ -100,40 +97,35 @@ public static function bytesArray($str, $format = 'x%02x')
}

/**
* @param string $str
* @param string $joiner string to join bytes of $str
* @returns string of bytes of $str represented in decimal format
* @return string of bytes of $str represented in decimal format
* @uses decArray()
*/
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for decString, but the $str parameter is still present in the function signature. Please restore the @param tag to maintain accurate documentation.

     * @param string $str

public static function decString($str, $joiner = ' ')
public static function decString(string $str, string $joiner = ' '): string
{
return implode($joiner, self::decArray($str));
}

/**
* @param string $str
* @returns string[] array of bytes of $str represented in decimal format ('%3d')
* @return string[] array of bytes of $str represented in decimal format ('%3d')
*/
public static function decArray($str)
public static function decArray(string $str): array
{
return self::bytesArray($str, '%3d');
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for decArray, but the $str parameter is still present in the function signature. This discrepancy should be corrected.

     * @param string $str

}

/**
* @param string $str
* @param string $format one of 'ctrl', 'hex', or 'dec'.
* See {@link self::asciiArray()} for more description
* @param string $joiner
* @returns string of bytes joined by $joiner
* @return string of bytes joined by $joiner
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $joiner tag was removed from the PHPDoc for asciiString, but the $joiner parameter is still present in the function signature. This should be re-added.

     * @param string $joiner

* @uses asciiArray()
*/
public static function asciiString($str, $format = 'ctrl', $joiner = ' ')
public static function asciiString(string $str, string $format = 'ctrl', string $joiner = ' '): string
{
return implode($joiner, self::asciiArray($str, $format));
}

/**
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param string $str tag was removed from the PHPDoc for asciiArray, but the $str parameter is still present in the function signature. Please add it back.

     * @param string $str

* @param string $str
* @param string $format one of 'ctrl', 'hex', or 'dec' for control,
* hexadecimal, or decimal format for bytes.
* - ctrl: ASCII control characters represented as text.
Expand All @@ -142,16 +134,16 @@ public static function asciiString($str, $format = 'ctrl', $joiner = ' ')
* others are represented as a decimal ('%03d')
* - hex: bytes represented in hexadecimal ('%02X')
* - dec: bytes represented in decimal ('%03d')
* @returns string[] array of bytes represented in the given format.
* @throws AvroException
* @return string[] array of bytes represented in the given format.
*/
public static function asciiArray($str, $format = 'ctrl')
public static function asciiArray(string $str, string $format = 'ctrl'): array
{
if (!in_array($format, ['ctrl', 'hex', 'dec'])) {
throw new AvroException('Unrecognized format specifier');
}

$ctrl_chars = [
$ctrlChars = [
'NUL',
'SOH',
'STX',
Expand Down Expand Up @@ -191,15 +183,15 @@ public static function asciiArray($str, $format = 'ctrl')
if ($db < 32) {
switch ($format) {
case 'ctrl':
$x[] = str_pad($ctrl_chars[$db], 3, ' ', STR_PAD_LEFT);
$x[] = str_pad($ctrlChars[$db], 3, ' ', STR_PAD_LEFT);

break;
case 'hex':
$x[] = sprintf("x%02X", $db);

break;
case 'dec':
$x[] = str_pad($db, 3, '0', STR_PAD_LEFT);
$x[] = str_pad((string) $db, 3, '0', STR_PAD_LEFT);

break;
}
Expand All @@ -218,15 +210,15 @@ public static function asciiArray($str, $format = 'ctrl')

break;
case 'dec':
$x[] = str_pad($db, 3, '0', STR_PAD_LEFT);
$x[] = str_pad((string) $db, 3, '0', STR_PAD_LEFT);

break;
}
} else {
if ('hex' === $format) {
$x[] = sprintf("x%02X", $db);
} else {
$x[] = str_pad($db, 3, '0', STR_PAD_LEFT);
$x[] = str_pad((string) $db, 3, '0', STR_PAD_LEFT);
}
}
}
Expand Down
53 changes: 24 additions & 29 deletions lang/php/lib/AvroGMP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,33 @@ class AvroGMP
/**
* @var \GMP memoized GMP resource for zero
*/
private static $gmp_0;
private static \GMP $gmp_0;
/**
* @var \GMP memoized GMP resource for one (1)
*/
private static $gmp_1;
private static \GMP $gmp_1;
/**
* @var \GMP memoized GMP resource for two (2)
*/
private static $gmp_2;
private static \GMP $gmp_2;
/**
* @var \GMP memoized GMP resource for 0x7f
*/
private static $gmp_0x7f;
private static \GMP $gmp_0x7f;
/**
* @var \GMP memoized GMP resource for 64-bit ~0x7f
*/
private static $gmp_n0x7f;
private static \GMP $gmp_n0x7f;
/**
* @var \GMP memoized GMP resource for 64-bits of 1
*/
private static $gmp_0xfs;
private static \GMP $gmp_0xfs;

/**
* @param int|string $n integer (or string representation of integer) to encode
* @return string $bytes of the long $n encoded per the Avro spec
*/
public static function encodeLong($n)
public static function encodeLong(int|string $n): string
{
$g = gmp_init($n);
$g = gmp_xor(
Expand All @@ -77,11 +77,10 @@ public static function encodeLong($n)

/**
* @interal Only works up to shift 63 (doesn't wrap bits around).
* @param int|resource|string $g
* @param int $shift number of bits to shift left
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param int|resource|string $g tag was removed from the PHPDoc for shiftLeft, but the $g parameter is still present in the function signature. Please restore the @param tag with the correct type hint int|string|\GMP.

     * @param int|string|\GMP $g

* @returns resource $g shifted left
* @return \GMP $g shifted left
*/
public static function shiftLeft($g, $shift)
public static function shiftLeft(int|string|\GMP $g, int $shift)
{
if (0 == $shift) {
return $g;
Expand All @@ -104,21 +103,19 @@ public static function shiftLeft($g, $shift)
}

/**
* @param \GMP $g resource
* @return \GMP resource 64-bit two's complement of input.
*/
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param \GMP $g resource tag was removed from the PHPDoc for gmpTwosComplement, but the $g parameter is still present in the function signature. Please restore the @param tag with the correct type hint int|string|\GMP.

     * @param int|string|\GMP $g

public static function gmpTwosComplement($g)
public static function gmpTwosComplement(int|string|\GMP $g)
{
return gmp_neg(gmp_sub(gmp_pow(self::gmp_2(), 64), $g));
}

/**
* Arithmetic right shift
* @param int|resource|string $g
* @param int $shift number of bits to shift right
* @returns resource $g shifted right $shift bits
* @return \GMP $g shifted right $shift bits
*/
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

high

The @param int|resource|string $g tag was removed from the PHPDoc for shiftRight, but the $g parameter is still present in the function signature. Please restore the @param tag with the correct type hint int|string|\GMP.

     * @param int|string|\GMP $g

public static function shiftRight($g, $shift)
public static function shiftRight(int|string|\GMP $g, int $shift)
{
if (0 == $shift) {
return $g;
Expand All @@ -143,13 +140,11 @@ public static function shiftRight($g, $shift)
return $m;
}

// phpcs:enable

/**
* @param int[] $bytes array of ascii codes of bytes to decode
* @return string represenation of decoded long.
*/
public static function decodeLongFromArray($bytes)
public static function decodeLongFromArray(array $bytes): string
{
$b = array_shift($bytes);
$g = gmp_init($b & 0x7F);
Expand All @@ -167,7 +162,7 @@ public static function decodeLongFromArray($bytes)
// phpcs:disable PSR1.Methods.CamelCapsMethodName

/**
* @returns \GMP GMP resource for two (2)
* @return \GMP GMP resource for two (2)
*/
private static function gmp_2()
{
Expand All @@ -179,9 +174,9 @@ private static function gmp_2()
}

/**
* @returns resource GMP resource for 64-bits of 1
* @return \GMP GMP resource for 64-bits of 1
*/
private static function gmp_0xfs()
private static function gmp_0xfs(): \GMP
{
if (!isset(self::$gmp_0xfs)) {
self::$gmp_0xfs = gmp_init('0xffffffffffffffff');
Expand All @@ -191,9 +186,9 @@ private static function gmp_0xfs()
}

/**
* @returns resource GMP resource for one (1)
* @return \GMP GMP resource for one (1)
*/
private static function gmp_1()
private static function gmp_1(): \GMP
{
if (!isset(self::$gmp_1)) {
self::$gmp_1 = gmp_init('1');
Expand All @@ -203,9 +198,9 @@ private static function gmp_1()
}

/**
* @returns resource GMP resource for zero
* @return \GMP GMP resource for zero
*/
private static function gmp_0()
private static function gmp_0(): \GMP
{
if (!isset(self::$gmp_0)) {
self::$gmp_0 = gmp_init('0');
Expand All @@ -215,9 +210,9 @@ private static function gmp_0()
}

/**
* @returns resource GMP resource for 64-bit ~0x7f
* @return \GMP GMP resource for 64-bit ~0x7f
*/
private static function gmp_n0x7f()
private static function gmp_n0x7f(): \GMP
{
if (!isset(self::$gmp_n0x7f)) {
self::$gmp_n0x7f = gmp_init('0xffffffffffffff80');
Expand All @@ -227,9 +222,9 @@ private static function gmp_n0x7f()
}

/**
* @returns resource GMP resource for 0x7f
* @return \GMP GMP resource for 0x7f
*/
private static function gmp_0x7f()
private static function gmp_0x7f(): \GMP
{
if (!isset(self::$gmp_0x7f)) {
self::$gmp_0x7f = gmp_init('0x7f');
Expand Down
Loading