[apex] Small refactorings in ApexCRUDViolationRule

To avoid some deprecations
This commit is contained in:
Andreas Dangel
2020-03-14 18:16:52 +01:00
parent be72ec153d
commit 383713fdbd

View File

@ -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<String, String> varToTypeMapping = new HashMap<>();
private final ListMultimap<String, String> typeToDMLOperationMapping = ArrayListMultimap.create();
private final Map<String, String> checkedTypeToDMLOperationViaESAPI = new HashMap<>();
private final Map<String, ASTMethod> 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<String, String> varToTypeMapping = new HashMap<>();
private final ListMultimap<String, String> typeToDMLOperationMapping = ArrayListMultimap.create();
private final Map<String, String> checkedTypeToDMLOperationViaESAPI = new HashMap<>();
private final Map<String, ASTMethod> 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<TypeRef> 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<ASTMethodCallExpression> prevCalls = getPreviousMethodCalls(node);
for (ASTMethodCallExpression prevCall : prevCalls) {
collectCRUDMethodLevelChecks(prevCall);
@ -401,7 +396,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
}
}
private Set<ASTMethodCallExpression> getPreviousMethodCalls(final AbstractApexNode<?> self) {
private Set<ASTMethodCallExpression> getPreviousMethodCalls(final ApexNode<?> self) {
final Set<ASTMethodCallExpression> 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<ASTMethodCallExpression> 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<ASTMethodCallExpression> innerMethodCalls, final List<ASTMethodCallExpression> 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);