Skip to content

Commit 6a217b1

Browse files
author
Vitaliy
authored
Merge pull request #59 from vasilii-b/observer-declaration-check
#27 [Inspection] Observer declaration check for duplicates
2 parents dc166be + af10b92 commit 6a217b1

File tree

5 files changed

+323
-1
lines changed

5 files changed

+323
-1
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* MFTF support (reference navigation, completion)
1010
* Fixed support of 2020.* versions of IDE's
1111
* Create a New Magento 2 Module action
12+
* Code Inspection: Duplicated Observer Usage in events XML
1213

1314
0.3.0
1415
=============

‎resources/META-INF/plugin.xml

+12-1
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,19 @@
102102

103103
<localInspection language="PHP" groupPath="PHP"
104104
shortName="PluginInspection" displayName="Inspection for the Plugin declaration"
105-
groupName="Magento" enabledByDefault="true" level="ERROR"
105+
groupName="Magento 2"
106+
enabledByDefault="true"
107+
level="ERROR"
106108
implementationClass="com.magento.idea.magento2plugin.inspections.php.PluginInspection"/>
109+
110+
<localInspection language="XML" groupPath="XML"
111+
shortName="ObserverDeclarationInspection"
112+
displayName="Duplicated Observer Usage in events XML"
113+
groupName="Magento 2"
114+
enabledByDefault="true"
115+
level="WARNING"
116+
implementationClass="com.magento.idea.magento2plugin.inspections.xml.ObserverDeclarationInspection"/>
117+
107118
<libraryRoot id=".phpstorm.meta.php" path=".phpstorm.meta.php/" runtime="false"/>
108119

109120
<internalFileTemplate name="Magento Module Composer"/>

‎resources/inspectionDescriptions/ObserverDeclarationInspection.html

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!--
2+
/*
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
-->
7+
8+
<html>
9+
<body>
10+
11+
<p>Observer names for events must be unique, otherwise overriding occurs. Overrides can be done by accident.</p>
12+
<p>
13+
In case overriding is wanted,
14+
<a href="https://devdocs.magento.com/guides/v2.3/extension-dev-guide/events-and-observers.html#disabling-an-observer">
15+
disable the original observer
16+
</a>
17+
and give a unique name to the current observer.
18+
</p>
19+
<p>
20+
<a href="https://devdocs.magento.com/guides/v2.3/extension-dev-guide/events-and-observers.html">Read more about Events & Observers</a>
21+
</p>
22+
</body>
23+
</html>

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

+280
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,280 @@
1+
/*
2+
* Copyright © Magento, Inc. All rights reserved.
3+
* See COPYING.txt for license details.
4+
*/
5+
6+
package com.magento.idea.magento2plugin.inspections.xml;
7+
8+
import com.intellij.codeInspection.ProblemHighlightType;
9+
import com.intellij.codeInspection.ProblemsHolder;
10+
import com.intellij.ide.highlighter.XmlFileType;
11+
import com.intellij.openapi.vfs.VirtualFile;
12+
import com.intellij.psi.*;
13+
import com.intellij.psi.search.GlobalSearchScope;
14+
import com.intellij.psi.util.PsiTreeUtil;
15+
import com.intellij.psi.xml.XmlAttribute;
16+
import com.intellij.psi.xml.XmlAttributeValue;
17+
import com.intellij.psi.xml.XmlDocument;
18+
import com.intellij.psi.xml.XmlTag;
19+
import com.jetbrains.php.lang.inspections.PhpInspection;
20+
import com.magento.idea.magento2plugin.indexes.EventIndex;
21+
import com.magento.idea.magento2plugin.magento.files.ModuleXml;
22+
import com.magento.idea.magento2plugin.magento.packages.MagentoPackages;
23+
import org.jetbrains.annotations.NotNull;
24+
import java.net.MalformedURLException;
25+
import java.net.URL;
26+
import java.util.*;
27+
28+
import com.intellij.openapi.vfs.VfsUtil;
29+
import org.jetbrains.annotations.Nullable;
30+
31+
public class ObserverDeclarationInspection extends PhpInspection {
32+
33+
@NotNull
34+
@Override
35+
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder problemsHolder, boolean b) {
36+
return new XmlElementVisitor() {
37+
private final String moduleXmlFileName = ModuleXml.getInstance().getFileName();
38+
private static final String eventsXmlFileName = "events.xml";
39+
private static final String duplicatedObserverNameSameFileProblemDescription = "The observer name already used in this file. For more details see Inspection Description.";
40+
private static final String duplicatedObserverNameProblemDescription =
41+
"The observer name \"%s\" for event \"%s\" is already used in the module \"%s\" (%s scope). For more details see Inspection Description.";
42+
private HashMap<String, VirtualFile> loadedFileHash = new HashMap<>();
43+
private final ProblemHighlightType errorSeverity = ProblemHighlightType.WARNING;
44+
45+
@Override
46+
public void visitFile(PsiFile file) {
47+
if (!file.getName().equals(eventsXmlFileName)) {
48+
return;
49+
}
50+
51+
XmlTag[] xmlTags = getFileXmlTags(file);
52+
EventIndex eventIndex = EventIndex.getInstance(file.getProject());
53+
54+
if (xmlTags == null) {
55+
return;
56+
}
57+
58+
HashMap<String, XmlTag> targetObserversHash = new HashMap<>();
59+
60+
for (XmlTag eventXmlTag: xmlTags) {
61+
HashMap<String, XmlTag> eventProblems = new HashMap<>();
62+
if (!eventXmlTag.getName().equals("event")) {
63+
continue;
64+
}
65+
66+
XmlAttribute eventNameAttribute = eventXmlTag.getAttribute("name");
67+
68+
String eventNameAttributeValue = eventNameAttribute.getValue();
69+
if (eventNameAttributeValue == null) {
70+
continue;
71+
}
72+
73+
List<XmlTag> targetObservers = fetchObserverTagsFromEventTag(eventXmlTag);
74+
75+
for (XmlTag observerXmlTag: targetObservers) {
76+
XmlAttribute observerNameAttribute = observerXmlTag.getAttribute("name");
77+
XmlAttribute observerDisabledAttribute = observerXmlTag.getAttribute("disabled");
78+
79+
if (observerNameAttribute == null || (observerDisabledAttribute != null && observerDisabledAttribute.getValue().equals("true"))) {
80+
continue;
81+
}
82+
83+
String observerName = observerNameAttribute.getValue();
84+
String observerKey = eventNameAttributeValue.concat("_").concat(observerName);
85+
if (targetObserversHash.containsKey(observerKey)) {
86+
problemsHolder.registerProblem(
87+
observerNameAttribute.getValueElement(),
88+
duplicatedObserverNameSameFileProblemDescription,
89+
errorSeverity
90+
);
91+
}
92+
targetObserversHash.put(observerKey, observerXmlTag);
93+
94+
List<HashMap<String, String>> modulesWithSameObserverName = fetchModuleNamesWhereSameObserverNameUsed(eventNameAttributeValue, observerName, eventIndex, file);
95+
for (HashMap<String, String> moduleEntry: modulesWithSameObserverName) {
96+
Map.Entry<String, String> module = moduleEntry.entrySet().iterator().next();
97+
String moduleName = module.getKey();
98+
String scope = module.getValue();
99+
String problemKey = observerKey.concat("_").concat(moduleName).concat("_").concat(scope);
100+
if (!eventProblems.containsKey(problemKey)){
101+
problemsHolder.registerProblem(
102+
observerNameAttribute.getValueElement(),
103+
String.format(
104+
duplicatedObserverNameProblemDescription,
105+
observerName,
106+
eventNameAttributeValue,
107+
moduleName,
108+
scope
109+
),
110+
errorSeverity
111+
);
112+
eventProblems.put(problemKey, observerXmlTag);
113+
}
114+
}
115+
}
116+
}
117+
}
118+
119+
private List<HashMap<String, String>> fetchModuleNamesWhereSameObserverNameUsed(String eventNameAttributeValue, String observerName, EventIndex eventIndex, PsiFile file) {
120+
List<HashMap<String, String>> modulesName = new ArrayList<>();
121+
String currentFileDirectory = file.getContainingDirectory().toString();
122+
String currentFileFullPath = currentFileDirectory.concat("/").concat(file.getName());
123+
124+
Collection<PsiElement> indexedEvents = eventIndex.getEventElements(eventNameAttributeValue, GlobalSearchScope.getScopeRestrictedByFileTypes(
125+
GlobalSearchScope.allScope(file.getProject()),
126+
XmlFileType.INSTANCE
127+
));
128+
129+
for (PsiElement indexedEvent: indexedEvents) {
130+
PsiFile indexedAttributeParent = PsiTreeUtil.getTopmostParentOfType(indexedEvent, PsiFile.class);
131+
if (indexedAttributeParent == null) {
132+
continue;
133+
}
134+
135+
String indexedEventAttributeValue = ((XmlAttributeValue) indexedEvent).getValue();
136+
if (!indexedEventAttributeValue.equals(eventNameAttributeValue)) {
137+
continue;
138+
}
139+
140+
String indexedFileDirectory = indexedAttributeParent.getContainingDirectory().toString();
141+
String indexedFileFullPath = indexedFileDirectory.concat("/").concat(indexedAttributeParent.getName());
142+
if (indexedFileFullPath.equals(currentFileFullPath)) {
143+
continue;
144+
}
145+
146+
String scope = getAreaFromFileDirectory(indexedAttributeParent);
147+
148+
List<XmlTag> indexObserversTags = fetchObserverTagsFromEventTag((XmlTag) indexedEvent.getParent().getParent());
149+
for (XmlTag indexObserversTag: indexObserversTags) {
150+
XmlAttribute indexedObserverNameAttribute = indexObserversTag.getAttribute("name");
151+
if (indexedObserverNameAttribute == null) {
152+
continue;
153+
}
154+
if (!observerName.equals(indexedObserverNameAttribute.getValue())){
155+
continue;
156+
}
157+
addModuleNameWhereSameObserverUsed(modulesName, indexedAttributeParent, scope);
158+
}
159+
}
160+
161+
return modulesName;
162+
}
163+
164+
private List<XmlTag> fetchObserverTagsFromEventTag(XmlTag eventXmlTag) {
165+
List<XmlTag> result = new ArrayList<>();
166+
XmlTag[] observerXmlTags = PsiTreeUtil.getChildrenOfType(eventXmlTag, XmlTag.class);
167+
if (observerXmlTags == null) {
168+
return result;
169+
}
170+
171+
for (XmlTag observerXmlTag: observerXmlTags) {
172+
if (!observerXmlTag.getName().equals("observer")) {
173+
continue;
174+
}
175+
176+
result.add(observerXmlTag);
177+
}
178+
179+
return result;
180+
}
181+
182+
private void addModuleNameWhereSameObserverUsed(List<HashMap<String, String>> modulesName, PsiFile indexedFile, String scope) {
183+
XmlTag moduleDeclarationTag = getModuleDeclarationTagByConfigFile(indexedFile);
184+
if (moduleDeclarationTag == null) return;
185+
186+
if (!moduleDeclarationTag.getName().equals("module")) {
187+
return;
188+
}
189+
XmlAttribute moduleNameAttribute = moduleDeclarationTag.getAttribute("name");
190+
if (moduleNameAttribute == null) {
191+
return;
192+
}
193+
194+
HashMap<String, String> moduleEntry = new HashMap<>();
195+
196+
moduleEntry.put(moduleNameAttribute.getValue(), scope);
197+
modulesName.add(moduleEntry);
198+
}
199+
200+
@Nullable
201+
private XmlTag getModuleDeclarationTagByConfigFile(PsiFile file) {
202+
String fileDirectory = file.getContainingDirectory().toString();
203+
String fileArea = file.getContainingDirectory().getName();
204+
String moduleXmlFilePath = getModuleXmlFilePathByConfigFileDirectory(fileDirectory, fileArea);
205+
206+
VirtualFile virtualFile = getFileByPath(moduleXmlFilePath);
207+
if (virtualFile == null) return null;
208+
209+
PsiFile moduleDeclarationFile = PsiManager.getInstance(file.getProject()).findFile(virtualFile);
210+
XmlTag[] moduleDeclarationTags = getFileXmlTags(moduleDeclarationFile);
211+
if (moduleDeclarationTags == null) {
212+
return null;
213+
}
214+
return moduleDeclarationTags[0];
215+
}
216+
217+
@Nullable
218+
private VirtualFile getFileByPath(String moduleXmlFilePath) {
219+
if (loadedFileHash.containsKey(moduleXmlFilePath)) {
220+
return loadedFileHash.get(moduleXmlFilePath);
221+
}
222+
VirtualFile virtualFile;
223+
try {
224+
virtualFile = VfsUtil.findFileByURL(new URL(moduleXmlFilePath));
225+
} catch (MalformedURLException e) {
226+
return null;
227+
}
228+
if (virtualFile == null) {
229+
return null;
230+
}
231+
loadedFileHash.put(moduleXmlFilePath, virtualFile);
232+
return virtualFile;
233+
}
234+
235+
private String getModuleXmlFilePathByConfigFileDirectory(String fileDirectory, String fileArea) {
236+
String moduleXmlFile = fileDirectory.replace(fileArea, "").concat(moduleXmlFileName);
237+
if (fileDirectory.endsWith("etc")) {
238+
moduleXmlFile = fileDirectory.concat("/").concat(moduleXmlFileName);
239+
}
240+
return moduleXmlFile.replace("PsiDirectory:", "file:");
241+
}
242+
243+
@Nullable
244+
private XmlTag[] getFileXmlTags(PsiFile file) {
245+
XmlDocument xmlDocument = PsiTreeUtil.getChildOfType(file, XmlDocument.class);
246+
XmlTag xmlRootTag = PsiTreeUtil.getChildOfType(xmlDocument, XmlTag.class);
247+
return PsiTreeUtil.getChildrenOfType(xmlRootTag, XmlTag.class);
248+
}
249+
250+
private String getAreaFromFileDirectory(@NotNull PsiFile file) {
251+
if (file.getParent() == null) {
252+
return "";
253+
}
254+
255+
String areaFromFileDirectory = file.getParent().getName();
256+
257+
if (areaFromFileDirectory.equals("etc")) {
258+
return MagentoPackages.AREA_BASE;
259+
}
260+
261+
List<String> possibleAreas = new ArrayList<>(Arrays.asList(
262+
MagentoPackages.AREA_ADMINHTML,
263+
MagentoPackages.AREA_FRONTEND,
264+
MagentoPackages.AREA_CRON,
265+
MagentoPackages.AREA_API_REST,
266+
MagentoPackages.AREA_API_SOAP,
267+
MagentoPackages.AREA_GRAPHQL
268+
));
269+
270+
for (String area: possibleAreas) {
271+
if (area.equals(areaFromFileDirectory)) {
272+
return area;
273+
}
274+
}
275+
276+
return "";
277+
}
278+
};
279+
}
280+
}

‎src/com/magento/idea/magento2plugin/magento/packages/MagentoPackages.java

+7
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,11 @@
66

77
public class MagentoPackages {
88
public static String PACKAGES_ROOT = "app/code";
9+
public static String AREA_BASE = "base";
10+
public static String AREA_ADMINHTML = "adminhtml";
11+
public static String AREA_FRONTEND = "frontend";
12+
public static String AREA_CRON = "crontab";
13+
public static String AREA_API_REST = "webapi_rest";
14+
public static String AREA_API_SOAP = "webapi_soap";
15+
public static String AREA_GRAPHQL = "graphql";
916
}

0 commit comments

Comments
 (0)