Skip to content

Web API XML service tag inspection #577

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 11 commits into from
Apr 26, 2021
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
7 changes: 7 additions & 0 deletions resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,13 @@
enabledByDefault="true" level="ERROR"
implementationClass="com.magento.idea.magento2plugin.inspections.xml.AclResourceXmlInspection"/>

<localInspection language="XML" groupPath="XML"
shortName="WebApiServiceInspection"
displayName="Inspection for the Web API XML service declaration"
groupName="Magento 2"
enabledByDefault="true" level="WARNING"
implementationClass="com.magento.idea.magento2plugin.inspections.xml.WebApiServiceInspection"/>

<internalFileTemplate name="Magento Composer JSON"/>
<internalFileTemplate name="Magento Registration PHP"/>
<internalFileTemplate name="Magento Module XML"/>
Expand Down
8 changes: 6 additions & 2 deletions resources/magento2/inspection.properties
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,9 @@ inspection.observer.disabledObserverDoesNotExist=This observer does not exist to
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
inspection.aclResource.error.missingAttribute=Attribute "{0}" is required
inspection.aclResource.error.idAttributeCanNotBeEmpty=Attribute value "{0}" can not be empty
inspection.error.missingAttribute=Attribute "{0}" is required
inspection.error.idAttributeCanNotBeEmpty=Attribute value "{0}" can not be empty
inspection.warning.class.does.not.exist=The class "{0}" does not exist
inspection.warning.method.does.not.exist=The method "{0}" does not exist in the service class
inspection.warning.method.should.have.public.access=The method "{0}" should have public access
inspection.warning.method.should.have.public.access.fix=Change the method access
10 changes: 6 additions & 4 deletions src/com/magento/idea/magento2plugin/MagentoIcons.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
/**
/*
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

package com.magento.idea.magento2plugin;

import com.intellij.openapi.util.IconLoader;
import javax.swing.*;
import javax.swing.Icon;

@SuppressWarnings({"PMD.ClassNamingConventions"})
public class MagentoIcons {
public static final Icon WEB_API = IconLoader.getIcon("/icons/webapi.png");
public static final Icon MODULE = IconLoader.getIcon("/icons/module.png");
public static final Icon WEB_API = IconLoader.getIcon("/icons/webapi.png", MagentoIcons.class);
public static final Icon MODULE = IconLoader.getIcon("/icons/module.png", MagentoIcons.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.magento.idea.magento2plugin.bundles.InspectionBundle;
import com.magento.idea.magento2plugin.inspections.php.util.PhpClassImplementsInterfaceUtil;
import com.magento.idea.magento2plugin.inspections.php.util.PhpClassImplementsNoninterceptableInterfaceUtil;
import com.magento.idea.magento2plugin.magento.files.AbstractPhpFile;
import com.magento.idea.magento2plugin.magento.files.Plugin;
import com.magento.idea.magento2plugin.magento.packages.MagentoPhpClass;
import com.magento.idea.magento2plugin.magento.packages.Package;
Expand Down Expand Up @@ -166,7 +167,7 @@ private void checkTargetMethod(
ProblemHighlightType.ERROR
);
}
if (!targetMethod.getAccess().toString().equals(Plugin.PUBLIC_ACCESS)) {
if (!targetMethod.getAccess().toString().equals(AbstractPhpFile.PUBLIC_ACCESS)) {
problemsHolder.registerProblem(
pluginMethod.getNameIdentifier(),
inspectionBundle.message("inspection.plugin.error.nonPublicMethod"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void visitXmlTag(final XmlTag xmlTag) {
problemsHolder.registerProblem(
identifier,
inspectionBundle.message(
"inspection.aclResource.error.idAttributeCanNotBeEmpty",
"inspection.error.idAttributeCanNotBeEmpty",
"id"
),
ProblemHighlightType.WARNING
Expand All @@ -80,7 +80,7 @@ public void visitXmlTag(final XmlTag xmlTag) {
problemsHolder.registerProblem(
identifier,
inspectionBundle.message(
"inspection.aclResource.error.missingAttribute",
"inspection.error.missingAttribute",
"title"
),
ProblemHighlightType.WARNING
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/*
* 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.codeInspection.XmlSuppressableInspectionTool;
import com.intellij.psi.PsiElementVisitor;
import com.intellij.psi.PsiFile;
import com.intellij.psi.XmlElementVisitor;
import com.intellij.psi.xml.XmlAttribute;
import com.intellij.psi.xml.XmlTag;
import com.jetbrains.php.PhpIndex;
import com.jetbrains.php.lang.psi.elements.Method;
import com.jetbrains.php.lang.psi.elements.PhpClass;
import com.magento.idea.magento2plugin.bundles.InspectionBundle;
import com.magento.idea.magento2plugin.inspections.xml.fix.MethodNotPublicAccessQuickFix;
import com.magento.idea.magento2plugin.magento.files.ModuleWebApiXmlFile;
import java.util.Collection;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@SuppressWarnings({"PMD.ExcessiveMethodLength", "PMD.NPathComplexity"})
public class WebApiServiceInspection extends XmlSuppressableInspectionTool {

@NotNull
@Override
public PsiElementVisitor buildVisitor(
final @NotNull ProblemsHolder problemsHolder,
final boolean isOnTheFly
) {
return new XmlElementVisitor() {
private final InspectionBundle inspectionBundle = new InspectionBundle();

@Override
public void visitXmlTag(final XmlTag xmlTag) {
final PsiFile file = xmlTag.getContainingFile();
final String filename = file.getName();
if (!filename.equals(ModuleWebApiXmlFile.FILE_NAME)) {
return;
}

if (!xmlTag.getName().equals(ModuleWebApiXmlFile.SERVICE_TAG_NAME)) {
return;
}

//Check whether the class attribute is not empty
final XmlAttribute classAttribute = xmlTag.getAttribute(
ModuleWebApiXmlFile.CLASS_ATTR
);
if (classAttribute == null) {
return;
}
final String classFqn = classAttribute.getValue();
if (classFqn == null || classFqn.isEmpty()) {
problemsHolder.registerProblem(
classAttribute,
inspectionBundle.message(
"inspection.error.idAttributeCanNotBeEmpty",
ModuleWebApiXmlFile.CLASS_ATTR
),
ProblemHighlightType.WARNING
);

return;
}

//Check whether the class exists
final PhpIndex phpIndex = PhpIndex.getInstance(
problemsHolder.getProject()
);
@NotNull final Collection<PhpClass> classes = phpIndex.getClassesByFQN(classFqn);
@NotNull final Collection<PhpClass> interfaces = phpIndex
.getInterfacesByFQN(classFqn);
if (classes.isEmpty() && interfaces.isEmpty()) {
problemsHolder.registerProblem(
classAttribute,
inspectionBundle.message(
"inspection.warning.class.does.not.exist",
classFqn
),
ProblemHighlightType.WARNING
);
return;
}

//Check whether the method attribute is not empty
final XmlAttribute methodAttribute = xmlTag.getAttribute(
ModuleWebApiXmlFile.METHOD_ATTR
);
if (methodAttribute == null) {
return;
}
final String methodName = methodAttribute.getValue();
if (methodName == null || methodName.isEmpty()) {
problemsHolder.registerProblem(
classAttribute,
inspectionBundle.message(
"inspection.error.idAttributeCanNotBeEmpty",
ModuleWebApiXmlFile.METHOD_ATTR
),
ProblemHighlightType.WARNING
);

return;
}

//Check whether method exists
Method targetMethod = findTargetMethod(classes, methodName);
if (targetMethod == null) {
targetMethod = findTargetMethod(interfaces, methodName);
}
if (targetMethod == null && methodAttribute.getValueElement() != null) {
problemsHolder.registerProblem(
methodAttribute.getValueElement(),
inspectionBundle.message(
"inspection.warning.method.does.not.exist",
methodName
),
ProblemHighlightType.WARNING
);

return;
}

//API method should have public access
if (targetMethod.getAccess() != null && !targetMethod.getAccess().isPublic()) {
problemsHolder.registerProblem(
methodAttribute,
inspectionBundle.message(
"inspection.warning.method.should.have.public.access",
methodName
),
ProblemHighlightType.WARNING,
new MethodNotPublicAccessQuickFix(targetMethod)
);
}
}

@Nullable
private Method findTargetMethod(
final Collection<PhpClass> classes,
final String methodName
) {
for (final PhpClass phpClass : classes) {
for (final Method method : phpClass.getMethods()) {
if (method.getName().equals(methodName)) {
return method;
}
}
}
return null;
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

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

import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.jetbrains.php.lang.inspections.quickfix.PhpChangeMethodModifiersQuickFix;
import com.jetbrains.php.lang.psi.elements.Method;
import com.jetbrains.php.lang.psi.elements.PhpModifier;
import com.magento.idea.magento2plugin.bundles.InspectionBundle;
import org.jetbrains.annotations.NotNull;

public class MethodNotPublicAccessQuickFix implements LocalQuickFix {
private final InspectionBundle inspectionBundle = new InspectionBundle();
private final Method method;

public MethodNotPublicAccessQuickFix(final Method method) {
this.method = method;
}

@NotNull
@Override
public String getFamilyName() {
return inspectionBundle.message("inspection.warning.method.should.have.public.access.fix");
}

@Override
public void applyFix(
final @NotNull Project project,
final @NotNull ProblemDescriptor descriptor
) {
PhpChangeMethodModifiersQuickFix.changeMethodModifier(
this.method,
PhpModifier.PUBLIC_IMPLEMENTED_DYNAMIC
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
public abstract class AbstractPhpFile implements ModuleFileInterface {

public static final String FILE_EXTENSION = "php";

public static final String PUBLIC_ACCESS = "public";

private final String moduleName;
private final String className;
private NamespaceBuilder namespaceBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ public final class ModuleWebApiXmlFile implements ModuleFileInterface {
public static final String FILE_NAME = "webapi.xml";
public static final String TEMPLATE = "Magento Web Api XML";
public static final String DECLARATION_TEMPLATE = "Magento Web Api Declaration";
public static final String SERVICE_TAG_NAME = "service";
public static final String CLASS_ATTR = "class";
public static final String METHOD_ATTR = "method";

@Override
public String getFileName() {
Expand Down
3 changes: 0 additions & 3 deletions src/com/magento/idea/magento2plugin/magento/files/Plugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ public enum PluginType {
around//NOPMD
}

//allowed methods access type
public static final String PUBLIC_ACCESS = "public";

private String fileName;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package com.magento.idea.magento2plugin.util.magento.plugin;

import com.jetbrains.php.lang.psi.elements.Method;
import com.magento.idea.magento2plugin.magento.files.Plugin;
import com.magento.idea.magento2plugin.magento.files.AbstractPhpFile;
import com.magento.idea.magento2plugin.magento.packages.MagentoPhpClass;

public final class IsPluginAllowedForMethodUtil {
Expand All @@ -30,6 +30,6 @@ public static boolean check(final Method targetMethod) {
if (targetMethod.isStatic()) {
return false;
}
return targetMethod.getAccess().toString().equals(Plugin.PUBLIC_ACCESS);
return targetMethod.getAccess().toString().equals(AbstractPhpFile.PUBLIC_ACCESS);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Magento\Catalog\Api\ProductRepositoryInterface" method="some" />
</route>
</routes>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Magento\Catalog\Api\ProductRepositoryInterface" method="save" />
</route>
</routes>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Not\Existent\Class" method="some" />
</route>
</routes>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Magento\Catalog\Api\ProductRepositoryInterface" method="notExistent" />
</route>
</routes>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Foo\Bar\Service\SimpleService" method="myProtected" />
</route>
</routes>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0"?>
<routes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd">
<route method="GET" url="/V1/my-awesome-route">
<service class="Foo\Bar\Service\SimpleService" method="execute" />
</route>
</routes>
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ public function execute(int $param1, string $param2)
{

}

protected function myProtected(int $param1, string $param2)
{

}
}
Loading