Skip to content

Xlsx Reader Default Cell Type #4139

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

Merged
merged 4 commits into from
Aug 24, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).

### Changed

- Xlsx Reader default datatype when none is specified in Xml is changed from string to numeric, which is how Excel treats it. There is expected to be little impact because DefaultValueBinder and AdvancedValueBinder correct mis-identification as string, and StringValueBinder usually expects string. [PR #4139](https://github.com/PHPOffice/PhpSpreadsheet/pull/4139)
- Currency and Accounting Wizards are changed to act like Excel, and a new CurrencyBase Wizard is added for for non-Excel formats. [Issue #4125](https://github.com/PHPOffice/PhpSpreadsheet/issues/4125) [Issue #4124](https://github.com/PHPOffice/PhpSpreadsheet/issues/4124) [PR #4127](https://github.com/PHPOffice/PhpSpreadsheet/pull/4127)

### Deprecated
Expand Down
54 changes: 36 additions & 18 deletions src/PhpSpreadsheet/Reader/Xlsx.php
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet

// Read cell!
switch ($cellDataType) {
case 's':
case DataType::TYPE_STRING:
if ((string) $c->v != '') {
$value = $sharedStrings[(int) ($c->v)];

Expand All @@ -858,7 +858,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
}

break;
case 'b':
case DataType::TYPE_BOOL:
if (!isset($c->f) || ((string) $c->f) === '') {
if (isset($c->v)) {
$value = self::castToBoolean($c);
Expand All @@ -869,22 +869,29 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
} else {
// Formula
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToBoolean');
if (isset($c->f['t'])) {
$att = $c->f;
$docSheet->getCell($r)->setFormulaAttributes($att);
}
self::storeFormulaAttributes($c->f, $docSheet, $r);
}

break;
case DataType::TYPE_STRING2:
if (isset($c->f)) {
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToString');
self::storeFormulaAttributes($c->f, $docSheet, $r);
} else {
$value = self::castToString($c);
}

break;
case 'inlineStr':
case DataType::TYPE_INLINE:
if (isset($c->f)) {
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToError');
self::storeFormulaAttributes($c->f, $docSheet, $r);
} else {
$value = $this->parseRichText($c->is);
}

break;
case 'e':
case DataType::TYPE_ERROR:
if (!isset($c->f)) {
$value = self::castToError($c);
} else {
Expand All @@ -902,20 +909,16 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
default:
if (!isset($c->f)) {
$value = self::castToString($c);
if (is_numeric($value)) {
$value += 0;
}
} else {
// Formula
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToString');
$formulaAttributes = [];
$attributes = $c->f->attributes();
if (isset($attributes['t'])) {
$formulaAttributes['t'] = (string) $attributes['t'];
}
if (isset($attributes['ref'])) {
$formulaAttributes['ref'] = (string) $attributes['ref'];
}
if (!empty($formulaAttributes)) {
$docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
if (is_numeric($calculatedValue)) {
$calculatedValue += 0;
}
self::storeFormulaAttributes($c->f, $docSheet, $r);
}

break;
Expand Down Expand Up @@ -2376,6 +2379,21 @@ private function processIgnoredErrors(SimpleXMLElement $xml, Worksheet $sheet):
}
}

private static function storeFormulaAttributes(SimpleXMLElement $f, Worksheet $docSheet, string $r): void
{
$formulaAttributes = [];
$attributes = $f->attributes();
if (isset($attributes['t'])) {
$formulaAttributes['t'] = (string) $attributes['t'];
}
if (isset($attributes['ref'])) {
$formulaAttributes['ref'] = (string) $attributes['ref'];
}
if (!empty($formulaAttributes)) {
$docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
}
}

private static function onlyNoteVml(string $data): bool
{
$data = str_replace('<br>', '<br/>', $data);
Expand Down
85 changes: 85 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/NumericCellTypeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

declare(strict_types=1);

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
use PHPUnit\Framework\TestCase;

class NumericCellTypeTest extends TestCase
{
private IValueBinder $oldBinder;

private string $filename = '';

protected function setUp(): void
{
$this->oldBinder = Cell::getValueBinder();

$binder = new class () implements IValueBinder {
public function bindValue(Cell $cell, mixed $value): bool
{
if (is_float($value) || is_int($value)) {
$type = DataType::TYPE_NUMERIC;
} elseif (is_string($value)) {
$type = DataType::TYPE_STRING;
} else {
return false;
}

$cell->setValueExplicit($value, $type);

return true;
}
};

Cell::setValueBinder($binder);
}

protected function tearDown(): void
{
Cell::setValueBinder($this->oldBinder);
if ($this->filename !== '') {
unlink($this->filename);
$this->filename = '';
}
}

public function testCellShouldHaveNumericTypeAttribute(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$array = [
['1.0'],
[1.0],
['-1.0'],
[-1.0],
['0'],
[0],
['0.0'],
[0.0],
['1e1'],
[1e1],
];
$sheet->fromArray($array, null, 'A1', true);

$this->filename = File::temporaryFilename();
$writer = new Writer($spreadsheet);
$writer->save($this->filename);
$spreadsheet->disconnectWorksheets();

$reader = new Reader();
$spreadsheet2 = $reader->load($this->filename);

$sheet2 = $spreadsheet2->getActiveSheet();
self::assertSame($array, $sheet2->toArray(null, false, false));
$spreadsheet2->disconnectWorksheets();
}
}
Loading