Skip to content

improve phpdoc#5962

Closed
ColonelMoutarde wants to merge 23 commits intoFreshRSS:edgefrom
ColonelMoutarde:chore/improve-phpdoc
Closed

improve phpdoc#5962
ColonelMoutarde wants to merge 23 commits intoFreshRSS:edgefrom
ColonelMoutarde:chore/improve-phpdoc

Conversation

@ColonelMoutarde
Copy link
Contributor

@ColonelMoutarde ColonelMoutarde commented Dec 22, 2023

Changes proposed in this pull request:

  • improve phpdoc

Pull request checklist:

  • clear commit messages
  • code manually tested
  • documentation updated

Alkarex
Alkarex reviewed Dec 22, 2023
View reviewed changes
@ColonelMoutarde ColonelMoutarde marked this pull request as draft December 22, 2023 10:02
@ColonelMoutarde ColonelMoutarde marked this pull request as ready for review December 22, 2023 10:40
ColonelMoutarde
ColonelMoutarde commented Dec 22, 2023
View reviewed changes
Alkarex
Alkarex reviewed Dec 29, 2023
View reviewed changes
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Jan 13, 2024
@Alkarex
PHPStan prepare exceptions
7df4ac0 
Take advantage of
https://phpstan.org/blog/bring-your-exceptions-under-control

Minimum changes to pass `tooWideThrowType` and `implicitThrows`.

Revert some mistakes from:
FreshRSS#5504
Preparation needed before new PRs of the same type:
FreshRSS#5962

Fix several wrong PHPDocs and catches:

> Method ... has ...Exception in PHPDoc @throws tag but it's not thrown.

> Dead catch - ...Exception is never thrown in the try block.
@Alkarex Alkarex mentioned this pull request Jan 13, 2024
Merged
@Alkarex
Copy link
Member

Alkarex commented Jan 13, 2024

Let's be a bit more methodic with #6037 first

@Alkarex Alkarex added the System care Everything related to system care label Jan 13, 2024
@Alkarex Alkarex added this to the 1.24.0 milestone Jan 13, 2024
Alkarex added a commit that referenced this pull request Jan 15, 2024
@Alkarex
PHPStan prepare exceptions (#6037)
314077a 
Take advantage of
https://phpstan.org/blog/bring-your-exceptions-under-control

Minimum changes to pass `tooWideThrowType` and `implicitThrows`.

Revert some mistakes from:
#5504
Preparation needed before new PRs of the same type:
#5962

Fix several wrong PHPDocs and catches:

> Method ... has ...Exception in PHPDoc @throws tag but it's not thrown.

> Dead catch - ...Exception is never thrown in the try block.
Alkarex
Alkarex reviewed Jan 17, 2024
View reviewed changes
Alkarex
Alkarex reviewed Jan 17, 2024
View reviewed changes
@Alkarex
Copy link
Member

Alkarex commented Jan 17, 2024

Adding all the @throws can probably be done automatically. It would be nice to search for a tool instead of doing too much by hand.
Maybe https://phpstan.org/blog/introducing-phpstan-pro for instance
Input / tests welcome

@Alkarex Alkarex modified the milestones: 1.24.0, 1.25.0 Apr 6, 2024
@Alkarex
Copy link
Member

Alkarex commented Jun 18, 2024 •
edited
Loading

To supplement my previous messages, I would like a more structured approach than random / partial changes in random files.

For instance, taking one of the files modified with @throws, many are missing. So either we do it fully or we do not. At least at file level, or for one type of precise exception.

Setting to true the following parameter:

FreshRSS/phpstan.neon

Line 42 in 7a9a24a

booleansInConditions: false # TODO pass

this gives:

$ vendor/bin/phpstan analyse app/Controllers/configureController.php
Note: Using configuration file /home/alexandre/GitHub/FreshRSS/phpstan.neon.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   configureController.php                                                                                                                                              
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  :175   Method FreshRSS_configure_Controller::integrationAction() throws checked exception Minz_ConfigurationException but it's missing from the PHPDoc @throws tag.         
  :176   Method FreshRSS_configure_Controller::integrationAction() throws checked exception Minz_ConfigurationException but it's missing from the PHPDoc @throws tag.         
  :251   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :252   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :260   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :261   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :262   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :273   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :274   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :283   Method FreshRSS_configure_Controller::archivingAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.            
  :285   Method FreshRSS_configure_Controller::archivingAction() throws checked exception Minz_ConfigurationNamespaceException but it's missing from the PHPDoc @throws tag.  
  :285   Method FreshRSS_configure_Controller::archivingAction() throws checked exception Minz_PDOConnectionException but it's missing from the PHPDoc @throws tag.           
  :288   Method FreshRSS_configure_Controller::archivingAction() throws checked exception Minz_ConfigurationNamespaceException but it's missing from the PHPDoc @throws tag.  
  :288   Method FreshRSS_configure_Controller::archivingAction() throws checked exception Minz_PDOConnectionException but it's missing from the PHPDoc @throws tag.           
  :309   Method FreshRSS_configure_Controller::queriesAction() throws checked exception Minz_ConfigurationException but it's missing from the PHPDoc @throws tag.             
  :326   Method FreshRSS_configure_Controller::queriesAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.              
  :327   Method FreshRSS_configure_Controller::queriesAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.              
  :332   Method FreshRSS_configure_Controller::queriesAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.              
  :364   Method FreshRSS_configure_Controller::queryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.                
  :369   Method FreshRSS_configure_Controller::queryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.                
  :400   Method FreshRSS_configure_Controller::queryAction() throws checked exception Minz_ConfigurationException but it's missing from the PHPDoc @throws tag.               
  :415   Method FreshRSS_configure_Controller::queryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.                
  :417   Method FreshRSS_configure_Controller::queryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.                
  :418   Method FreshRSS_configure_Controller::queryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.                
  :431   Method FreshRSS_configure_Controller::deleteQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.          
  :436   Method FreshRSS_configure_Controller::deleteQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.          
  :438   Method FreshRSS_configure_Controller::deleteQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.          
  :439   Method FreshRSS_configure_Controller::deleteQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.          
  :453   Method FreshRSS_configure_Controller::bookmarkQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.        
  :459   Method FreshRSS_configure_Controller::bookmarkQueryAction() throws checked exception Minz_ConfigurationException but it's missing from the PHPDoc @throws tag.       
  :463   Method FreshRSS_configure_Controller::bookmarkQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.        
  :464   Method FreshRSS_configure_Controller::bookmarkQueryAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.        
  :493   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
  :498   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
  :499   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
  :500   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
  :501   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
  :502   Method FreshRSS_configure_Controller::systemAction() throws checked exception FreshRSS_Context_Exception but it's missing from the PHPDoc @throws tag.               
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 


                                                                                                                        
 [ERROR] Found 38 errors

Furthermore, some of those insights might indicate that we need to think how we handle exceptions in some cases.

P.S.: 889 "errors" of this type in the whole project

Alkarex
Alkarex reviewed Aug 7, 2024
View reviewed changes
app/Controllers/statsController.php
Comment on lines +71 to +72
$iMax = count($feedByCategory_calculated);
for ($i = 0; $i < $iMax; $i++) {
Copy link
Member

@Alkarex Alkarex Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not correspond to the topic of the PR on improving PHPDoc (which was already quite vague).
I am not so found of PRs with various random changes, and prefer focussed PRs doing one thing well

@Alkarex Alkarex modified the milestones: 1.25.0, 1.24.3 Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alkarex Alkarex Alkarex left review comments

Assignees

No one assigned

Labels

System care Everything related to system care

Projects

None yet

Milestone

1.24.3

Development

Successfully merging this pull request may close these issues.

3 participants

@ColonelMoutarde @Alkarex @math-GH