Skip to content

Conversation

@cviebrock
Copy link
Contributor

@cviebrock cviebrock commented Oct 3, 2025

Fixes all level 1 issues and those that are caught with checkFunctionArgumentTypes: true

... with one exception that I'm sure how best to fix:

 ------ --------------------------------------------------------------------------------- 
  Line   Swat/SwatCascadeFlydown.php                                                      
 ------ --------------------------------------------------------------------------------- 
  169    Parameter #2 $title of class SwatOption constructor expects string, null given.  
         🪪 argument.type                                                                 
         ✏️ Swat/SwatCascadeFlydown.php:169                                               
 ------ --------------------------------------------------------------------------------- 

Instead of doing $options = [new SwatOption(null, null)]; could we just do $options = [];? If not, then we would need to change the constructor of SwatOption to allow a null title.

✅ Switched to use SwatOption(null, '') now as it matches other places in the code.

 ------ ---------------------------------------- 
  Line   Swat/SwatTableViewInputRow.php          
 ------ ---------------------------------------- 
  680    Variable $column might not be defined.  
         🪪 variable.undefined                   
         ✏️ Swat/SwatTableViewInputRow.php:680   
 ------ ---------------------------------------- 

❓ My guess is that somewhere in SwatTableViewInputRow::displayEnterAnotherRow() we should return early (and not output anything?) if $columns is empty.

@cviebrock cviebrock force-pushed the phpstan-level-1-issues branch from 27da9dd to 5e5780d Compare October 3, 2025 21:56
@cviebrock cviebrock marked this pull request as ready for review October 3, 2025 22:20
@fokked
Copy link

fokked commented Oct 6, 2025

@gauthierm
Copy link
Member

Instead of doing $options = [new SwatOption(null, null)]; could we just do $options = [];? If not, then we would need to change the constructor of SwatOption to allow a null title.

I think we have a blank option for browser compatibility. At the time it was written, it's likely that a select with no elements did not render properly. The code in SwatFlydown will only render a select if there are >= 1 options.

@cviebrock
Copy link
Contributor Author

Then I'd suggest we do the same thing as we do when we want a blank option, and just do new SwatOption(null, '') (or maybe even SwatFlydownBlankOption(null, '')?)

@gauthierm
Copy link
Member

cviebrock#2

@gauthierm gauthierm merged commit f21ff82 into silverorange:master Oct 6, 2025
2 checks passed
@cviebrock cviebrock deleted the phpstan-level-1-issues branch October 6, 2025 18:55
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.

3 participants