Skip to content
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.

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

ENH Fix deprecation issues for PHP 8.1 compatibility #10273

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class GeocodableDataObject implements ModelTypePlugin
);

// only apply the plugin to geocodable DataObjects
if (!Extensible::has_extension($class, Geocodable::class)) {
if (!ViewableData::has_extension($class, Geocodable::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest using this syntax instead because ViewableData does not inherently have anything to do with the current context. But in the grander scheme, it doesn't really matter.

Suggested change
if (!ViewableData::has_extension($class, Geocodable::class)) {
if (!$class::has_extension(Geocodable::class)) {

Copy link
Member Author

@emteknetnz emteknetnz Apr 6, 2022

Choose a reason for hiding this comment

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

We don't know ahead of time if $class has the Extensible trait, so we'd need something like this

method_exists($class, 'has_extension') && call_user_func("$class::has_extension", $class, Geocodable::class)

Which is very verbose. Therefore I think it's better to simply use

ViewableData::has_extension($class, Geocodable::class)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair point.

return;
}

Expand Down Expand Up @@ -85,7 +85,7 @@ class GeocodableQuery implements ModelQueryPlugin
{
$class = $query->getModel()->getSourceClass();
// Only apply to geocodable objects
if (!Extensible::has_extension($class, Geocodable::class)) {
if (!ViewableData::has_extension($class, Geocodable::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!ViewableData::has_extension($class, Geocodable::class)) {
if (!$class::has_extension(Geocodable::class)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

As above

return;
}

Expand Down Expand Up @@ -177,7 +177,7 @@ public function apply(ModelQuery $query, Schema $schema, array $config = []): vo
{
$class = $query->getModel()->getSourceClass();
// Only apply to geocodable objects
if (!Extensible::has_extension($class, Geocodable::class)) {
if (!ViewableData::has_extension($class, Geocodable::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!ViewableData::has_extension($class, Geocodable::class)) {
if (!$class::has_extension(Geocodable::class)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

As above

return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Control/HTTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public static function setGetVar($varname, $varvalue, $currentURL = null, $separ
$path = (isset($parts['path']) && $parts['path'] != '') ? $parts['path'] : '';

// handle URL params which are existing / new
$params = ($params) ? '?' . http_build_query($params, null, $separator) : '';
$params = ($params) ? '?' . http_build_query($params, '', $separator) : '';

// keep fragments (anchors) intact.
$fragment = (isset($parts['fragment']) && $parts['fragment'] != '') ? '#' . $parts['fragment'] : '';
Expand Down
2 changes: 1 addition & 1 deletion src/Control/HTTPResponse_Exception.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function __construct($body = null, $statusCode = null, $statusDescription
$this->setResponse($response);
}

parent::__construct($this->getResponse()->getBody(), $this->getResponse()->getStatusCode());
parent::__construct((string) $this->getResponse()->getBody(), $this->getResponse()->getStatusCode());
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/Core/ClassInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use SilverStripe\Dev\Deprecation;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DB;
use SilverStripe\View\ViewableData;

/**
* Provides introspection information about the class tree.
Expand Down Expand Up @@ -597,7 +598,7 @@ public static function classesWithExtension(

// only keep classes with the Extension applied
$classes = array_filter($classes, function ($class) use ($extensionClass) {
return Extensible::has_extension($class, $extensionClass);
return ViewableData::has_extension($class, $extensionClass);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return ViewableData::has_extension($class, $extensionClass);
return $class::has_extension($extensionClass);

Copy link
Member Author

Choose a reason for hiding this comment

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

As above

});

return $classes;
Expand Down
1 change: 0 additions & 1 deletion src/Dev/CSVParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ public function provideHeaderRow($headerRow)
*/
protected function openFile()
{
ini_set('auto_detect_line_endings', 1);
$this->fileHandle = fopen($this->filename, 'r');

if ($this->providedHeaderRow) {
Expand Down
11 changes: 0 additions & 11 deletions src/Dev/CsvBulkLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ public function preview($filepath)
*/
protected function processAll($filepath, $preview = false)
{
$previousDetectLE = ini_get('auto_detect_line_endings');
ini_set('auto_detect_line_endings', true);

$this->extend('onBeforeProcessAll', $filepath, $preview);

$result = BulkLoader_Result::create();
Expand Down Expand Up @@ -144,8 +141,6 @@ protected function processAll($filepath, $preview = false)
$failedMessage = sprintf($failedMessage . " because %s", $e->getMessage());
}
print $failedMessage . PHP_EOL;
} finally {
ini_set('auto_detect_line_endings', $previousDetectLE);
}

$this->extend('onAfterProcessAll', $result, $preview);
Expand Down Expand Up @@ -182,9 +177,6 @@ protected function getNormalisedColumnMap()
protected function splitFile($path, $lines = null)
{
Deprecation::notice('5.0', 'splitFile is deprecated, please process files using a stream');
$previous = ini_get('auto_detect_line_endings');

ini_set('auto_detect_line_endings', true);

if (!is_int($lines)) {
$lines = $this->config()->get("lines");
Expand Down Expand Up @@ -230,11 +222,8 @@ protected function splitFile($path, $lines = null)
$count = 0;
}
}

fclose($to);

ini_set('auto_detect_line_endings', $previous);

return $files;
}

Expand Down
2 changes: 1 addition & 1 deletion src/ORM/FieldType/DBDecimal.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public function prepValueForDB($value)
return 0;
}

if (ctype_digit($value)) {
if (ctype_digit((string) $value)) {
return (int)$value;
}

Expand Down
4 changes: 2 additions & 2 deletions src/ORM/Hierarchy/Hierarchy.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use SilverStripe\Admin\LeftAndMain;
use SilverStripe\Control\Controller;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Extensible;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\SS_List;
use SilverStripe\ORM\ValidationResult;
Expand All @@ -17,6 +16,7 @@
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
use Exception;
use SilverStripe\View\ViewableData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use SilverStripe\View\ViewableData;

Copy link
Member Author

Choose a reason for hiding this comment

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

As above


/**
* DataObjects that use the Hierarchy extension can be be organised as a hierarchy, with children and parents. The most
Expand Down Expand Up @@ -418,7 +418,7 @@ private function getHierarchyBaseClass(): string
{
$ancestry = ClassInfo::ancestry($this->owner);
$ancestorClass = array_shift($ancestry);
while ($ancestorClass && !Extensible::has_extension($ancestorClass, self::class)) {
while ($ancestorClass && !ViewableData::has_extension($ancestorClass, self::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
while ($ancestorClass && !ViewableData::has_extension($ancestorClass, self::class)) {
while ($ancestorClass && !$ancestorClass::has_extension(self::class)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

As above

$ancestorClass = array_shift($ancestry);
}

Expand Down
2 changes: 1 addition & 1 deletion src/View/Parsers/HTMLValue.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public function __call($method, $arguments)
{
$doc = $this->getDocument();

if (method_exists($doc, $method)) {
if ($doc && method_exists($doc, $method)) {
return call_user_func_array([$doc, $method], $arguments);
} else {
return parent::__call($method, $arguments);
Expand Down
9 changes: 5 additions & 4 deletions tests/php/Forms/DatetimeFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace SilverStripe\Forms\Tests;

use IntlDateFormatter;
use SilverStripe\Control\Controller;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\Forms\DatetimeField;
Expand Down Expand Up @@ -256,25 +257,25 @@ public function testValidateMinDateStrtotime()
{
$f = new DatetimeField('Datetime');
$f->setMinDatetime('-7 days');
$f->setValue(strftime('%Y-%m-%d %T', strtotime('-8 days', DBDatetime::now()->getTimestamp())));
$f->setValue(date('Y-m-d H:i:s', strtotime('-8 days', DBDatetime::now()->getTimestamp())));
$this->assertFalse($f->validate(new RequiredFields()), 'Date below min datetime, with strtotime');

$f = new DatetimeField('Datetime');
$f->setMinDatetime('-7 days');
$f->setValue(strftime('%Y-%m-%d %T', strtotime('-7 days', DBDatetime::now()->getTimestamp())));
$f->setValue(date('Y-m-d H:i:s', strtotime('-7 days', DBDatetime::now()->getTimestamp())));
$this->assertTrue($f->validate(new RequiredFields()), 'Date matching min datetime, with strtotime');
}

public function testValidateMaxDateStrtotime()
{
$f = new DatetimeField('Datetime');
$f->setMaxDatetime('7 days');
$f->setValue(strftime('%Y-%m-%d %T', strtotime('8 days', DBDatetime::now()->getTimestamp())));
$f->setValue(date('Y-m-d H:i:s', strtotime('8 days', DBDatetime::now()->getTimestamp())));
$this->assertFalse($f->validate(new RequiredFields()), 'Date above max date, with strtotime');

$f = new DatetimeField('Datetime');
$f->setMaxDatetime('7 days');
$f->setValue(strftime('%Y-%m-%d %T', strtotime('7 days', DBDatetime::now()->getTimestamp())));
$f->setValue(date('Y-m-d H:i:s', strtotime('7 days', DBDatetime::now()->getTimestamp())));
$this->assertTrue($f->validate(new RequiredFields()), 'Date matching max date, with strtotime');
}

Expand Down
4 changes: 2 additions & 2 deletions tests/php/Forms/FormTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,8 @@ protected function clean($input)
{
return str_replace(
[
html_entity_decode(' ', null, 'UTF-8'),
html_entity_decode(' ', null, 'UTF-8'), // narrow non-breaking space
html_entity_decode(' ', 0, 'UTF-8'),
html_entity_decode(' ', 0, 'UTF-8'), // narrow non-breaking space
],
' ',
trim($input)
Expand Down
4 changes: 2 additions & 2 deletions tests/php/Forms/NumericFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ protected function clean($input)
{
return str_replace(
[
html_entity_decode(' ', null, 'UTF-8'),
html_entity_decode(' ', null, 'UTF-8'), // narrow non-breaking space
html_entity_decode(' ', 0, 'UTF-8'),
html_entity_decode(' ', 0, 'UTF-8'), // narrow non-breaking space
],
' ',
trim($input)
Expand Down
2 changes: 1 addition & 1 deletion tests/php/ORM/DBMoneyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public function testHasAmount()
*/
protected function clean($input)
{
$nbsp = html_entity_decode(' ', null, 'UTF-8');
$nbsp = html_entity_decode(' ', 0, 'UTF-8');
return str_replace(' ', $nbsp, trim($input));
}
}