Detecting duplicated array keys

[rafalwrzeszcz]

This is the implementation fo #322 request.

I added it to Design section, any better idea?

[ravage84]

@rafalwrzeszcz thanks for the PR.

About the Ruleset, I think CleanCode or UnusedCode would be a better fit, as duplicated array keys have nothing to do with software design.

https://phpmd.org/rules/index.html

https://phpmd.org/rules/index.html#clean-code-rules
https://phpmd.org/rules/index.html#unused-code-rules

[ravage84]

AppVeyor fails:
https://ci.appveyor.com/project/phpmd/phpmd/build/57

There were 10 errors:

1) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleNotAppliesToMethodWithAssotiativeArrayDefinitionWithoutDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

2) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToMethodWithAssotiativeArrayDefinitionWithDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

3) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToMethodWithAssotiativeArrayDefinitionWithDuplicatedMixedTypeKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

4) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToMethodWithAssotiativeArrayDefinitionWithDuplicatedMixedQuotedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

5) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesMultipleTimesToMethodWithAssotiativeArrayDefinitionWithDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

6) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleNotAppliesToFunctionWithAssotiativeArrayDefinitionWithoutDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

7) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToFunctionWithAssotiativeArrayDefinitionWithDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

8) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToFunctionWithAssotiativeArrayDefinitionWithDuplicatedMixedTypeKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

9) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesToFunctionWithAssotiativeArrayDefinitionWithDuplicatedMixedQuotedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

10) PHPMD\Rule\Design\DuplicatedArrayKeyTest::testRuleAppliesMultipleTimesToFunctionWithAssotiativeArrayDefinitionWithDuplicatedKeys
TypeError: Argument 1 passed to PHPUnit_Framework_TestCase::onNotSuccessfulTest() must be an instance of Exception, instance of Error given, called in C:\projects\phpmd\vendor\phpunit\phpunit\src\Framework\TestCase.php on line 851

FAILURES! 
Tests: 434, Assertions: 513, Errors: 10. 

[rafalwrzeszcz]

@ravage84 Moved to clean code. Any clue why AppVeyor is failing? The same test suite runs fine on Scrutinizer and Travis (and also locally).

It may be PHP7-related, but Travis build for PHP7 succeeds.

[ravage84]

Nope, no idea. Can you try to execute the tests on a Windows machine with PHP7?

[ravage84]

Not sure, if that's really necessary.

[ravage84]

Would your code be able to detect duplicated array keys, if one would use a constant or class constant as the array key(s)?

For example:

define('FOO', 'foobar');
return array(
   FOO => 42,
   FOO => 43,
);

const FOO = 'foobar';
public function testBla() {
    return array(
       self::FOO => 42,
       self::FOO => 43,
    );
}

or mixed:

define('FOO', 'foobar');
return array(
   FOO => 42,
   'foobar' => 43,
);

const FOO = 'foobar';
public function testBla() {
    return array(
       self::FOO => 42,
       'foobar' => 43,
    );
}

[rafalwrzeszcz]

@ravage84 No and I don't think it's feasible in a reasonable time/PR size - constant can come from outside of the file or even a library.

But it should detect keys defined with same constant twice (like self::FOO => 'one', self::FOO => 'two') as it checks the literal definition.

[ravage84]

That's fine for me. Just wanted to check.

[rafalwrzeszcz]

@ravage84 Then it has to wait few days, not sure if I will find some time to set up env for Win before the weekend.

[ravage84]

Take your time. I would like @manuelpichler to have an eye on this anyway. But I know he is very busy lately.

[ravage84]

@rafalwrzeszcz how is your status on this?

[ravage84]

@rafalwrzeszcz can you give me an update on this?

[eeree]

This one will end up with:
PHP Fatal error: Nesting level too deep - recursive dependency? in src\main\php\PHPMD\Rule\CleanCode\DuplicatedArrayKey.php on line 87
Comparing objects in PHP is recursive (deep-comparison).

[eeree]

This one could be improved, as given array:

$arr = [
    'foo',
    'bar',
    '' => 'baz',
    0 => '20',
    '0' => '30',
    false => '40',
    1 => '50',
    '1' => '60',
    true => '70',
    null => '80',
];

Will result in:

Array
(
    [0] => 40
    [1] => 70
    [] => 80
)

All due to casting.

[eeree]

It can be simplified to trim($key, '\'"');

[ravage84]

Closing as superseded by #484