Merge pull request #4226 from aaronhurst-google:feature/replace-jorje-for-visualforce

[visualforce] Replace uses of Jorje types in pmd-visualforce #4226
This commit is contained in:
Andreas Dangel
2022-11-25 15:12:22 +01:00
6 changed files with 115 additions and 69 deletions

View File

@ -71,29 +71,32 @@ PMD 7 will remove support for `--files` in favor of these new flags.
#### Deprecated API
The following APIs have been marked as deprecated for removal in PMD 7:
* The following core APIs have been marked as deprecated for removal in PMD 7:
- {% jdoc core::PMD %} and {% jdoc core::PMD.StatusCode %} - PMD 7 will ship with a revamped CLI split from pmd-core. To programatically launch analysis you can use {% jdoc core::PmdAnalysis %}.
- {% jdoc !!core::PMDConfiguration#getAllInputPaths() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getInputPathList() %}
- {% jdoc !!core::PMDConfiguration#setInputPaths(List) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputPathList(List) %}
- {% jdoc !!core::PMDConfiguration#addInputPath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#addInputPath(Path) %}
- {% jdoc !!core::PMDConfiguration#getInputFilePath() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getInputFile() %}
- {% jdoc !!core::PMDConfiguration#getIgnoreFilePath() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getIgnoreFile() %}
- {% jdoc !!core::PMDConfiguration#setInputFilePath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputFilePath(Path) %}
- {% jdoc !!core::PMDConfiguration#setIgnoreFilePath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setIgnoreFilePath(Path) %}
- {% jdoc !!core::PMDConfiguration#getInputUri() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getUri() %}
- {% jdoc !!core::PMDConfiguration#setInputUri(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputUri(URI) %}
- {% jdoc !!core::PMDConfiguration#getReportFile() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getReportFilePath() %}
- {% jdoc !!core::PMDConfiguration#setReportFile(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setReportFile(Path) %}
- {% jdoc !!core::PMDConfiguration#isStressTest() %} and {% jdoc !!core::PMDConfiguration#setStressTest(boolean) %} - Will be removed with no replacement.
- {% jdoc !!core::PMDConfiguration#isBenchmark() %} and {% jdoc !!core::PMDConfiguration#setBenchmark(boolean) %} - Will be removed with no replacement, the CLI will still support it.
- {% jdoc core::cpd.CPD %} and {% jdoc core::cpd.CPD.StatusCode %} - PMD 7 will ship with a revamped CLI split from pmd-core. An alterative to programatically launch CPD analysis will be added in due time.
- {% jdoc core::PMD %} and {% jdoc core::PMD.StatusCode %} - PMD 7 will ship with a revamped CLI split from pmd-core. To programatically launch analysis you can use {% jdoc core::PmdAnalysis %}.
- {% jdoc !!core::PMDConfiguration#getAllInputPaths() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getInputPathList() %}
- {% jdoc !!core::PMDConfiguration#setInputPaths(List) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputPathList(List) %}
- {% jdoc !!core::PMDConfiguration#addInputPath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#addInputPath(Path) %}
- {% jdoc !!core::PMDConfiguration#getInputFilePath() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getInputFile() %}
- {% jdoc !!core::PMDConfiguration#getIgnoreFilePath() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getIgnoreFile() %}
- {% jdoc !!core::PMDConfiguration#setInputFilePath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputFilePath(Path) %}
- {% jdoc !!core::PMDConfiguration#setIgnoreFilePath(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setIgnoreFilePath(Path) %}
- {% jdoc !!core::PMDConfiguration#getInputUri() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getUri() %}
- {% jdoc !!core::PMDConfiguration#setInputUri(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setInputUri(URI) %}
- {% jdoc !!core::PMDConfiguration#getReportFile() %} - It is now superceded by {% jdoc !!core::PMDConfiguration#getReportFilePath() %}
- {% jdoc !!core::PMDConfiguration#setReportFile(String) %} - It is now superceded by {% jdoc !!core::PMDConfiguration#setReportFile(Path) %}
- {% jdoc !!core::PMDConfiguration#isStressTest() %} and {% jdoc !!core::PMDConfiguration#setStressTest(boolean) %} - Will be removed with no replacement.
- {% jdoc !!core::PMDConfiguration#isBenchmark() %} and {% jdoc !!core::PMDConfiguration#setBenchmark(boolean) %} - Will be removed with no replacement, the CLI will still support it.
- {% jdoc core::cpd.CPD %} and {% jdoc core::cpd.CPD.StatusCode %} - PMD 7 will ship with a revamped CLI split from pmd-core. An alterative to programatically launch CPD analysis will be added in due time.
* In order to reduce the dependency on Apex Jorje classes, the method {% jdoc !!visualforce::lang.vf.DataType#fromBasicType(apex.jorje.semantic.symbol.type.BasicType) %}
has been deprecated. The equivalent method {% jdoc visualforce::lang.vf.DataType#fromTypeName(java.lang.String) %} should be used instead.
### External Contributions
* [#4184](https://github.com/pmd/pmd/pull/4184): \[java]\[doc] TestClassWithoutTestCases - fix small typo in description - [Valery Yatsynovich](https://github.com/valfirst) (@valfirst)
* [#4198](https://github.com/pmd/pmd/pull/4198): \[doc] Add supported CPD languages - [Jeroen van Wilgenburg](https://github.com/jvwilge) (@jvwilge)
* [#4202](https://github.com/pmd/pmd/pull/4202): \[java] Fix #4200 and #4201: ClassWithOnlyPrivateConstructorsShouldBeFinal, CommentDefaultAccessModifier: Exclude lombok @<!-- -->Value annotation - [Lynn](https://github.com/LynnBroe) (@LynnBroe)
* [#4205](https://github.com/pmd/pmd/pull/4205): \[doc] Clarify Scala support (no built-in rules) - [Eldrick Wega](https://github.com/Eldrick19) (@Eldrick19)
* [#4226](https://github.com/pmd/pmd/pull/4226): \[visualforce] Replace uses of Jorje types in pmd-visualforce - [Aaron Hurst](https://github.com/aaronhurst-google) (@aaronhurst-google)
{% endtocmaker %}

View File

@ -23,8 +23,6 @@ import net.sourceforge.pmd.lang.apex.ApexLanguageModule;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.ast.Node;
import apex.jorje.semantic.symbol.type.BasicType;
/**
* Responsible for storing a mapping of Apex Class properties that can be referenced from Visualforce to the type of the
* property.
@ -51,8 +49,8 @@ class ApexClassPropertyTypes extends SalesforceFieldTypes {
Node node = parser.parse(apexFilePath.toString(), reader);
ApexClassPropertyTypesVisitor visitor = new ApexClassPropertyTypesVisitor();
visitor.visit((ApexNode<?>) node, null);
for (Pair<String, BasicType> variable : visitor.getVariables()) {
putDataType(variable.getKey(), DataType.fromBasicType(variable.getValue()));
for (Pair<String, String> variable : visitor.getVariables()) {
putDataType(variable.getKey(), DataType.fromTypeName(variable.getValue()));
}
} catch (IOException e) {
throw new ContextedRuntimeException(e)

View File

@ -12,14 +12,11 @@ import org.apache.commons.lang3.tuple.Pair;
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
import net.sourceforge.pmd.lang.apex.ast.ASTModifierNode;
import net.sourceforge.pmd.lang.apex.ast.ASTProperty;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexParserVisitorAdapter;
import apex.jorje.semantic.symbol.member.method.Generated;
import apex.jorje.semantic.symbol.member.method.MethodInfo;
import apex.jorje.semantic.symbol.type.BasicType;
/**
* Visits an Apex class to determine a mapping of referenceable expressions to expression type.
*/
@ -37,15 +34,15 @@ final class ApexClassPropertyTypesVisitor extends ApexParserVisitorAdapter {
private static final String RETURN_TYPE_VOID = "void";
/**
* Pairs of (variableName, BasicType)
* Pairs of (variableName, typeName)
*/
private final List<Pair<String, BasicType>> variables;
private final List<Pair<String, String>> variables;
ApexClassPropertyTypesVisitor() {
this.variables = new ArrayList<>();
}
public List<Pair<String, BasicType>> getVariables() {
public List<Pair<String, String>> getVariables() {
return this.variables;
}
@ -55,11 +52,10 @@ final class ApexClassPropertyTypesVisitor extends ApexParserVisitorAdapter {
*/
@Override
public Object visit(ASTMethod node, Object data) {
MethodInfo mi = node.getNode().getMethodInfo();
if (mi.getParameterTypes().isEmpty()
if (node.getArity() == 0
&& isVisibleToVisualForce(node)
&& !RETURN_TYPE_VOID.equalsIgnoreCase(mi.getReturnType().getApexName())
&& (mi.getGenerated().equals(Generated.USER) || mi.isPropertyAccessor())) {
&& !RETURN_TYPE_VOID.equalsIgnoreCase(node.getReturnType())
&& (node.hasRealLoc() || node.getFirstParentOfType(ASTProperty.class) != null)) {
StringBuilder sb = new StringBuilder();
List<ASTUserClass> parents = node.getParentsOfType(ASTUserClass.class);
Collections.reverse(parents);
@ -74,7 +70,7 @@ final class ApexClassPropertyTypesVisitor extends ApexParserVisitorAdapter {
}
sb.append(name);
variables.add(Pair.of(sb.toString(), mi.getReturnType().getBasicType()));
variables.add(Pair.of(sb.toString(), node.getReturnType()));
}
return super.visit((ApexNode<?>) node, data);
}

View File

@ -22,10 +22,10 @@ import apex.jorje.semantic.symbol.type.BasicType;
*/
public enum DataType {
AutoNumber(false),
Checkbox(false, BasicType.BOOLEAN),
Currency(false, BasicType.CURRENCY),
Date(false, BasicType.DATE),
DateTime(false, BasicType.DATE_TIME),
Checkbox(false, "Boolean"),
Currency(false, "Currency"),
Date(false, "Date"),
DateTime(false, "Datetime"),
Email(false),
EncryptedText(true),
ExternalLookup(true),
@ -35,19 +35,19 @@ public enum DataType {
IndirectLookup(false),
Location(false),
LongTextArea(true),
Lookup(false, BasicType.ID),
Lookup(false, "ID"),
MasterDetail(false),
MetadataRelationship(false),
MultiselectPicklist(true),
Note(true),
Number(false, BasicType.DECIMAL, BasicType.DOUBLE, BasicType.INTEGER, BasicType.LONG),
Number(false, "Decimal", "Double", "Integer", "Long"),
Percent(false),
Phone(false),
Picklist(true),
Summary(false),
Text(true, BasicType.STRING),
Text(true, "String"),
TextArea(true),
Time(false, BasicType.TIME),
Time(false, "Time"),
Url(false),
/**
* Indicates that Metatada was found, but it's type was not mappable. This could because it is a type which isn't
@ -64,26 +64,27 @@ public enum DataType {
public final boolean requiresEscaping;
/**
* The set of {@link BasicType}s that map to this type. Multiple types can map to a single instance of this enum.
* The set of primitive type names that map to this type. Multiple types can map to a single instance of this enum.
* Note: these strings are not case-normalized.
*/
private final Set<BasicType> basicTypes;
private final Set<String> basicTypeNames;
/**
* A case insensitive map of the enum name to its instance. The case metadata is not guaranteed to have the correct
* A map of the lower-case-normalized enum name to its instance. The case metadata is not guaranteed to have the correct
* case.
*/
private static final Map<String, DataType> CASE_INSENSITIVE_MAP = new HashMap<>();
private static final Map<String, DataType> CASE_NORMALIZED_MAP = new HashMap<>();
/**
* Map of BasicType to DataType. Multiple BasicTypes may map to one DataType.
* A map of the lower-case-normalized primitive type names to DataType. Multiple types may map to one DataType.
*/
private static final Map<BasicType, DataType> BASIC_TYPE_MAP = new HashMap<>();
private static final Map<String, DataType> BASIC_TYPE_MAP = new HashMap<>();
static {
for (DataType dataType : DataType.values()) {
CASE_INSENSITIVE_MAP.put(dataType.name().toLowerCase(Locale.ROOT), dataType);
for (BasicType basicType : dataType.basicTypes) {
BASIC_TYPE_MAP.put(basicType, dataType);
CASE_NORMALIZED_MAP.put(dataType.name().toLowerCase(Locale.ROOT), dataType);
for (String typeName : dataType.basicTypeNames) {
BASIC_TYPE_MAP.put(typeName.toLowerCase(Locale.ROOT), dataType);
}
}
}
@ -93,7 +94,7 @@ public enum DataType {
*/
public static DataType fromString(String value) {
value = value != null ? value : "";
DataType dataType = CASE_INSENSITIVE_MAP.get(value.toLowerCase(Locale.ROOT));
DataType dataType = CASE_NORMALIZED_MAP.get(value.toLowerCase(Locale.ROOT));
if (dataType == null) {
dataType = DataType.Unknown;
@ -105,9 +106,46 @@ public enum DataType {
/**
* Map to correct instance, returns {@code Unknown} if the value can't be mapped.
*
* @deprecated Use {@link #fromTypeName(String)} instead.
*/
@Deprecated
public static DataType fromBasicType(BasicType value) {
DataType dataType = value != null ? BASIC_TYPE_MAP.get(value) : null;
if (value != null) {
switch (value) {
case BOOLEAN:
return Checkbox;
case CURRENCY:
return Currency;
case DATE:
return Date;
case DATE_TIME:
return DateTime;
case ID:
return Lookup;
case DECIMAL:
case DOUBLE:
case INTEGER:
case LONG:
return Number;
case STRING:
return Text;
case TIME:
return Time;
default:
break;
}
}
LOGGER.fine("Unable to determine DataType of " + value);
return Unknown;
}
/**
* Map to correct instance, returns {@code Unknown} if the value can't be mapped.
*/
public static DataType fromTypeName(String value) {
value = value != null ? value : "";
DataType dataType = BASIC_TYPE_MAP.get(value.toLowerCase(Locale.ROOT));
if (dataType == null) {
dataType = DataType.Unknown;
@ -121,11 +159,11 @@ public enum DataType {
this(requiresEscaping, null);
}
DataType(boolean requiresEscaping, BasicType... basicTypes) {
DataType(boolean requiresEscaping, String... basicTypeNames) {
this.requiresEscaping = requiresEscaping;
this.basicTypes = new HashSet<>();
if (basicTypes != null) {
this.basicTypes.addAll(Arrays.asList(basicTypes));
this.basicTypeNames = new HashSet<>();
if (basicTypeNames != null) {
this.basicTypeNames.addAll(Arrays.asList(basicTypeNames));
}
}
}

View File

@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.vf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.io.BufferedReader;
import java.io.IOException;
@ -28,8 +29,6 @@ import net.sourceforge.pmd.lang.apex.ApexLanguageModule;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.ast.Node;
import apex.jorje.semantic.symbol.type.BasicType;
public class ApexClassPropertyTypesVisitorTest {
@Test
public void testApexClassIsProperlyParsed() throws IOException {
@ -46,21 +45,21 @@ public class ApexClassPropertyTypesVisitorTest {
visitor.visit((ApexNode<?>) node, null);
}
List<Pair<String, BasicType>> variables = visitor.getVariables();
List<Pair<String, String>> variables = visitor.getVariables();
assertEquals(7, variables.size());
Map<String, BasicType> variableNameToVariableType = new Hashtable<>();
for (Pair<String, BasicType> variable : variables) {
Map<String, String> variableNameToVariableType = new Hashtable<>();
for (Pair<String, String> variable : variables) {
// Map the values and ensure there were no duplicates
BasicType previous = variableNameToVariableType.put(variable.getKey(), variable.getValue());
String previous = variableNameToVariableType.put(variable.getKey(), variable.getValue());
assertNull(variable.getKey(), previous);
}
assertEquals(BasicType.ID, variableNameToVariableType.get("ApexController.AccountIdProp"));
assertEquals(BasicType.ID, variableNameToVariableType.get("ApexController.AccountId"));
assertEquals(BasicType.STRING, variableNameToVariableType.get("ApexController.AccountName"));
assertEquals(BasicType.APEX_OBJECT, variableNameToVariableType.get("ApexController.InnerController"));
assertEquals(BasicType.ID, variableNameToVariableType.get("ApexController.InnerController.InnerAccountIdProp"));
assertEquals(BasicType.ID, variableNameToVariableType.get("ApexController.InnerController.InnerAccountId"));
assertEquals(BasicType.STRING, variableNameToVariableType.get("ApexController.InnerController.InnerAccountName"));
assertTrue("ID".equalsIgnoreCase(variableNameToVariableType.get("ApexController.AccountIdProp")));
assertTrue("ID".equalsIgnoreCase(variableNameToVariableType.get("ApexController.AccountId")));
assertTrue("String".equalsIgnoreCase(variableNameToVariableType.get("ApexController.AccountName")));
assertTrue("ApexController.InnerController".equalsIgnoreCase(variableNameToVariableType.get("ApexController.InnerController")));
assertTrue("ID".equalsIgnoreCase(variableNameToVariableType.get("ApexController.InnerController.InnerAccountIdProp")));
assertTrue("ID".equalsIgnoreCase(variableNameToVariableType.get("ApexController.InnerController.InnerAccountId")));
assertTrue("String".equalsIgnoreCase(variableNameToVariableType.get("ApexController.InnerController.InnerAccountName")));
}
}

View File

@ -22,7 +22,19 @@ public class DataTypeTest {
}
@Test
public void testFromBasicType() {
public void testFromTypeName() {
assertEquals(DataType.Checkbox, DataType.fromTypeName("Boolean"));
assertEquals(DataType.Currency, DataType.fromTypeName("Currency"));
assertEquals(DataType.DateTime, DataType.fromTypeName("Datetime"));
assertEquals(DataType.Number, DataType.fromTypeName("DECIMAL"));
assertEquals(DataType.Number, DataType.fromTypeName("double"));
assertEquals(DataType.Text, DataType.fromTypeName("string"));
assertEquals(DataType.Unknown, DataType.fromTypeName("Object"));
assertEquals(DataType.Unknown, DataType.fromTypeName(null));
}
@Test
public void testDeprecatedFromBasicType() {
assertEquals(DataType.Checkbox, DataType.fromBasicType(BasicType.BOOLEAN));
assertEquals(DataType.Number, DataType.fromBasicType(BasicType.DECIMAL));
assertEquals(DataType.Number, DataType.fromBasicType(BasicType.DOUBLE));