* Remove extra space
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove null checks on properties that cannot be null
The value could be null only if the property exists in stdClass
returned by mysqli but is null. Mysqli doesn't do that. Unless the value
can be null in some other way, this check is redundant.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add psalm-return
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove $handle parameter of getUniqueCondition()
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Type hint the $row parameter
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
Co-authored-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
* Refactor getComments()
Use early return and remove redundant if statement.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Fix indentation
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove temporary variable
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant code
Remove redundant if statement.
Break at the end of the loop.
elseif when there is no other choice.
Redundant ! empty().
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Collapse if statement into an elvis operator
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Better type hinting for return array
We can tell Psalm what to expect, but we should declare the variables first.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove dead code
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Use array_keys instead of unused variable
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add type declaration and psalm-return
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add psalm-return to checkChildForeignReferences()
Also added type declaration, and simplified the code
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getChildReferences()
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Improve type inference for foreignDropdown
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* [bugfix] Cast the value to a string to avoid type mismatch.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add proper string type declaration to $foreign_filter
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Use str_starts_with()
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant !empty()
!empty() on a declared variable is redundant.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Replace loop with array_column()
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Simpify assignment by removing temporary variable
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant if statements and array_merge
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove temporary variable
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Unindent code
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add helper method fetchByMode()
This is done to avoid calling methods through variables. It is the same
in terms of performance.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Move if statements to where they are relevant
I had a choice to either move the while loop out of the if statements
or move the if statements inside. I decided it made more sense to move
them inside.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add NULL to phpdoc for the two parameters that can be null
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Simplify return statement
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Improve performance and simplify code
Since we are always using STORE mode then we can use data_seek() method.
This will allow us to get rid of the loop and use fetchByMode() method.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Inline value defaulting
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant check
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* [bugfix] getColumnNames() should return an array of strings
There were multiple issues here. If we are selecting only a certain
column from the SQL then let's use the handy function instead of
array_keys. We will never get anything other than an array so checking
if it is not an array makes no sense. Returning null only to cast it to
an array is pointless. If really necessary, we could still check for an
empty array. The ColumnController returned the value without even
casting it to an array. I am not sure how jQuery/JavaScript dealt with it.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getColumnMapFromSql and add type hint
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getVirtualTables
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant loop
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant usage of freeResult()
If there was no chance for the object to leak the scope and it is the
end of the scope then there's no reason to call this method.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Replace dynamic variable with declare one
It's always the same name. If we were creating more than one, we could
use associative array, but in this case there seems to be absolutely no
need for any of this. Dynamic variables make it harder to debug the code.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Update psalm-baseline.xml
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Widen the condition per williamdes suggestion
This will allow value such as '' and '0' to pass through. Previously
`if(! empty($database))` and `if($database)` dissallowed them.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* fetchResult() should return only arrays
Tests fixed to return arrays. Non-empty in case of true, empty in case
of false.
Fixed a lot of Psalm issues where the condition was always true.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Simplify condition
isset is not needed since the variable is set and we check explicitely for an array so it cannot be null either.
!empty is not needed because the variable is set, so we just need to check if it has non-empty value
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Reduce indentation thanks to early returns
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Collapse isset check
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant variable assignments
$specialCharsEncoded is immediately overwritten in if/else.
$noSupportTypes is always empty and never used in this code.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant !empty() checks on parameters and defined variables
if(!empty($var)) on defined variables is redundant and equivalent to if($var)
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant ternary
$dispval is declared to be a string so the ternary is redundant
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor showEmptyResultMessageOrSetUniqueCondition
Replace boolean variable with returns, which leads to redundant else
block. Remove redundant unset, which leads to redundant variable assignment.
The return statement can be replaced with a boolean cast.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant unset at the end of the method
When a variable leaves scope it is automatically unset
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* [RISKY] Remove temporary var and error supression
It's risky because I was not able to determine the purpose of the error
suppression. In case of error, the function seems to return -1, but that
has not been checked here either. It was possible to silence undefined
global variable, but in that case, why was it not done only on that one
line?
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant condition
The variable is declared as int|false so if it is not false and not 0,
the only logical choice is a non-zero integer, which is always true.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Misleading type hint leading to invalid unit tests
According to DbiMysqli the first argument should be mysqli_result.
The correct type hint cannot be array. It can be object though.
Tested with a debugger that the passed value is really mysqli_result.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove dead code
Remove $inputType. I left the comemnt for posterity, but the variable is
dead. If in the future someone decides to implement it again, then
introducing the variable will not be a problem.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Move block of code to where it should be
I am not sure about this block of code. I don't fully understand what
it is supposed to do. However, making the query at the start of the
function doesn't make sense to me.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Use array_keys instead of foreach with dummy variable
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Use is_null instead of isset
True conditions are easier to read than inverted conditions.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant variable and break
When the loop breaks then the variable is true.
When the loop finishes then the variable is false.
We can simply replace it with return true/false to make it simpler.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add type declarations
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getEnumSetAndTimestampColumns
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant condition
According to searchColumnInForeigners $foreigner is dependant on values
from $foreigners which makes this check redundant.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getColumnEnumValues
Simplify parameter list, add psalm types, and simplify code
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getColumnSetValueAndSelectSize
Unfortunately, I cannot type hint the return value as then I would have
to type-hint the param as well.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove ternary operator
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getSelectOptionForUpload
Added types, removed NULL return (it is only outputted in Twig as
HTML string so NULL made no sense), and simpified parameters.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getMaxUploadSize
According to getMaxUploadSize the value can only be int.
Comparisons of numerical strings should be avoided. Psalm return type
can be added with the exact types.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getColumnSize
It takes a string as a second parameter now.
The reason for this is that during normal execution, the value comes
from column specifier e.g. VARCHAR(255), which is a string. The value
in spec_in_brackets could also represent ENUM/SET values, so it can't be
forced as an int.
This change required changing tests to pass a string instead of an array
with an int.
The behaviour should remain the same thanks to the (int) cast (previously
implicit, now explicit).
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove parameter $realNullValue
This parameter was always false. Instead define it as false inside
each method. The methods are private, so this should not cause any problems.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add psalm-return
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Add an early return and clarify return type
This function should never return false
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Change return type to multidimensional array
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Refactor getCommentsMap
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Type hint $tableColumns as multidimensional array
This simplifies static analysis a little bit
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Update phpstan baseline
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Fix invalid unit test
The method getTableColumns should return a multidimensional array always.
Added proper type hint to avoid further mistakes and changed mock.
According to my analysis this method should always return a
multidimensional array because parameter $column to getColumns() is null.
Also, this test is pretty useless as it just seems to test the bahaviour
of array_values which behaves as an identity function right now.
Note: The DBAL needs urgent refactoring too. I imagine array_values is
redundant at the moment. The getColumns should only return a list or
columns or a single column. There's no need to reindex the list. I think
Psalm should have picked it up, but I didn't check.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove redundant call to Util::extractColumnSpec()
Warning! There is no coverage in unit tests for this piece of code.
While I have analysed the code flow manually, it is possible the
duplicated call had some unknown to me purpose.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Update psalm-baseline.xml
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Adhere to the coding standard
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Revert "Remove ternary operator"
This reverts commit 3cbec2cebb.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Rename $field_MD5 to $fieldHashMd5
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Exclude string '0' from comparison and invert the order of operation
This helps with readability and it's unlikely that we would want to
check for string '0' here.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Rename $spec_in_brackets to $specInBrackets
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove else statements
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Introduce array_key_exists() and a local variable
We also add phpdoc comment telling static analysis that the value should
be a string. This is what the current code expects already.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
* Remove object parameter type declaration
Upon further inspection I realize this was wrong. The unit tests rely
on the parameter type being an integer and so we cannot force it to be
only an object. I leave the type hint as object as array was probably
never the right type.
Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
