Merge pull request #432 from drpayyne/issue-424

Added inspection warning for disabled observer

Author: coderimus

resources/magento2/inspection.properties

@@ -15,6 +15,7 @@ inspection.plugin.error.redundantParameter=Redundant parameter
 inspection.plugin.error.typeIncompatibility=Possible type incompatibility. Consider changing the parameter according to the target method.
 inspection.observer.duplicateInSameFile=The observer name already used in this file. For more details see Inspection Description.
 inspection.observer.duplicateInOtherPlaces=The observer name "{0}" for event "{1}" is already used in the module "{2}" ({3} scope). For more details see Inspection Description.
+inspection.observer.disabledObserverDoesNotExist=This observer does not exist to be disabled. For more details, see Inspection Description.
 inspection.cache.disabledCache=Cacheable false attribute on the default layout will disable cache site-wide
 inspection.moduleDeclaration.warning.wrongModuleName=Provided module name "{0}" does not match expected "{1}"
 inspection.moduleDeclaration.fix=Fix module name

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

@@ -94,10 +94,7 @@ public void visitFile(final PsiFile file) {
                         final XmlAttribute observerDisabledAttribute =
                                 observerXmlTag.getAttribute("disabled");
 
-                        if (observerNameAttribute == null || (
-                                observerDisabledAttribute != null//NOPMD
-                                && observerDisabledAttribute.getValue().equals("true"))
-                        ) {
+                        if (observerNameAttribute == null) {
                             continue;
                         }
 
@@ -122,6 +119,21 @@ public void visitFile(final PsiFile file) {
                                         eventIndex,
                                         file
                                 );
+
+                        if (observerDisabledAttribute != null
+                                && observerDisabledAttribute.getValue() != null
+                                && observerDisabledAttribute.getValue().equals("true")
+                                && modulesWithSameObserverName.isEmpty()
+                        ) {
+                            problemsHolder.registerProblem(
+                                    observerNameAttribute.getValueElement(),
+                                    inspectionBundle.message(
+                                            "inspection.observer.disabledObserverDoesNotExist"
+                                    ),
+                                    errorSeverity
+                            );
+                        }
+
                         for (final HashMap<String, String> moduleEntry:
                                 modulesWithSameObserverName) {
                             final Map.Entry<String, String> module = moduleEntry

testData/inspections/xml/ObserverDeclarationInspection/disablingNonExistingObserver/events.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+<!--
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+-->
+<config>
+    <event name="test_event_in_test_class">
+        <observer name=<warning descr="This observer does not exist to be disabled. For more details, see Inspection Description.">"test_non_existing_observer"</warning>
+                  instance="Magento\Catalog\Observer\TestObserver"
+                  disabled="true"/>
+    </event>
+</config>

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

@@ -31,4 +31,12 @@ public void testObserverNameUsedInDifferentFile() {
         myFixture.configureByFile(getFixturePath(ModuleEventsXml.FILE_NAME));
         myFixture.testHighlighting(true, false, false);
     }
+
+    /**
+     * Tests warning for disabling of non-existing observer.
+     */
+    public void testDisablingNonExistingObserver() {
+        myFixture.configureByFile(getFixturePath(ModuleEventsXml.FILE_NAME));
+        myFixture.testHighlighting(true, false, false);
+    }
 }