From 383713fdbdb2ecc4f68493caff256220ddb35fc2 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 14 Mar 2020 18:16:52 +0100 Subject: [PATCH] [apex] Small refactorings in ApexCRUDViolationRule To avoid some deprecations --- .../rule/security/ApexCRUDViolationRule.java | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java index 2dd0cff20b..f1571cd01c 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java @@ -40,7 +40,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration; import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression; -import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; import net.sourceforge.pmd.lang.apex.rule.internal.Helper; import net.sourceforge.pmd.lang.ast.Node; @@ -64,12 +64,6 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern SELECT_FROM_PATTERN = Pattern.compile("[\\S|\\s]+?FROM[\\s]+?(\\w+)", Pattern.CASE_INSENSITIVE); - private final Map varToTypeMapping = new HashMap<>(); - private final ListMultimap typeToDMLOperationMapping = ArrayListMultimap.create(); - private final Map checkedTypeToDMLOperationViaESAPI = new HashMap<>(); - private final Map classMethods = new WeakHashMap<>(); - private String className; - private static final String IS_CREATEABLE = "isCreateable"; private static final String IS_DELETABLE = "isDeletable"; private static final String IS_UPDATEABLE = "isUpdateable"; @@ -93,6 +87,12 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?i).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); + private final Map varToTypeMapping = new HashMap<>(); + private final ListMultimap typeToDMLOperationMapping = ArrayListMultimap.create(); + private final Map checkedTypeToDMLOperationViaESAPI = new HashMap<>(); + private final Map classMethods = new WeakHashMap<>(); + private String className; + public ApexCRUDViolationRule() { setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); @@ -187,19 +187,15 @@ public class ApexCRUDViolationRule extends AbstractApexRule { List typeArgs = a.getTypeArguments(); if (!names.isEmpty()) { - StringBuffer sb = new StringBuffer(); - for (Identifier id : names) { - sb.append(id.getValue()).append("."); - } - sb.deleteCharAt(sb.length() - 1); + String namesString = names.stream().map(Identifier::getValue).collect(Collectors.joining(".")); - switch (sb.toString().toLowerCase(Locale.ROOT)) { + switch (namesString.toLowerCase(Locale.ROOT)) { case "list": case "map": addParametersToMapping(node, typeArgs); break; default: - varToTypeMapping.put(Helper.getFQVariableName(node), getSimpleType(sb.toString())); + varToTypeMapping.put(Helper.getFQVariableName(node), getSimpleType(namesString)); break; } @@ -275,7 +271,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { public Object visit(final ASTProperty node, Object data) { ASTField field = node.getFirstChildOfType(ASTField.class); if (field != null) { - String fieldType = field.getNode().getFieldInfo().getType().getApexName(); + String fieldType = field.getType(); addVariableToMapping(Helper.getFQVariableName(field), fieldType); } @@ -284,7 +280,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } private void collectCRUDMethodLevelChecks(final ASTMethodCallExpression node) { - final String method = node.getNode().getMethodName(); + final String method = node.getMethodName(); final ASTReferenceExpression ref = node.getFirstChildOfType(ASTReferenceExpression.class); if (ref == null) { return; @@ -339,10 +335,9 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return false; } - private boolean isWithSecurityEnforced(final AbstractApexNode node) { + private boolean isWithSecurityEnforced(final ApexNode node) { if (node instanceof ASTSoqlExpression) { - boolean temp = WITH_SECURITY_ENFORCED.matcher(((ASTSoqlExpression) node).getQuery()).matches(); - return temp;//WITH_SECURITY_ENFORCED.matcher(((ASTSoqlExpression) node).getQuery()).matches(); + return WITH_SECURITY_ENFORCED.matcher(((ASTSoqlExpression) node).getQuery()).matches(); } return false; } @@ -369,7 +364,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } } - private void checkForCRUD(final AbstractApexNode node, final Object data, final String crudMethod) { + private void checkForCRUD(final ApexNode node, final Object data, final String crudMethod) { final Set prevCalls = getPreviousMethodCalls(node); for (ASTMethodCallExpression prevCall : prevCalls) { collectCRUDMethodLevelChecks(prevCall); @@ -401,7 +396,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } } - private Set getPreviousMethodCalls(final AbstractApexNode self) { + private Set getPreviousMethodCalls(final ApexNode self) { final Set innerMethodCalls = new HashSet<>(); final ASTMethod outerMethod = self.getFirstParentOfType(ASTMethod.class); if (outerMethod != null) { @@ -420,7 +415,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return innerMethodCalls; } - private void recursivelyEvaluateCRUDMethodCalls(final AbstractApexNode self, + private void recursivelyEvaluateCRUDMethodCalls(final ApexNode self, final Set innerMethodCalls, final ASTBlockStatement blockStatement) { if (blockStatement != null) { int numberOfStatements = blockStatement.getNumChildren(); @@ -434,7 +429,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } } - AbstractApexNode match = n.getFirstDescendantOfType(self.getClass()); + ApexNode match = n.getFirstDescendantOfType(self.getClass()); if (Objects.equal(match, self)) { break; } @@ -447,7 +442,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } } - private void mapCallToMethodDecl(final AbstractApexNode self, + private void mapCallToMethodDecl(final ApexNode self, final Set innerMethodCalls, final List nodes) { for (ASTMethodCallExpression node : nodes) { if (Objects.equal(node, self)) { @@ -513,7 +508,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } - private void validateCRUDCheckPresent(final AbstractApexNode node, final Object data, final String crudMethod, + private void validateCRUDCheckPresent(final ApexNode node, final Object data, final String crudMethod, final String typeCheck) { boolean missingKey = !typeToDMLOperationMapping.containsKey(typeCheck); boolean isImproperDMLCheck = !isProperESAPICheckForDML(typeCheck, crudMethod);