Cell::setValueExplicit raises "Unhandled Exception" warning

[PrOF-kk]

This is:

- [x] a bug report
- [ ] a feature request
- [x] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

setValueExplicit raises an exception like LogicException or similar, which are considered unchecked by most IDEs

What is the current behavior?

IDE (PhpStorm 2023.2.2) warns about "Unhandled \PhpOffice\PhpSpreadsheet\Exception"

What are the steps to reproduce?

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require __DIR__ . '/vendor/autoload.php';

$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();

$spreadsheet->getActiveSheet()->getCell('A1')->setValueExplicit(42, \PhpOffice\PhpSpreadsheet\Cell\DataType::TYPE_NUMERIC);

If this is an issue with reading a specific spreadsheet file, then it may be appropriate to provide a sample file that demonstrates the problem; but please keep it as small as possible, and sanitize any confidential information before uploading.

What features do you think are causing the issue

• Reader
• Writer
• Styles
• Data Validations
• Formula Calculations
• Charts
• AutoFilter
• Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

Yes

Which versions of PhpSpreadsheet and PHP are affected?

Using 1.29.0

[oleibman]

Not a PhpStorm user, so you'll have to do some detective work for me. First, I'll point out that using setValueExplicit here is probably unnecessary if you are using the default value binder - you could more easily use:

setValue(42);

I'll get back to your original statement in a moment. If you use setValue, does the IDE accept that statement without warning?

If it does, the difference between setValue and setValueExplicit regarding Exceptions is that the setValue doc block contains an @throws annotation and setValueExplicit does not. If you add that annotation to setValueExplicit's doc block and restore your original statement, does the IDE now accept it without warning? Is that true regardless of whether your statement is found in a try/catch block (I can't tell if that's the case from your snippet)?

[PrOF-kk]

Hello, thanks for taking a look.

setValue(42);

This is in the context of a custom (simple) value binder, so I can't use setValue()

Does the IDE accept that statement without warning?

No, that raises the same warning.

---

As far as I understand it, PhpStorm doesn't trust the doc block if there's no @throws, and does some code analysis on its own (https://blog.jetbrains.com/phpstorm/2018/04/configurable-unchecked-exceptions/).
So if I manually modify Cell::setValue and lie in the doc block by replacing @throws Exception with @throws \InvalidArgumentException the warning is not raised anymore, so I think the problem is that PhpSpreadsheet\Exception is considered a checked exception instead of an unchecked one.

[oleibman]

From the link to which you pointed, it seems like you can configure your own unchecked exceptions list. Can you not configure it to treat \PhpOffice\PhpSpreadsheet\Exception as unchecked?

If you can't, I think you need to check with PhpStorm about what the "correct" solution would be here. In principle, I don't mind adding or changing the @throws annotation, but I'm not willing to declare it as something which is clearly not true.

[PrOF-kk]

Can you not configure it to treat \PhpOffice\PhpSpreadsheet\Exception as unchecked?

I could, but then the IDE wouldn't warn me about any \PhpOffice\PhpSpreadsheet\Exception, even where enforcing a try-catch makes sense.

In principle, I don't mind adding or changing the @throws annotation, but I'm not willing to declare it as something which is clearly not true.

Of course, I agree.

I think using InvalidArgumentException or extending it (or LogicException) might be a clean solution

[oleibman]

PhpOffice\PhpSpreadsheet\Exception covers more than invalid arguments, and LogicException seems misleading. If you changePhpOffice\PhpSpreadsheet\Exception to extend RuntimeException rather than Exception, would that help? Would it require you to wrap all setValueExplicit and setValue calls in try-catch (which I would definitely consider a problem)? Please note that I will probably not be able to see your reply for a couple of weeks after today.

[PrOF-kk]

PhpOffice\PhpSpreadsheet\Exception covers more than invalid arguments

Yes, it's used as a catch-all generic "Exception from PhpSpreadsheet" as far as I can tell

If you changePhpOffice\PhpSpreadsheet\Exception to extend RuntimeException rather than Exception, would that help?

It doesn't raise any warnings from the IDE anymore

Would it require you to wrap all setValueExplicit and setValue calls in try-catch (which I would definitely consider a problem)?

No.
Basically the distinction between checked and unchecked exceptions is a convention from the Java world.

Please note that I will probably not be able to see your reply for a couple of weeks after today.

No worries!

[oleibman]

@PrOF-kk Because I am not a PhpStorm user, please verify that PR #3765 meets your needs and doesn't cause any new problems. I will merge it when you confirm.

[PrOF-kk]

Whoops, I didn't get a notification, sorry. I'll get to it as soon as I can, probably Monday

[PrOF-kk]

I finally got composer to install the right branch and tested it. No more warnings, thanks!