Feature: analyser: make check-throws configurable

[bendavies]

Feature

I would like check-throws to be more configurable in the same way as phpstan: https://phpstan.org/blog/bring-your-exceptions-under-control

Features requested:

1. Unchecked Exceptions Mark specific exception classes as unchecked so they don’t require @throws or handling.

2. Checked Exceptions Mark specific exception classes as checked, requiring documentation and handling.

3. Docblock Validation Errors when a checked exception is thrown but not listed in @throws. Errors when an exception appears in @throws but is never actually thrown.

4. Catch Block Checks Warns when a catch block catches an exception that cannot be thrown. Suppress dead-catch warnings for unchecked exceptions.

5. Implicit Throw Behaviour Controls interpretation of missing @throws:

• true: may throw
• false: assumed not to throw

6. Inline @throws Support Allow annotating specific statements as throwing exceptions.

Example

[bendavies]

just seen you've done some of this in https://github.com/carthage-software/mago/commit/6582fa9fad5f49062d4ab299c6c89d2d44905b47

[azjezz]

i dont think its worth supporting inline throws, or implicit throws, i actually only learned about @throws void yesterday from that blog post, and i dont think it is a good thing to add. the current approach we did in the latest commit about this is IMO enough.

[aszenz]

i dont think its worth supporting inline throws, or implicit throws, i actually only learned about @throws void yesterday from that blog post, and i dont think it is a good thing to add. the current approach we did in the latest commit about this is IMO enough.

throws void is useful if u are implementing an interface, and the interface has a throws clause, but your implementation doesn't throw any exception.

[rodnaph]

This now seems to have been largely added in https://github.com/carthage-software/mago/commit/6582fa9fad5f49062d4ab299c6c89d2d44905b47 (as noted above), but I'm running into a workflow issue with PHPUnit tests so have a question...

When a method throws an exception which needs to be handled (ie. is not ignored by any of the configuration from mago.toml), any PHPUnit test which calls this method will now also need to either handle it, or declare it as thrown. Am I missing a pattern for how this can be better handled?

[rodnaph]

Further to the above, I assumed I would have been able to add @throws to the PHPUnit test, but that does not seem to work (and I don't think it would be good UX to have to add it anyway). I'll keep looking for a better solution...

[rodnaph]

Further to the above, I assumed I would have been able to add @throws to the PHPUnit test, but that does not seem to work (and I don't think it would be good UX to have to add it anyway). I'll keep looking for a better solution...

Scratch that, I must have messed something up I think it does work, apologies.

[azjezz]

@rodnaph you can configure mago to ignore all of phpunit exceptions ( specifically, phpunit root exception, that all others extend ): https://mago.carthage.software/tools/analyzer/configuration-reference#exception-filtering

[rodnaph]

@rodnaph you can configure mago to ignore all of phpunit exceptions ( specifically, phpunit root exception, that all others extend ): https://mago.carthage.software/tools/analyzer/configuration-reference#exception-filtering

Hey, yes I've done that, and added specific class ignores for my application exceptions. I'm now trying to remove my application exceptions from the ignores and ensure they are handled, but these exceptions also get reported from within PHPUnit tests, for example...

<?php

class MyClass {
  /** @throws MyException */
  public function foo(): void {
    // ...
  }
}

class MyClassTest extends \PHPUnit\Framework\TestCase
{
  public function testFooDoesSomething(): void {
    $class = new MyClass();
    $class->foo();
  }
}

This then raises the exception that testFooDoesSomething throws MyException. Which can be handled via:

class MyClassTest extends \PHPUnit\Framework\TestCase
{
  /** @throws MyException */
  public function testFooDoesSomething(): void {
    $class = new MyClass();
    $class->foo();
  }
}

I see it kind of makes sense... but it doesn't seem like great DX, did I miss something? Thanks.

[azjezz]

Hm, we can add configuration to say do not report unhandled exceptions in specific class and its subtypes, so you can configure PHPUnit\Framework\TestCase 🤔