Heads up: ongoing attempt to comply with phpstan rule level 6

@wn_name is working on bringing tt-rss codebase to phpstan rule level 6 which involves enforcing much stricter type checking on everything, mostly function parameters and return values.

it’s a huge invasive change which has the potential to cause many issues and we’re not sure when it gets into master, still testing from any interested people would be much appreciated.

working branch is here - https://git.tt-rss.org/fox/tt-rss/src/branch/wip-phpstan-level6

update: it’s becoming increasingly likely that this will end up in master. there’s about 700 warnings left to resolve.

there are, however, IMPORTANT CHANGES related to PLUGINS: all hook function prototypes are now defined in base Plugin class and should be implemented verbatim, including argument lists, because PHP doesn’t support function argument overloading.

any plugins implementing hook functions with wrong argument lists would CAUSE A STARTUP CRASH (PHP E_COMPILE_ERROR) which seems to be impossible to properly catch.

current shitty workaround is blacklisting this plugin from loading any further within login session and enabling general tt-rss safe mode. which should get things going after a reload (or three).

i encourage all third party plugin authors who are reading this message be ready for the inevitable™ and verify that their implemented hook methods match prototypes defined here:

https://git.tt-rss.org/fox/tt-rss/src/branch/wip-phpstan-level6/classes/plugin.php#L104

current definitions are not necessarily final (I haven’t went through them all), some further revisions might be made if I made a mistake while getting definitions from pluginhost.

All PHPStan warnings up to and including level 6 have been addressed, so now might be an ideal time for people interested in helping test (which will help ensure these changes don’t break stuff for you) to do so.

:+1:

i’m finishing with the unbundled plugins. about 20 warnings left.

i was originally planning to merge this next week but it seems that this went a lot faster than expected. i’m not sure if plugin authors would actually wake up before we go and break their stuff (:smiling_imp:) anyway so merge might actually happen a lot sooner, maybe thursday, unless major issues are discovered.

merge is planned for tomorrow. other new things:

  • Dojo/Dijit updated to 1.16.4
  • auto-generated documentation (its pretty bare right now, but after merge a lot of information related to plugin development would appear there)
  • some minor dependency updates from composer

Just for reference:

Hello,

ist this related to my actual problem with TT-RSS in docker? Here a part from the logfile of the container ttrss-docker_app_1. What do I have to do to start the container successfully again?

updating plugins.local/nginx_xaccel...
From https://git.tt-rss.org/fox/ttrss-nginx-xaccel
 * branch            master     -> FETCH_HEAD
Already up to date.
NOTICE:  extension "pg_trgm" already exists, skipping
PHP Fatal error:  Declaration of mailer_smtp::init(PluginHost $host) must be compatible with Plugin::init($host) in /var/www/html/tt-rss/plugins.local/mailer_smtp/init.php on line 23
db:5432 - accepting connections
WARNING: ca-certificates.crt does not contain exactly one certificate or CRL: skipping
updating tt-rss source in /var/www/html/tt-rss from https://git.tt-rss.org/fox/tt-rss.git...
From https://git.tt-rss.org/fox/tt-rss
 * branch                master     -> FETCH_HEAD

Solution: I manually deleted the plugin mailer_smtp

you could’ve updated it instead. :man_shrugging:

e: maybe docker container should try updating all plugins.local on startup, not just nginx_xaccel? this would increase startup time but would prevent issues such as this one.

https://git.tt-rss.org/fox/ttrss-docker-compose/commit/bd0c7ebf0ee0d2f147f2567d4a46d5237925de35 + same for docker hub image

thanks for the reply - removing from the docker volume was the easist way. I have reinstalled the plugin as the container was able to start again.

This is for a docker install, and I think I’m on the latest, or at least I did a new pull. I updated or removed plugins as necessary, and now everything starts and seems to run fine with the web interface.

When using the android app and doing “Mark all read” I get this error

app_1      | [18-Nov-2021 16:49:51 UTC] PHP Fatal error:  Uncaught TypeError: Fe
eds::_catchup(): Argument #2 ($cat_view) must be of type bool, null given, calle
d in /var/www/html/tt-rss/classes/api.php on line 426 and defined in /var/www/html/tt-rss/classes/feeds.php:755
app_1      | Stack trace:
app_1      | #0 /var/www/html/tt-rss/classes/api.php(426): Feeds::_catchup()
app_1      | #1 /var/www/html/tt-rss/api/index.php(55): API->catchupFeed()
app_1      | #2 {main}
app_1      |   thrown in /var/www/html/tt-rss/classes/feeds.php on line 755

Mark all read works in the web interface and the Pluchon tiny Reader IOS app.

Let me know if any additional information would be useful, or if this could be caused by some unique issue on my end.

https://community.tt-rss.org/t/tt-rss-app-mark-as-read/5055

at least check the frontpage before posting

Only 20 minutes before my post! I think I had the edit window open when that post was put up.

Thanks for the quick fix. I was just looking if there was a way to delete my post.

maybe you could help, I think the error appeared after an update in tt-rss

I’m on rpi 4 and up2date master branch. thanks in advance

E_COMPILE_ERROR (64) plugins.local/fever/fever_api.php:113 Declaration of FeverAPI::before($method) must be compatible with Handler::before(string $method): bool

Remote IP: 192.168.1.226
Request URI: /tt-rss/plugins.local/fever/?api&feeds
User agent: Reeder/4020.89.01 CFNetwork/1325.0.1 Darwin/21.1.0

I updated the Fever API Plugin to resolve this earlier today. I opened a pull request for the DigitalDJ repo, but if you want to fix your issue now you can checkout my fork of the API here GitHub - eric-pierce/tinytinyrss-fever-plugin: Tiny Tiny RSS Fever API Plugin

edit: The updates are now merged into master on the DigitalDJ repo

Think I stumbled upon another issue caused by this merge:

Uncaught TypeError: Sanitizer::sanitize(): Argument #2 ($force_remove_images) must be of type bool, null given, called in /var/www/html/tt-rss/classes/feeds.php on line 282 and defined in /var/www/html/tt-rss/classes/sanitizer.php:65
Stack trace:
#0 /var/www/html/tt-rss/classes/feeds.php(282): Sanitizer::sanitize()
#1 /var/www/html/tt-rss/classes/feeds.php(527): Feeds->_format_headlines_list()
#2 /var/www/html/tt-rss/backend.php(136): Feeds->view()
#3 {main}
  thrown

Happens when opening “starred articles” special feed - but only when scrolling through the articles, so I guess this might be an caused by an old starred article which has an unexpected null value for this field, and thus may not be obvious or happening to others … result is “starred articles” not being displayed at all or stopping at a certain article, not loading any more lines, depending on when it hits the affected database record I guess.

My environment: Running dynamic docker setup on RPi 4, on latest version as of today (v21.11-2b4ba59a7). Issue first started showing when this merge happened.

… and since this is my first post here: Thanks fox for this very nice piece of software, which made me learn (and like) docker in the first place :grin:

thanks for reporting, this should fix it:

https://git.tt-rss.org/fox/tt-rss/commit/e7111e4f1422095afcafc92703c4d10dc3358014

Thanks for the quick fix - I updated and tested and the reported error is gone. My starred articles still don’t work though, now it happens a few lines further down in feeds.php:

Uncaught TypeError: sql_bool_to_bool(): Argument #1 ($s) must be of type string, null given, called in /var/www/html/tt-rss/classes/feeds.php on line 300 and defined in /var/www/html/tt-rss/include/functions.php:344
Stack trace:
#0 /var/www/html/tt-rss/classes/feeds.php(300): sql_bool_to_bool()
#1 /var/www/html/tt-rss/classes/feeds.php(527): Feeds->_format_headlines_list()
#2 /var/www/html/tt-rss/backend.php(136): Feeds->view()
#3 {main}
  thrown

I get the feeling that I might have some unusual articles or broken db records in my starred feed? Let me know if I can use this circumstance to test anything else that might not be obvious for this edge case.

my guess would be you opening archived+starred articles, kept from removed feeds. they lack some feed-related database information because the feed is, well, no longer there.

https://git.tt-rss.org/fox/tt-rss/commit/3323ae78ce4e021b4ffc00b96770fc23bbbc8e47

edge cases are a good thing. i completely forgot about archived feed.