AC-663 PHPCS classes test #274

svera · 2021-09-16T09:30:18Z

This second part of AC-663 is focused on checking that all references to classes in xml files under etc follows PSR standards.

…into AC-663_phpcs-ClassesTest

jcuerdo · 2021-09-16T10:15:47Z

Magento2/Tests/Legacy/ClassesConfigurationUnitTest.1.xml

+                </field>
+                <field id="order_status" translate="label" type="select" sortOrder="20" showInDefault="1" showInWebsite="1" canRestore="1">
+                    <label>New Order Status</label>
+                    <source_model>Sales\MODEL\Config\Source\Order\Status\NewStatus</source_model>


How about this line? Should it work?

@jcuerdo is not in the Magento namespace, thus it is ignored

jcuerdo · 2021-09-16T10:16:38Z

Magento2/Tests/Legacy/ClassesConfigurationUnitTest.php

+        }
+        if ($testFile === 'ClassesConfigurationUnitTest.2.xml') {
+            return [
+                22 => 1,


In lines 22 and 42 there are no classes...

@jcuerdo sniffer returns the position of the opening tag where the issue happens

svera · 2021-09-16T09:31:23Z

Magento2/Sniffs/Legacy/ClassesConfigurationSniff.php

+        $classes = $this->getXmlNode(
+            $xml,
+            '
+            /config//resource_adapter | /config/*[not(name()="sections")]//class[not(ancestor::observers)]


This regexp must be updated almost certainly, as I think some of these paths are no longer accurate. Could you please check them out @sivaschenko ?

svera · 2021-09-16T13:04:02Z

Magento2/Tests/Legacy/ClassesConfigurationUnitTest.1.xml

+                </field>
+                <field id="order_status" translate="label" type="select" sortOrder="20" showInDefault="1" showInWebsite="1" canRestore="1">
+                    <label>New Order Status</label>
+                    <source_model>Sales\MODEL\Config\Source\Order\Status\NewStatus</source_model>


@jcuerdo is not in the Magento namespace, thus it is ignored

svera · 2021-09-16T13:05:04Z

Magento2/Tests/Legacy/ClassesConfigurationUnitTest.php

+        }
+        if ($testFile === 'ClassesConfigurationUnitTest.2.xml') {
+            return [
+                22 => 1,


@jcuerdo sniffer returns the position of the opening tag where the issue happens

sivaschenko

Thanks for the PR @svera ! Please see my review comments

Magento2/Sniffs/Legacy/ExtendedNode.php

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php

Magento2/ruleset.xml

…f.php Co-authored-by: Sergii Ivashchenko <[email protected]>

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php

…into AC-663_phpcs-ClassesTest

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php

sivaschenko · 2021-10-13T13:01:36Z

Magento2/ruleset.xml

@@ -148,6 +148,14 @@
        <exclude-pattern>*\.xml$</exclude-pattern>
        <exclude-pattern>*\.js$</exclude-pattern>
    </rule>
+    <rule ref="Magento2.Legacy.ClassReferencesInConfigurationFiles">
+        <include-pattern>*\/etc/*.xml$</include-pattern>


Layout files are tested in this sniff as well

@sivaschenko there are also checks which involve references to classes in layouts, look at collectClassesInLayout(). Maybe we should rename this sniff to ClassReferencesInXMLFilesSniff?

I think the naming is fine. Should we add the include pattern for layout files?

I've removed layout files check from sniff, so I guess we no longer need to include its pattern here @sivaschenko

…f.php Co-authored-by: Sergii Ivashchenko <[email protected]>

svera · 2021-10-13T14:46:47Z

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php

+    {
+        return $this->getValuesFromXmlTagAttribute(
+            $xml,
+            '/layout//@module',


@sivaschenko I see a lot of occurrences of <page layout="... but none of <layout>... module=""> which are the ones we are looking for at the moment. Are these also outdated?

yes, this check can be removed as well

Removed as talked offline @sivaschenko

…oding-standard into AC-663_phpcs-ClassesTest

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php

…f.php Co-authored-by: Jorge Cuerdo Álvarez <[email protected]>

svera · 2021-10-14T11:08:31Z

@magento import pr to magento-commerce/magento-coding-standard

m2-github-services · 2021-10-14T11:09:26Z

@svera the Pull Request is successfully imported.

svera added 6 commits September 6, 2021 13:40

AC-663: Create phpcs static check for ClassesTest::testPhpCode

56e2de8

AC-663: Create phpcs static check for ClassesTest::testPhpCode

c9703a6

AC-663: Create phpcs static check for ClassesTest::testPhpCode

5a83385

AC-663: Create phpcs static check for ClassesTest::testPhpCode

5b40712

Merge branch 'develop' of github.com:magento/magento-coding-standard …

4fe226e

…into AC-663_phpcs-ClassesTest

AC-663: Create phpcs static check for ClassesTest::testPhpCode

9d542d9

m2-community-project bot added the Progress: ready for review label Sep 16, 2021

svera added 5 commits September 16, 2021 11:33

AC-663: Create phpcs static check for ClassesTest::testPhpCode

2c5c076

AC-663: Create phpcs static check for ClassesTest::testPhpCode

3636e9d

AC-663: Create phpcs static check for ClassesTest::testPhpCode

0293560

AC-663: Create phpcs static check for ClassesTest::testPhpCode

b91d0d4

AC-663: Create phpcs static check for ClassesTest::testPhpCode

717c998

jcuerdo reviewed Sep 16, 2021

View reviewed changes

m2-community-project bot assigned jcuerdo Sep 16, 2021

m2-community-project bot added Progress: review and removed Progress: ready for review labels Sep 16, 2021

svera commented Sep 16, 2021

View reviewed changes

svera added 7 commits September 16, 2021 15:30

AC-663: Create phpcs static check for ClassesTest::testPhpCode

5810c6e

AC-663: Create phpcs static check for ClassesTest::testPhpCode

0bc42a6

AC-663: Create phpcs static check for ClassesTest::testPhpCode

19bb57b

AC-663: Create phpcs static check for ClassesTest::testPhpCode

48a5816

AC-663: Create phpcs static check for ClassesTest::testPhpCode

b10df58

AC-663: Create phpcs static check for ClassesTest::testPhpCode

64fa0cc

AC-663: Create phpcs static check for ClassesTest::testPhpCode

5aa5721

sivaschenko requested changes Sep 17, 2021

View reviewed changes

m2-community-project bot assigned sivaschenko Sep 17, 2021

m2-community-project bot added Progress: changes requested and removed Progress: review labels Sep 17, 2021

svera and others added 2 commits September 20, 2021 09:37

Update Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSnif…

389d7ca

…f.php Co-authored-by: Sergii Ivashchenko <[email protected]>

Update Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSnif…

c480100

…f.php Co-authored-by: Sergii Ivashchenko <[email protected]>

svera commented Sep 21, 2021

View reviewed changes

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php Outdated Show resolved Hide resolved

svera added 2 commits October 13, 2021 12:36

Merge branch 'develop' of github.com:magento/magento-coding-standard …

6558e29

…into AC-663_phpcs-ClassesTest

AC-663: Create phpcs static check for ClassesTest::testPhpCode

36776b8

svera marked this pull request as ready for review October 13, 2021 10:51

loginesta previously approved these changes Oct 13, 2021

View reviewed changes

m2-community-project bot assigned loginesta Oct 13, 2021

m2-community-project bot added Progress: approved and removed Progress: review labels Oct 13, 2021

sivaschenko reviewed Oct 13, 2021

View reviewed changes

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php Outdated Show resolved Hide resolved

m2-community-project bot added Progress: review and removed Progress: approved labels Oct 13, 2021

sivaschenko reviewed Oct 13, 2021

View reviewed changes

Update Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSnif…

39267ba

…f.php Co-authored-by: Sergii Ivashchenko <[email protected]>

svera dismissed loginesta’s stale review via 39267ba October 13, 2021 13:03

svera commented Oct 13, 2021

View reviewed changes

svera added 3 commits October 13, 2021 17:16

AC-663: Create phpcs static check for ClassesTest::testPhpCode

8402f7d

Merge branch 'AC-663_phpcs-ClassesTest' of github.com:svera/magento-c…

1338211

…oding-standard into AC-663_phpcs-ClassesTest

AC-663: Create phpcs static check for ClassesTest::testPhpCode

ee4c7d6

jcuerdo reviewed Oct 14, 2021

View reviewed changes

Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSniff.php Outdated Show resolved Hide resolved

svera and others added 2 commits October 14, 2021 12:49

Update Magento2/Sniffs/Legacy/ClassReferencesInConfigurationFilesSnif…

08ed34b

…f.php Co-authored-by: Jorge Cuerdo Álvarez <[email protected]>

AC-663: Create phpcs static check for ClassesTest::testPhpCode

c9ca788

sivaschenko approved these changes Oct 14, 2021

View reviewed changes

m2-community-project bot added Progress: approved and removed Progress: review labels Oct 14, 2021

magento-devops-reposync-svc merged commit 3b0e492 into magento:develop Oct 14, 2021

m2-community-project bot added Progress: done and removed Progress: approved labels Oct 14, 2021

sivaschenko mentioned this pull request Oct 31, 2021

Combined sivaschenko/magento-coding-standard#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AC-663 PHPCS classes test #274

AC-663 PHPCS classes test #274

svera commented Sep 16, 2021

jcuerdo Sep 16, 2021

svera Sep 16, 2021

jcuerdo Sep 16, 2021

svera Sep 16, 2021

svera Sep 16, 2021

svera Sep 16, 2021

svera Sep 16, 2021

sivaschenko left a comment

sivaschenko Oct 13, 2021

svera Oct 13, 2021

sivaschenko Oct 13, 2021

svera Oct 13, 2021

svera Oct 13, 2021 •

edited

Loading

sivaschenko Oct 13, 2021

svera Oct 13, 2021

svera commented Oct 14, 2021

m2-github-services commented Oct 14, 2021

AC-663 PHPCS classes test #274

AC-663 PHPCS classes test #274

Conversation

svera commented Sep 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svera Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svera commented Oct 14, 2021

m2-github-services commented Oct 14, 2021

svera Oct 13, 2021 •

edited

Loading