Skip to content

Allow true return annotation in upgrade.php function#371

Open
daniil-berg wants to merge 1 commit intomoodlehq:mainfrom
daniil-berg:fix/savepoints-allow-true
Open

Allow true return annotation in upgrade.php function#371
daniil-berg wants to merge 1 commit intomoodlehq:mainfrom
daniil-berg:fix/savepoints-allow-true

Conversation

@daniil-berg
Copy link

Technically, the xmldb_..._upgrade function in db/upgrade.php always returns true. Since PHP 8.2 you can actually annotate it accordingly. I discovered that doing so causes this weird warning from the savepoints checker:

    + WARN: Detected fewer 'if' blocks (0) than 'savepoint' calls (1). Repeated savepoints?
    + NOTE: cannot find 'if' blocks within the upgrade function

This PR just extends the function RegEx to also match a true return type annotation, not just one with bool. (Also being more lenient with the whitespace here would not hurt since this is not a style checker.)

It seems tests/check_upgrade_savepoints.bats does the testing and there already is the tests/fixtures/check_upgrade_savepoints/returning_bool.patch fixture for bool return types. But I have never seen this type of testing setup before, so I am not sure how best to add a corresponding test case.

If you tell me how I should go about it, I am sure adding a test case for this should not be too difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant