Skip to content

Added inspection warning for disabled observer #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions resources/magento2/inspection.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}