Skip to content

Commit 1cd4dc8

Browse files
author
Igor Melnikov
committed
MAGETWO-61240: Fix \Magento\Sniffs\Translation\ConstantUsageSniff
Addressing code review feedback
1 parent 611ad9d commit 1cd4dc8

File tree

6 files changed

+50
-189
lines changed

6 files changed

+50
-189
lines changed

‎dev/tests/static/framework/Magento/TestFramework/Utility/File.php

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,6 @@
1313
*/
1414
class File
1515
{
16-
/**@#+
17-
* File types offset flags
18-
*/
19-
const INCLUDE_APP_CODE = Files::INCLUDE_APP_CODE;
20-
const INCLUDE_PUB_CODE = Files::INCLUDE_PUB_CODE;
21-
const INCLUDE_LIBS = Files::INCLUDE_LIBS;
22-
const INCLUDE_TEMPLATES = Files::INCLUDE_TEMPLATES;
23-
const INCLUDE_TESTS = Files::INCLUDE_TESTS;
24-
const INCLUDE_SETUP = 128;
25-
const INCLUDE_NON_CLASSES = Files::INCLUDE_NON_CLASSES;
26-
const AS_DATA_SET = Files::AS_DATA_SET;
27-
/**#@-*/
28-
2916
/**
3017
* @var RegexIteratorFactory
3118
*/
@@ -53,29 +40,23 @@ public function __construct(
5340
/**
5441
* Get list of PHP files
5542
*
56-
* @param int $flags
5743
* @return array
5844
* @throws \Exception
5945
*/
60-
public function getPhpFiles(
61-
$flags = self::INCLUDE_APP_CODE
62-
| self::INCLUDE_PUB_CODE
63-
| self::INCLUDE_LIBS
64-
| self::INCLUDE_TEMPLATES
65-
| self::INCLUDE_TESTS
66-
| self::INCLUDE_SETUP
67-
| self::INCLUDE_NON_CLASSES
68-
| self::AS_DATA_SET
69-
) {
46+
public function getPhpFiles()
47+
{
7048
$files = array_merge(
71-
$this->fileUtilities->getPhpFiles((2147483647 - self::AS_DATA_SET) & $flags),
72-
$this->getSetupPhpFiles($flags)
49+
$this->fileUtilities->getPhpFiles(
50+
Files::INCLUDE_APP_CODE
51+
| Files::INCLUDE_PUB_CODE
52+
| Files::INCLUDE_LIBS
53+
| Files::INCLUDE_TEMPLATES
54+
| Files::INCLUDE_TESTS
55+
| Files::INCLUDE_NON_CLASSES
56+
),
57+
$this->getSetupPhpFiles()
7358
);
74-
75-
if ($flags & self::AS_DATA_SET) {
76-
return Files::composeDataSets($files);
77-
}
78-
return $files;
59+
return Files::composeDataSets($files);
7960
}
8061

8162
/**
@@ -84,17 +65,15 @@ public function getPhpFiles(
8465
* @param int $flags
8566
* @return array
8667
*/
87-
private function getSetupPhpFiles($flags)
68+
private function getSetupPhpFiles()
8869
{
8970
$files = [];
90-
if ($flags & self::INCLUDE_SETUP) {
91-
$regexIterator = $this->regexIteratorFactory->create(
92-
BP . '/setup',
93-
'/.*php^/'
94-
);
95-
foreach ($regexIterator as $file) {
96-
$files = array_merge($files, [$file]);
97-
}
71+
$regexIterator = $this->regexIteratorFactory->create(
72+
BP . '/setup',
73+
'/.*php^/'
74+
);
75+
foreach ($regexIterator as $file) {
76+
$files = array_merge($files, [$file]);
9877
}
9978
return $files;
10079
}

‎dev/tests/static/framework/Magento/TestFramework/Utility/FunctionDetector.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
namespace Magento\TestFramework\Utility;
77

88
/**
9-
* A helper detects functions
9+
* Check if one or more functions are used in the file
1010
*/
1111
class FunctionDetector
1212
{
@@ -27,16 +27,16 @@ class FunctionDetector
2727
* ],
2828
* ]
2929
*
30-
* @param string $fileFullPath
30+
* @param string $filePath
3131
* @param string[] $functions
3232
* @return array
3333
*/
34-
public function detectFunctions($fileFullPath, $functions)
34+
public function detect($filePath, $functions)
3535
{
3636
$result = [];
3737
$regexp = $this->composeRegexp($functions);
3838
if ($regexp) {
39-
$file = file($fileFullPath);
39+
$file = file($filePath);
4040
array_unshift($file, '');
4141
$lines = preg_grep(
4242
$regexp,

‎dev/tests/static/framework/tests/unit/testsuite/Magento/TestFramework/Utility/FileTest.php

Lines changed: 13 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -46,54 +46,20 @@ protected function setUp()
4646
);
4747
}
4848

49-
public function testGetPhpFilesWithoutSetup()
49+
public function testGetPhpFiles()
5050
{
5151
$appFiles = [
5252
'file1',
5353
'file2'
5454
];
55+
$setupFiles = [
56+
'file3'
57+
];
5558
$expected = [
5659
'file1' => ['file1'],
57-
'file2' => ['file2']
60+
'file2' => ['file2'],
61+
'file3' => ['file3']
5862
];
59-
$this->regexIteratorFactoryMock->expects($this->never())
60-
->method('create');
61-
$this->fileUtilitiesMock->expects($this->once())
62-
->method('getPhpFiles')
63-
->with(
64-
File::INCLUDE_APP_CODE
65-
| File::INCLUDE_PUB_CODE
66-
| File::INCLUDE_LIBS
67-
| File::INCLUDE_TEMPLATES
68-
| File::INCLUDE_TESTS
69-
| File::INCLUDE_NON_CLASSES
70-
)
71-
->willReturn($appFiles);
72-
$actual = $this->file->getPhpFiles(
73-
File::INCLUDE_APP_CODE
74-
| File::INCLUDE_PUB_CODE
75-
| File::INCLUDE_LIBS
76-
| File::INCLUDE_TEMPLATES
77-
| File::INCLUDE_TESTS
78-
| File::INCLUDE_NON_CLASSES
79-
| File::AS_DATA_SET
80-
);
81-
$this->assertEquals($expected, $actual);
82-
}
83-
84-
/**
85-
* @param array $appFiles
86-
* @param array$setupFiles
87-
* @param int $flags
88-
* @param array $expected
89-
* @dataProvider getPhpFilesWithSetupDataProvider
90-
*/
91-
public function testGetPhpFilesWithSetup(
92-
$appFiles,
93-
$setupFiles,
94-
$flags,
95-
$expected
96-
) {
9763
$iteratorMock = $this->getMock(\IteratorAggregate::class, [], [], '', false);
9864
$iteratorMock->expects($this->any())
9965
->method('getIterator')
@@ -104,61 +70,14 @@ public function testGetPhpFilesWithSetup(
10470
$this->fileUtilitiesMock->expects($this->once())
10571
->method('getPhpFiles')
10672
->with(
107-
File::INCLUDE_APP_CODE
108-
| File::INCLUDE_PUB_CODE
109-
| File::INCLUDE_LIBS
110-
| File::INCLUDE_TEMPLATES
111-
| File::INCLUDE_TESTS
112-
| File::INCLUDE_SETUP
113-
| File::INCLUDE_NON_CLASSES
73+
Files::INCLUDE_APP_CODE
74+
| Files::INCLUDE_PUB_CODE
75+
| Files::INCLUDE_LIBS
76+
| Files::INCLUDE_TEMPLATES
77+
| Files::INCLUDE_TESTS
78+
| Files::INCLUDE_NON_CLASSES
11479
)
11580
->willReturn($appFiles);
116-
$this->assertEquals($expected, $this->file->getPhpFiles($flags));
117-
}
118-
119-
/**
120-
* @return array
121-
*/
122-
public function getPhpFilesWithSetupDataProvider()
123-
{
124-
$flags = File::INCLUDE_APP_CODE
125-
| File::INCLUDE_PUB_CODE
126-
| File::INCLUDE_LIBS
127-
| File::INCLUDE_TEMPLATES
128-
| File::INCLUDE_TESTS
129-
| File::INCLUDE_SETUP
130-
| File::INCLUDE_NON_CLASSES;
131-
return [
132-
[
133-
[
134-
'file1',
135-
'file2'
136-
],
137-
[
138-
'file3'
139-
],
140-
$flags | File::AS_DATA_SET,
141-
[
142-
'file1' => ['file1'],
143-
'file2' => ['file2'],
144-
'file3' => ['file3']
145-
]
146-
],
147-
[
148-
[
149-
'file1',
150-
'file2'
151-
],
152-
[
153-
'file3'
154-
],
155-
$flags,
156-
[
157-
'file1',
158-
'file2',
159-
'file3'
160-
]
161-
]
162-
];
81+
$this->assertEquals($expected, $this->file->getPhpFiles());
16382
}
16483
}

‎dev/tests/static/framework/tests/unit/testsuite/Magento/TestFramework/Utility/FunctionDetectorTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ public function testDetectFunctions()
1111
{
1212
$fixturePath = __DIR__ . '/_files/test.txt';
1313
$expectedResults = [
14-
24 => ['strtoupper', 'md5'],
15-
36 => ['security'],
16-
37 => ['security'],
14+
1 => ['strtoupper', 'strtolower'],
15+
3 => ['foo'],
16+
4 => ['foo'],
1717
];
1818
$functionDetector = new FunctionDetector();
19-
$lines = $functionDetector->detectFunctions($fixturePath, ['security', 'md5', 'test', 'strtoupper']);
19+
$lines = $functionDetector->detect($fixturePath, ['foo', 'strtoupper', 'test', 'strtolower']);
2020
$this->assertEquals($expectedResults, $lines);
2121
}
2222
}

‎dev/tests/static/framework/tests/unit/testsuite/Magento/TestFramework/Utility/_files/test.txt

Lines changed: 9 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,9 @@
1-
<?php
2-
/**
3-
* Copyright © 2016 Magento. All rights reserved.
4-
* See COPYING.txt for license details.
5-
*/
6-
namespace Magento\Authorizenet\Model\Security;
7-
8-
/**
9-
* security
10-
*/
11-
class Test extends AuthorizenetResponse
12-
{
13-
/**
14-
* Generates an Md5 hash to compare against AuthNet's.
15-
*
16-
* @param string $merchantMd5
17-
* @param string $merchantApiLogin
18-
* @param string $amount
19-
* @param string $transactionId
20-
* @return string
21-
*/
22-
public function checkSecurity($merchantMd5, $merchantApiLogin, $amount, $transactionId)
23-
{
24-
return strtoupper(md5($merchantMd5 . $merchantApiLogin . $transactionId . $amount));
25-
}
26-
27-
/**
28-
* Return if is valid order id.
29-
*
30-
* @param string $merchantMd5
31-
* @param string $merchantApiLogin
32-
* @return bool
33-
*/
34-
public function security($merchantMd5, $merchantApiLogin)
35-
{
36-
$hash = $this->generateHash(security($merchantMd5), $merchantApiLogin, $this->getXAmount());
37-
security(
38-
'something'
39-
);
40-
return Security::compareStrings($hash, $this->getData('x_MD5_Hash'));
41-
42-
$this->security('check');
43-
Security::security();
44-
return $this->getXResponseCode() == \Magento\Authorizenet\Model\Security::RESPONSE_CODE_APPROVED;
45-
}
46-
}
1+
strtoupper(strtolower($foo . $bar))
2+
function foo($merchantMd5, $merchantApiLogin)
3+
$this->generateHash(foo($bar), $foo)
4+
foo(
5+
'bar'
6+
)
7+
Foo::bar($foo, $this->getData('bar'))
8+
$this->foo('bar')
9+
Foo::foo()

‎dev/tests/static/testsuite/Magento/Test/Legacy/UnsecureFunctionsUsageTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public function testUnsecureFunctionsUsage()
9797
$invoker(
9898
function ($fileFullPath) use ($functionDetector) {
9999
$functions = $this->getFunctions($fileFullPath);
100-
$lines = $functionDetector->detectFunctions($fileFullPath, array_keys($functions));
100+
$lines = $functionDetector->detect($fileFullPath, array_keys($functions));
101101

102102
$message = '';
103103
if (!empty($lines)) {

0 commit comments

Comments
 (0)