phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[apex] Fix ApexCRUDViolation rule

Browse Source 
- reflection not necessary anymore
- constructor is now not anymore returned
as <init> but as the actual name
This commit is contained in:
Andreas Dangel
2017-12-03 19:53:11 +01:00
parent 7f8c8a0854
commit dbcd36cd7a
1 changed files with 32 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java
View File

@ -4,14 +4,12 @@
package net.sourceforge.pmd.lang.apex.rule.security; package net.sourceforge.pmd.lang.apex.rule.security;
import java.lang.reflect.Field;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.WeakHashMap; import java.util.WeakHashMap;
import java.util.regex.Matcher; import java.util.regex.Matcher;
@ -66,6 +64,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
private final ListMultimap<String, String> typeToDMLOperationMapping = ArrayListMultimap.create(); private final ListMultimap<String, String> typeToDMLOperationMapping = ArrayListMultimap.create();
private final HashMap<String, String> checkedTypeToDMLOperationViaESAPI = new HashMap<>(); private final HashMap<String, String> checkedTypeToDMLOperationViaESAPI = new HashMap<>();
private final WeakHashMap<String, ASTMethod> classMethods = new WeakHashMap<>(); private final WeakHashMap<String, ASTMethod> classMethods = new WeakHashMap<>();
private String className;
private static final String IS_CREATEABLE = "isCreateable"; private static final String IS_CREATEABLE = "isCreateable";
private static final String IS_DELETABLE = "isDeletable"; private static final String IS_DELETABLE = "isDeletable";
@ -100,6 +99,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
return data; // stops all the rules return data; // stops all the rules
} }
className = node.getImage();
for (ASTMethod n : node.findDescendantsOfType(ASTMethod.class)) { for (ASTMethod n : node.findDescendantsOfType(ASTMethod.class)) {
StringBuilder sb = new StringBuilder().append(n.getNode().getDefiningType().getApexName()).append(":") StringBuilder sb = new StringBuilder().append(n.getNode().getDefiningType().getApexName()).append(":")
.append(n.getNode().getMethodInfo().getCanonicalName()).append(":") .append(n.getNode().getMethodInfo().getCanonicalName()).append(":")
@ -107,8 +108,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
classMethods.put(sb.toString(), n); classMethods.put(sb.toString(), n);
} }
node.childrenAccept(this, data); return super.visit(node, data);
return data;
} }
@Override @Override
@ -176,19 +176,13 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
public Object visit(final ASTFieldDeclaration node, Object data) { public Object visit(final ASTFieldDeclaration node, Object data) {
ASTFieldDeclarationStatements field = node.getFirstParentOfType(ASTFieldDeclarationStatements.class); ASTFieldDeclarationStatements field = node.getFirstParentOfType(ASTFieldDeclarationStatements.class);
if (field != null) { if (field != null) {
try {
TypeRef a = field.getNode().getTypeName(); TypeRef a = field.getNode().getTypeName();
Field classNameField = a.getClass().getDeclaredField("className"); List<Identifier> names = a.getNames();
Field typeArgsField = a.getClass().getDeclaredField("typeArguments"); List<TypeRef> typeArgs = a.getTypeArguments();
classNameField.setAccessible(true);
typeArgsField.setAccessible(true);
if (classNameField.get(a) instanceof ArrayList<?>) { if (!names.isEmpty()) {
@SuppressWarnings("unchecked")
ArrayList<Identifier> innerField = (ArrayList<Identifier>) classNameField.get(a);
if (!innerField.isEmpty()) {
StringBuffer sb = new StringBuffer(); StringBuffer sb = new StringBuffer();
for (Identifier id : innerField) { for (Identifier id : names) {
sb.append(id.getValue()).append("."); sb.append(id.getValue()).append(".");
} }
sb.deleteCharAt(sb.length() - 1); sb.deleteCharAt(sb.length() - 1);
@ -196,9 +190,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
switch (sb.toString().toLowerCase()) { switch (sb.toString().toLowerCase()) {
case "list": case "list":
case "map": case "map":
if (typeArgsField.get(a) instanceof Optional<?>) { addParametersToMapping(node, typeArgs);
addParametersToMapping(node, a, typeArgsField);
}
break; break;
default: default:
varToTypeMapping.put(Helper.getFQVariableName(node), getSimpleType(sb.toString())); varToTypeMapping.put(Helper.getFQVariableName(node), getSimpleType(sb.toString()));
@ -207,12 +199,6 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
} }
} }
} catch (NoSuchFieldException | IllegalArgumentException | IllegalAccessException | SecurityException e) {
e.printStackTrace();
}
}
final ASTSoqlExpression soql = node.getFirstChildOfType(ASTSoqlExpression.class); final ASTSoqlExpression soql = node.getFirstChildOfType(ASTSoqlExpression.class);
if (soql != null) { if (soql != null) {
checkForAccessibility(soql, data); checkForAccessibility(soql, data);
@ -222,23 +208,17 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
} }
private void addParametersToMapping(final ASTFieldDeclaration node, TypeRef a, Field typeArgsField) private void addParametersToMapping(final ASTFieldDeclaration node, List<TypeRef> typeArgs) {
throws IllegalAccessException { for (int i = 0; i < typeArgs.size(); i++) {
Optional<?> optionalContainer = (Optional<?>) typeArgsField.get(a); if (typeArgs.get(i) instanceof ClassTypeRef) {
if (optionalContainer.isPresent()) { innerAddParametrizedClassToMapping(node, (ClassTypeRef) typeArgs.get(i));
ArrayList<?> inner = (ArrayList<?>) optionalContainer.get();
for (int i = 0; i < inner.size(); i++) {
if (inner.get(i) instanceof ClassTypeRef) {
innerAddParametrizedClassToMapping(node, (ClassTypeRef) inner.get(i));
} }
if (inner.get(i) instanceof ArrayTypeRef) { if (typeArgs.get(i) instanceof ArrayTypeRef) {
ArrayTypeRef atr = (ArrayTypeRef) inner.get(i); ArrayTypeRef atr = (ArrayTypeRef) typeArgs.get(i);
if (atr.getHeldType() instanceof ClassTypeRef) { if (atr.getHeldType() instanceof ClassTypeRef) {
innerAddParametrizedClassToMapping(node, (ClassTypeRef) atr.getHeldType()); innerAddParametrizedClassToMapping(node, (ClassTypeRef) atr.getHeldType());
} }
} }
}
} }
} }
@ -470,7 +450,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
private List<ASTMethod> findConstructorlMethods(final AbstractApexNode<?> node) { private List<ASTMethod> findConstructorlMethods(final AbstractApexNode<?> node) {
final ArrayList<ASTMethod> ret = new ArrayList<>(); final ArrayList<ASTMethod> ret = new ArrayList<>();
final Set<String> constructors = classMethods.keySet().stream() final Set<String> constructors = classMethods.keySet().stream()
.filter(p -> (p.contains("<init>") || p.contains("<clinit>"))).collect(Collectors.toSet()); .filter(p -> (p.contains("<init>") || p.contains("<clinit>")
|| p.startsWith(className + ":" + className + ":"))).collect(Collectors.toSet());
for (String c : constructors) { for (String c : constructors) {
ret.add(classMethods.get(c)); ret.add(classMethods.get(c));