Developed preference xml inspections #578

Iamwade · 2021-04-23T18:30:09Z

Description (*)
Added inspection to the di.xml file for preference.
The inspection is performed for preference attributes "for" and "type".
If the class in the specified path for these attributes does not exist, then the corresponding warning will be displayed

Fixed Issues (if relevant)

Fixes Develop preference xml inspections #566

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with integration/functional tests (if applicable)
All automated tests passed successfully (all builds are green)

Iamwade · 2021-04-23T18:31:18Z

Waiting for #577

Iamwade · 2021-04-28T19:30:43Z

Waiting for #582

bohdan-harniuk

Hello, @Iamwade! I've left few comments in your PR. Please, check them.

bohdan-harniuk · 2021-04-30T19:09:36Z

resources/inspectionDescriptions/PreferenceXmlInspections.html

+  <li>existence of attribute values</li>
+</ul>
+</body>
+</html>


@Iamwade, please add an empty line in the and of the file.

bohdan-harniuk · 2021-04-30T19:13:09Z

resources/inspectionDescriptions/PreferenceXmlInspections.html

+<html>
+<body>
+<p>
+  Validates classes and interfaces for  attributes "for" and the "type" for the "preference" tag


@Iamwade, please remove extra space after the first "for". And if you can, please, formulate this sentence differently. For now, it is hard to read.

bohdan-harniuk · 2021-04-30T19:16:53Z

resources/META-INF/plugin.xml

@@ -219,6 +219,13 @@
                         enabledByDefault="true" level="WARNING"
                         implementationClass="com.magento.idea.magento2plugin.inspections.xml.InvalidDependencyInjectionTypeInspection"/>

+        <localInspection language="XML" groupPath="XML"
+                         shortName="PreferenceXmlInspections"
+                         displayName="Inspection the existence of PHP classes for attributes in the preference"


@Iamwade, please extract displayName attribute value to the magento2.inspection bundle and reference it from this context:

Suggested change

displayName="Inspection the existence of PHP classes for attributes in the preference"

bundle="magento2.inspection" key="inspection.displayName.PreferenceXmlInspections"

bohdan-harniuk · 2021-04-30T19:17:28Z

resources/META-INF/plugin.xml

+        <localInspection language="XML" groupPath="XML"
+                         shortName="PreferenceXmlInspections"
+                         displayName="Inspection the existence of PHP classes for attributes in the preference"
+                         groupName="Magento 2"


Suggested change

groupName="Magento 2"

groupBundle="magento2.inspection" groupKey="inspection.group.name"

src/com/magento/idea/magento2plugin/inspections/xml/PreferenceDeclarationInspection.java

bohdan-harniuk · 2021-04-30T19:23:26Z

tests/com/magento/idea/magento2plugin/inspections/xml/PreferenceDeclarationInspectionTest.java

+
+    private static final String ARGUMENT_VALUE_IS_EMPTY =
+            "inspection.error.idAttributeCanNotBeEmpty";
+


@Iamwade, do we really need extra empty lines after each constant property?

bohdan-harniuk · 2021-04-30T19:27:26Z

tests/com/magento/idea/magento2plugin/inspections/xml/PreferenceDeclarationInspectionTest.java

+     * Test for an error for the "type" attribute because it is empty.
+     * <preference for="" type="Foo\Bar\Model\Logger"/>
+     */
+    public void testAttrArgForValuesIsEmpty() {


@Iamwade, sorry, but I cannot understand this...you are testing type attribute, but method annotation, description, name and test data file name says that you are testing For attribute. Could you check it?

tests/com/magento/idea/magento2plugin/inspections/xml/PreferenceDeclarationInspectionTest.java

bohdan-harniuk · 2021-04-30T19:31:24Z

tests/com/magento/idea/magento2plugin/inspections/xml/PreferenceDeclarationInspectionTest.java

+
+        final String typeAttrIsEmptyMessage =  inspectionBundle.message(
+                ARGUMENT_VALUE_IS_EMPTY,
+                ModuleDiXml.PREFERENCE_ATTR_FOR


@Iamwade, check it, please. (the reason the same as for previous methods)

Iamwade · 2021-04-30T20:41:27Z

@bohdan-harniuk
Thank you for the code review, the described points have been fixed

bohdan-harniuk

Hello, @Iamwade! I've left one more comment in your PR. Please, check it.

testData/inspections/xml/PreferenceDeclarationInspection/classAttrForDoesNotExists/di.xml

bohdan-harniuk

@Iamwade, thank you for your work here!

VitaliyBoyko

Getting an error when removing the attribute value

Iamwade · 2021-05-05T12:37:02Z

@VitaliyBoyko a bug when removing an attribute value has been fixed. In the process of fixing this error, I found this error for the tag in the file
https://github.com/magento/magento2-phpstorm-plugin/blob/4.0.0-develop/src/com/magento/idea/magento2plugin/inspections/xml/InvalidDependencyInjectionTypeInspection.java
and I fixed it too.
Please review.

Iamwade added 2 commits April 23, 2021 20:47

Develop preference xml inspections

88dfb3d

Corrected the manual file

6af755e

magento-engcom-team added Partner: Atwix partners-contribution labels Apr 23, 2021

m2-community-project bot added the Progress: ready for review label Apr 23, 2021

Iamwade added 2 commits April 28, 2021 21:36

fixed merge conflict

6e24fa5

refactoring code and adding tests

e03abae

Iamwade added 4 commits April 30, 2021 17:43

fixed merge conflict

2b22440

fixed check style conflict

62fa5a9

fixed PMD conflict

d66ee46

fixed PMD conflict and tests

9a5672a

Iamwade marked this pull request as ready for review April 30, 2021 16:11

bohdan-harniuk requested changes Apr 30, 2021

View reviewed changes

m2-community-project bot added Progress: changes requested and removed Progress: ready for review labels Apr 30, 2021

fixed code after review

961f8c9

m2-community-project bot added Progress: ready for review and removed Progress: changes requested labels Apr 30, 2021

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

bohdan-harniuk requested changes May 1, 2021

View reviewed changes

testData/inspections/xml/PreferenceDeclarationInspection/classAttrForDoesNotExists/di.xml Outdated Show resolved Hide resolved

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

fixed code test file after review

2efa4be

m2-community-project bot added Progress: ready for review and removed Progress: changes requested labels May 3, 2021

bohdan-harniuk previously approved these changes May 3, 2021

View reviewed changes

m2-community-project bot added Progress: approved and removed Progress: ready for review labels May 3, 2021

update branch

d92b020

VitaliyBoyko self-requested a review May 4, 2021 07:12

VitaliyBoyko self-assigned this May 4, 2021

VitaliyBoyko added the QA failed label May 4, 2021

VitaliyBoyko requested changes May 4, 2021

View reviewed changes

m2-community-project bot added Progress: changes requested and removed Progress: approved labels May 4, 2021

Iamwade dismissed bohdan-harniuk’s stale review via 454c97a May 5, 2021 12:24

fixed error when adding an attribute with empty value

084b53a

Iamwade force-pushed the developed-preference-xml-inspections branch from 454c97a to 084b53a Compare May 5, 2021 12:27

Merge branch '4.0.0-develop' into developed-preference-xml-inspections

ea56035

VitaliyBoyko added Complex Award: test coverage labels May 6, 2021

VitaliyBoyko approved these changes May 6, 2021

View reviewed changes

VitaliyBoyko merged commit 0b7041d into magento:4.0.0-develop May 6, 2021

m2-community-project bot added Progress: approved Progress: done and removed Progress: changes requested labels May 6, 2021

VitaliyBoyko removed the QA failed label May 6, 2021

m2-community-project bot removed the Progress: approved label May 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Developed preference xml inspections #578

Developed preference xml inspections #578

Iamwade commented Apr 23, 2021

Iamwade commented Apr 23, 2021

Iamwade commented Apr 28, 2021

bohdan-harniuk left a comment

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

bohdan-harniuk Apr 30, 2021

Iamwade Apr 30, 2021

bohdan-harniuk May 1, 2021

Iamwade commented Apr 30, 2021

bohdan-harniuk left a comment

bohdan-harniuk left a comment

VitaliyBoyko left a comment

Iamwade commented May 5, 2021

	displayName="Inspection the existence of PHP classes for attributes in the preference"
	bundle="magento2.inspection" key="inspection.displayName.PreferenceXmlInspections"

	groupName="Magento 2"
	groupBundle="magento2.inspection" groupKey="inspection.group.name"


		private static final String ARGUMENT_VALUE_IS_EMPTY =
		"inspection.error.idAttributeCanNotBeEmpty";

Developed preference xml inspections #578

Developed preference xml inspections #578

Conversation

Iamwade commented Apr 23, 2021

Iamwade commented Apr 23, 2021

Iamwade commented Apr 28, 2021

bohdan-harniuk 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Iamwade commented Apr 30, 2021

bohdan-harniuk left a comment

Choose a reason for hiding this comment

bohdan-harniuk left a comment

Choose a reason for hiding this comment

VitaliyBoyko left a comment

Choose a reason for hiding this comment

Iamwade commented May 5, 2021