Skip to content
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.

Sign up for GitHub

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

#27 [Inspection] Observer declaration check for duplicates #59

Merged
merged 8 commits into from
Mar 27, 2020
Merged

#27 [Inspection] Observer declaration check for duplicates #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eca74bd
Observer declaration check added
Mar 10, 2020
52b1b8b
Merge branch '1.0.0-develop' of https://github.com/magento/magento2-p…
Mar 25, 2020
2a3bf94
Adjusted the duplicated observer name inspection
Mar 25, 2020
aeca5fe
Added missed adjustments to the plugin
Mar 25, 2020
7aef516
Merge branch '1.0.0-develop' of https://github.com/magento/magento2-p…
Mar 25, 2020
9515e3d
Added requested changes by the reviewer
Mar 26, 2020
ae4c0e8
Merge branch 'observer-declaration-check' of https://github.com/vasil…
Mar 26, 2020
af10b92
Added the missing area scope into the check
Mar 26, 2020
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 CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* MFTF support (reference navigation, completion)
* Fixed support of 2020.* versions of IDE's
* Create a New Magento 2 Module action
* Code Inspection: Duplicated Observer Usage in events XML

0.3.0
=============
Expand Down
13 changes: 12 additions & 1 deletion resources/META-INF/plugin.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,19 @@

<localInspection language="PHP" groupPath="PHP"
shortName="PluginInspection" displayName="Inspection for the Plugin declaration"
groupName="Magento" enabledByDefault="true" level="ERROR"
groupName="Magento 2"
enabledByDefault="true"
level="ERROR"
implementationClass="com.magento.idea.magento2plugin.inspections.php.PluginInspection"/>

<localInspection language="XML" groupPath="XML"
shortName="ObserverDeclarationInspection"
displayName="Duplicated Observer Usage in events XML"
groupName="Magento 2"
enabledByDefault="true"
level="WARNING"
implementationClass="com.magento.idea.magento2plugin.inspections.xml.ObserverDeclarationInspection"/>

<libraryRoot id=".phpstorm.meta.php" path=".phpstorm.meta.php/" runtime="false"/>

<internalFileTemplate name="Magento Module Composer"/>
Expand Down
23 changes: 23 additions & 0 deletions resources/inspectionDescriptions/ObserverDeclarationInspection.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!--
/*
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->

<html>
<body>

<p>Observer names for events must be unique, otherwise overriding occurs. Overrides can be done by accident.</p>
<p>
In case overriding is wanted,
<a href="https://devdocs.magento.com/guides/v2.3/extension-dev-guide/events-and-observers.html#disabling-an-observer">
disable the original observer
</a>
and give a unique name to the current observer.
</p>
<p>
<a href="https://devdocs.magento.com/guides/v2.3/extension-dev-guide/events-and-observers.html">Read more about Events & Observers</a>
</p>
</body>
</html>
280 changes: 280 additions & 0 deletions src/com/magento/idea/magento2plugin/inspections/xml/ObserverDeclarationInspection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,280 @@
/*
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

package com.magento.idea.magento2plugin.inspections.xml;

import com.intellij.codeInspection.ProblemHighlightType;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.ide.highlighter.XmlFileType;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.*;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.xml.XmlAttribute;
import com.intellij.psi.xml.XmlAttributeValue;
import com.intellij.psi.xml.XmlDocument;
import com.intellij.psi.xml.XmlTag;
import com.jetbrains.php.lang.inspections.PhpInspection;
import com.magento.idea.magento2plugin.indexes.EventIndex;
import com.magento.idea.magento2plugin.magento.files.ModuleXml;
import com.magento.idea.magento2plugin.magento.packages.MagentoPackages;
import org.jetbrains.annotations.NotNull;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.*;

import com.intellij.openapi.vfs.VfsUtil;
import org.jetbrains.annotations.Nullable;

public class ObserverDeclarationInspection extends PhpInspection {

@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder problemsHolder, boolean b) {
return new XmlElementVisitor() {
private final String moduleXmlFileName = ModuleXml.getInstance().getFileName();
private static final String eventsXmlFileName = "events.xml";
private static final String duplicatedObserverNameSameFileProblemDescription = "The observer name already used in this file. For more details see Inspection Description.";
private static final String duplicatedObserverNameProblemDescription =
"The observer name \"%s\" for event \"%s\" is already used in the module \"%s\" (%s scope). For more details see Inspection Description.";
private HashMap<String, VirtualFile> loadedFileHash = new HashMap<>();
private final ProblemHighlightType errorSeverity = ProblemHighlightType.WARNING;

@Override
public void visitFile(PsiFile file) {
if (!file.getName().equals(eventsXmlFileName)) {
return;
}

XmlTag[] xmlTags = getFileXmlTags(file);
EventIndex eventIndex = EventIndex.getInstance(file.getProject());

if (xmlTags == null) {
return;
}

HashMap<String, XmlTag> targetObserversHash = new HashMap<>();

for (XmlTag eventXmlTag: xmlTags) {
HashMap<String, XmlTag> eventProblems = new HashMap<>();
if (!eventXmlTag.getName().equals("event")) {
continue;
}

XmlAttribute eventNameAttribute = eventXmlTag.getAttribute("name");

String eventNameAttributeValue = eventNameAttribute.getValue();
if (eventNameAttributeValue == null) {
continue;
}

List<XmlTag> targetObservers = fetchObserverTagsFromEventTag(eventXmlTag);

for (XmlTag observerXmlTag: targetObservers) {
XmlAttribute observerNameAttribute = observerXmlTag.getAttribute("name");
XmlAttribute observerDisabledAttribute = observerXmlTag.getAttribute("disabled");

if (observerNameAttribute == null || (observerDisabledAttribute != null && observerDisabledAttribute.getValue().equals("true"))) {
continue;
}

String observerName = observerNameAttribute.getValue();
String observerKey = eventNameAttributeValue.concat("_").concat(observerName);
if (targetObserversHash.containsKey(observerKey)) {
problemsHolder.registerProblem(
observerNameAttribute.getValueElement(),
duplicatedObserverNameSameFileProblemDescription,
errorSeverity
);
}
targetObserversHash.put(observerKey, observerXmlTag);

List<HashMap<String, String>> modulesWithSameObserverName = fetchModuleNamesWhereSameObserverNameUsed(eventNameAttributeValue, observerName, eventIndex, file);
for (HashMap<String, String> moduleEntry: modulesWithSameObserverName) {
Map.Entry<String, String> module = moduleEntry.entrySet().iterator().next();
String moduleName = module.getKey();
String scope = module.getValue();
String problemKey = observerKey.concat("_").concat(moduleName).concat("_").concat(scope);
if (!eventProblems.containsKey(problemKey)){
problemsHolder.registerProblem(
observerNameAttribute.getValueElement(),
String.format(
duplicatedObserverNameProblemDescription,
observerName,
eventNameAttributeValue,
moduleName,
scope
),
errorSeverity
);
eventProblems.put(problemKey, observerXmlTag);
}
}
}
}
}

private List<HashMap<String, String>> fetchModuleNamesWhereSameObserverNameUsed(String eventNameAttributeValue, String observerName, EventIndex eventIndex, PsiFile file) {
List<HashMap<String, String>> modulesName = new ArrayList<>();
String currentFileDirectory = file.getContainingDirectory().toString();
String currentFileFullPath = currentFileDirectory.concat("/").concat(file.getName());

Collection<PsiElement> indexedEvents = eventIndex.getEventElements(eventNameAttributeValue, GlobalSearchScope.getScopeRestrictedByFileTypes(
GlobalSearchScope.allScope(file.getProject()),
XmlFileType.INSTANCE
));

for (PsiElement indexedEvent: indexedEvents) {
PsiFile indexedAttributeParent = PsiTreeUtil.getTopmostParentOfType(indexedEvent, PsiFile.class);
if (indexedAttributeParent == null) {
continue;
}

String indexedEventAttributeValue = ((XmlAttributeValue) indexedEvent).getValue();
if (!indexedEventAttributeValue.equals(eventNameAttributeValue)) {
continue;
}

String indexedFileDirectory = indexedAttributeParent.getContainingDirectory().toString();
String indexedFileFullPath = indexedFileDirectory.concat("/").concat(indexedAttributeParent.getName());
if (indexedFileFullPath.equals(currentFileFullPath)) {
continue;
}

String scope = getAreaFromFileDirectory(indexedAttributeParent);

List<XmlTag> indexObserversTags = fetchObserverTagsFromEventTag((XmlTag) indexedEvent.getParent().getParent());
for (XmlTag indexObserversTag: indexObserversTags) {
XmlAttribute indexedObserverNameAttribute = indexObserversTag.getAttribute("name");
if (indexedObserverNameAttribute == null) {
continue;
}
if (!observerName.equals(indexedObserverNameAttribute.getValue())){
continue;
}
addModuleNameWhereSameObserverUsed(modulesName, indexedAttributeParent, scope);
}
}

return modulesName;
}

private List<XmlTag> fetchObserverTagsFromEventTag(XmlTag eventXmlTag) {
List<XmlTag> result = new ArrayList<>();
XmlTag[] observerXmlTags = PsiTreeUtil.getChildrenOfType(eventXmlTag, XmlTag.class);
if (observerXmlTags == null) {
return result;
}

for (XmlTag observerXmlTag: observerXmlTags) {
if (!observerXmlTag.getName().equals("observer")) {
continue;
}

result.add(observerXmlTag);
}

return result;
}

private void addModuleNameWhereSameObserverUsed(List<HashMap<String, String>> modulesName, PsiFile indexedFile, String scope) {
XmlTag moduleDeclarationTag = getModuleDeclarationTagByConfigFile(indexedFile);
if (moduleDeclarationTag == null) return;

if (!moduleDeclarationTag.getName().equals("module")) {
return;
}
XmlAttribute moduleNameAttribute = moduleDeclarationTag.getAttribute("name");
if (moduleNameAttribute == null) {
return;
}

HashMap<String, String> moduleEntry = new HashMap<>();

moduleEntry.put(moduleNameAttribute.getValue(), scope);
modulesName.add(moduleEntry);
}

@Nullable
private XmlTag getModuleDeclarationTagByConfigFile(PsiFile file) {
String fileDirectory = file.getContainingDirectory().toString();
String fileArea = file.getContainingDirectory().getName();
String moduleXmlFilePath = getModuleXmlFilePathByConfigFileDirectory(fileDirectory, fileArea);

VirtualFile virtualFile = getFileByPath(moduleXmlFilePath);
if (virtualFile == null) return null;

PsiFile moduleDeclarationFile = PsiManager.getInstance(file.getProject()).findFile(virtualFile);
XmlTag[] moduleDeclarationTags = getFileXmlTags(moduleDeclarationFile);
if (moduleDeclarationTags == null) {
return null;
}
return moduleDeclarationTags[0];
}

@Nullable
private VirtualFile getFileByPath(String moduleXmlFilePath) {
if (loadedFileHash.containsKey(moduleXmlFilePath)) {
return loadedFileHash.get(moduleXmlFilePath);
}
VirtualFile virtualFile;
try {
virtualFile = VfsUtil.findFileByURL(new URL(moduleXmlFilePath));
} catch (MalformedURLException e) {
return null;
}
if (virtualFile == null) {
return null;
}
loadedFileHash.put(moduleXmlFilePath, virtualFile);
return virtualFile;
}

private String getModuleXmlFilePathByConfigFileDirectory(String fileDirectory, String fileArea) {
String moduleXmlFile = fileDirectory.replace(fileArea, "").concat(moduleXmlFileName);
if (fileDirectory.endsWith("etc")) {
moduleXmlFile = fileDirectory.concat("/").concat(moduleXmlFileName);
}
return moduleXmlFile.replace("PsiDirectory:", "file:");
}

@Nullable
private XmlTag[] getFileXmlTags(PsiFile file) {
XmlDocument xmlDocument = PsiTreeUtil.getChildOfType(file, XmlDocument.class);
XmlTag xmlRootTag = PsiTreeUtil.getChildOfType(xmlDocument, XmlTag.class);
return PsiTreeUtil.getChildrenOfType(xmlRootTag, XmlTag.class);
}

private String getAreaFromFileDirectory(@NotNull PsiFile file) {
if (file.getParent() == null) {
return "";
}

String areaFromFileDirectory = file.getParent().getName();

if (areaFromFileDirectory.equals("etc")) {
return MagentoPackages.AREA_BASE;
}

List<String> possibleAreas = new ArrayList<>(Arrays.asList(
MagentoPackages.AREA_ADMINHTML,
MagentoPackages.AREA_FRONTEND,
MagentoPackages.AREA_CRON,
MagentoPackages.AREA_API_REST,
MagentoPackages.AREA_API_SOAP,
MagentoPackages.AREA_GRAPHQL
));

for (String area: possibleAreas) {
if (area.equals(areaFromFileDirectory)) {
return area;
}
}

return "";
}
};
}
}
7 changes: 7 additions & 0 deletions src/com/magento/idea/magento2plugin/magento/packages/MagentoPackages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,11 @@

public class MagentoPackages {
public static String PACKAGES_ROOT = "app/code";
public static String AREA_BASE = "base";
public static String AREA_ADMINHTML = "adminhtml";
public static String AREA_FRONTEND = "frontend";
public static String AREA_CRON = "crontab";
public static String AREA_API_REST = "webapi_rest";
public static String AREA_API_SOAP = "webapi_soap";
public static String AREA_GRAPHQL = "graphql";
}