From 5964add71fa9c5d57feb45322a24800f5eb9dad8 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 9 Feb 2024 10:49:00 +0100 Subject: [PATCH] [apex] ASTFieldDeclarationStatements - clarify #getTypeArguments Also simplify ApexCRUDViolationRule Fixes TODO(b/239648780) --- .../pmd/lang/apex/ast/ASTField.java | 2 +- .../ast/ASTFieldDeclarationStatements.java | 38 +++++-------- .../pmd/lang/apex/ast/ASTParameter.java | 2 +- .../lang/apex/ast/ASTVariableDeclaration.java | 2 +- .../rule/security/ApexCRUDViolationRule.java | 33 ++++-------- .../ASTFieldDeclarationStatementsTest.java | 53 +++++++++++++++++++ 6 files changed, 80 insertions(+), 50 deletions(-) create mode 100644 pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatementsTest.java diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTField.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTField.java index 8d670f2675..0abfdfcbf9 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTField.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTField.java @@ -51,7 +51,7 @@ public final class ASTField extends AbstractApexNode.Many { /** * Returns the type name. * - * This includes any type arguments. (This is tested.) + *

This includes any type arguments. (This is tested.) * If the type is a primitive, its case will be normalized. */ public String getType() { diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatements.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatements.java index 6f9f7f3622..89862a631d 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatements.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatements.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.apex.ast; import java.util.ArrayList; import java.util.List; +import com.google.summit.ast.TypeRef; import com.google.summit.ast.declaration.FieldDeclarationGroup; @@ -30,40 +31,29 @@ public final class ASTFieldDeclarationStatements extends AbstractApexNode.Single /** * Returns the type name. * - * This includes any type arguments. + *

This includes any type arguments. * If the type is a primitive, its case will be normalized. */ public String getTypeName() { return caseNormalizedTypeIfPrimitive(node.getType().asCodeString()); } - /* - private static String identifiersToString(List identifiers) { - return identifiers.stream().map(Identifier::getValue).collect(Collectors.joining(".")); - } + /** + * This returns the first level of the type arguments. If there are nested + * types (e.g. {@code List>}), then these returned types + * contain themselves type arguments. + * + *

Note: This method only exists for this AST type and in no other type, + * even though type arguments are possible e.g. for {@link ASTVariableDeclaration#getType()}. */ - // TODO(b/239648780) - public List getTypeArguments() { - /* List result = new ArrayList<>(); - - if (node.getTypeName() != null) { - List typeArgs = node.getTypeName().getTypeArguments(); - for (TypeRef arg : typeArgs) { - if (arg instanceof ClassTypeRef) { - result.add(identifiersToString(arg.getNames())); - } else if (arg instanceof ArrayTypeRef) { - ArrayTypeRef atr = (ArrayTypeRef) arg; - if (atr.getHeldType() instanceof ClassTypeRef) { - result.add(identifiersToString(atr.getHeldType().getNames())); - } - } + // note: for void types, there are no components anyway + for (TypeRef.Component component : node.getType().getComponents()) { + for (TypeRef typeRef : component.getArgs()) { + result.add(caseNormalizedTypeIfPrimitive(typeRef.asCodeString())); } } - */ - // TODO(b/239648780) - - return new ArrayList<>(); + return result; } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTParameter.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTParameter.java index d3c4e6254d..b5a24279a8 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTParameter.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTParameter.java @@ -30,7 +30,7 @@ public final class ASTParameter extends AbstractApexNode.SingleThis includes any type arguments. * If the type is a primitive, its case will be normalized. */ public String getType() { diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTVariableDeclaration.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTVariableDeclaration.java index dc9b5387f1..869eed545e 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTVariableDeclaration.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTVariableDeclaration.java @@ -27,7 +27,7 @@ public final class ASTVariableDeclaration extends AbstractApexNode.SingleThis includes any type arguments. * If the type is a primitive, its case will be normalized. */ public String getType() { 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 604eaab826..576b4d4436 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 @@ -70,6 +70,8 @@ import com.google.common.collect.HashMultimap; public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern SELECT_FROM_PATTERN = Pattern.compile("[\\S|\\s]+?FROM[\\s]+?(\\w+)", Pattern.CASE_INSENSITIVE); + private static final Pattern EXTRACT_SIMPLE_TYPE_PATTERN = Pattern.compile("^(?:list<)?list<(\\S+?)>>?$", + Pattern.CASE_INSENSITIVE); private static final String IS_CREATEABLE = "isCreateable"; private static final String IS_DELETABLE = "isDeletable"; @@ -359,18 +361,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { ASTFieldDeclarationStatements field = node.ancestors(ASTFieldDeclarationStatements.class).first(); if (field != null) { String namesString = field.getTypeName(); - - switch (namesString.toLowerCase(Locale.ROOT)) { - case "list": - case "map": - for (String typeArg : field.getTypeArguments()) { - varToTypeMapping.put(Helper.getFQVariableName(node), typeArg); - } - break; - default: - varToTypeMapping.put(Helper.getFQVariableName(node), getSimpleType(namesString)); - break; - } + addVariableToMapping(Helper.getFQVariableName(node), namesString); } final ASTSoqlExpression soql = node.firstChild(ASTSoqlExpression.class); if (soql != null) { @@ -378,7 +369,6 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } return data; - } @Override @@ -402,21 +392,18 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } private void addVariableToMapping(final String variableName, final String type) { - switch (type.toLowerCase(Locale.ROOT)) { - case "list": - case "map": - break; - default: - varToTypeMapping.put(variableName, getSimpleType(type)); - break; - } + varToTypeMapping.put(variableName, getSimpleType(type)); } + /** + * Extracts the type arguments from a type, e.g. given {@code List} it will return {@code String}. + * + *

Note: Maps are not supported + */ private String getSimpleType(final String type) { String typeToUse = type; - Pattern pattern = Pattern.compile("^[list<]?list<(\\S+?)>[>]?$", Pattern.CASE_INSENSITIVE); - Matcher matcher = pattern.matcher(typeToUse); + Matcher matcher = EXTRACT_SIMPLE_TYPE_PATTERN.matcher(typeToUse); if (matcher.find()) { typeToUse = matcher.group(1); diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatementsTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatementsTest.java new file mode 100644 index 0000000000..c83f1511aa --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ASTFieldDeclarationStatementsTest.java @@ -0,0 +1,53 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.ast; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ASTFieldDeclarationStatementsTest extends ApexParserTestBase { + + @Test + void getSimpleTypeName() { + ASTFieldDeclarationStatements fields = parse("class Foo { String field; }").descendants(ASTFieldDeclarationStatements.class).first(); + assertEquals("String", fields.getTypeName()); + assertTrue(fields.getTypeArguments().isEmpty()); + } + + @Test + void getListTypeName() { + ASTFieldDeclarationStatements fields = parse("class Foo { List field; }").descendants(ASTFieldDeclarationStatements.class).first(); + assertEquals("List", fields.getTypeName()); + assertEquals(1, fields.getTypeArguments().size()); + assertEquals("String", fields.getTypeArguments().get(0)); + } + + @Test + void getListTypeNameComponents() { + ASTFieldDeclarationStatements fields = parse("class Foo { my.List field; }").descendants(ASTFieldDeclarationStatements.class).first(); + assertEquals("my.List", fields.getTypeName()); + assertEquals(1, fields.getTypeArguments().size()); + assertEquals("my.String", fields.getTypeArguments().get(0)); + } + + @Test + void getNestedListTypeName() { + ASTFieldDeclarationStatements fields = parse("class Foo { List> field; }").descendants(ASTFieldDeclarationStatements.class).first(); + assertEquals("List>", fields.getTypeName()); + assertEquals(1, fields.getTypeArguments().size()); + assertEquals("List", fields.getTypeArguments().get(0)); + } + + @Test + void getMapTypeName() { + ASTFieldDeclarationStatements fields = parse("class Foo { Map field; }").descendants(ASTFieldDeclarationStatements.class).first(); + assertEquals("Map", fields.getTypeName()); + assertEquals(2, fields.getTypeArguments().size()); + assertEquals("String", fields.getTypeArguments().get(0)); + assertEquals("Integer", fields.getTypeArguments().get(1)); + } +}