noverify
noverify copied to clipboard
Report non-literal key duplicates in dupArrayKeys diagnostic
Code Example
// 1. Constants
$example1 = [
C1 => 1,
C2 => 2,
C1 => 3, // Duplicate key C1
];
$example2 = [
T::C1 => 1,
T::C2 => 2,
T::C1 => 3, // Duplicate key T1::C1
];
// 2. Expressions
$example3 = [
$k[0] => 1,
$k[1] => 2,
$k[0] => 3, // Duplicate key $k[0]
];
// Note: if $k is a class that implements ArrayAccess, it's
// safer to ignore it or check whether element fetching
// is a side-effect-free method.
// 3. Optional: anything that satisfies sideEffectFree()
$example3 = [
'a' . $s => 1,
'b' . $s => 2,
'a' . $s => 3, // Duplicate key 'a'.$s
];
Currently, dupArrayKeys
only handles simple literal keys (ints and strings).
Note that only int
and string
typed keys are valid in PHP (all other types are converted to them). If an illegal-typed value is used, dupArrayKeys
should ignore it as it should be caught by another diagnostic. That being said, it's debatable whether the code below should be reported as having duplicated keys:
$example4 = [
new T() => 1,
new T() => 2,
];
Syntactically it does contain duplicated new T()
key but in practice we should warn about illegal key type.
Actual Behavior
No warnings.
Expected Behavior
Duplicate array key warning.
Only constant expressions should be counted though as there exist things like this (in theory):
<?php
[ rand() => 1, rand() => 2 ];
This code is not great but it usually does not contain duplicate keys :)
We have sideEffectFree(expr)
that can reject non-pure calls and other expressions that can have side effects. If it's not good enough, we can adjust it to be good enough.
Maybe we can use nodeSet for this, but maybe it's still easier to support a few types of nodes:
-
expr.ConstFetch
-
expr.ClassConstFetch
-
scalar.Lnumber
-
scalar.Dnumber
-
scalar.String
(Maybe something else?)
Guys, sorry about this spam, I thought that testing pull requests in my fork won't mention original repo issue. :smile: Please don't look at this commits, they are in dead branches, just look at PR I will have my pull request now opened here