[core] Deprecate IntegerProperty#1485
Conversation
Generated by 🚫 Danger |
| Since arbitrary types may be represented, `type` will become obsolete as it can't represent generic types, | ||
| which will nevertheless be representable with the XML syntax. It was only used for documentation, but a | ||
| new way to document these properties exhaustively will be added with 7.0.0. | ||
| * {% jdoc :PDr#errorFor(java.lang.Object) %} is deprecated as its return type will be changed to `Optional<String>` with the shift to Java 8. |
There was a problem hiding this comment.
This might be different - if we collect errors and return all violated constraints on the property, we might just add a List<String> errorsFor(T value) or something even more sophisticated. We could have a look e.g. at the bean validation api - there a Set<ConstraintValidation> is returned. So we could return something more structured than just a string.
| = PropertyFactory.intProperty("problemDepth") | ||
| .desc("The if statement depth reporting threshold") | ||
| .range(1, 25).defaultValue(3).uiOrder(1.0f).build(); | ||
| .require(positive()).defaultValue(3).build(); |
There was a problem hiding this comment.
Just wondered, whether we would have documented the minimum/maximum already (which we now change....) - but we don't: https://pmd.github.io/pmd-6.9.0/pmd_rules_apex_design.html#avoiddeeplynestedifstmts
So. no problem here. Limiting the the problemDepth to 25 was arbitrary anyways.
| * <ul> | ||
| * <li>A name: filled-in when obtaining the builder | ||
| * <li>A description: see {@link #desc(String)} | ||
| * <li>A default value: see {@link #defaultValue(Object)} |
There was a problem hiding this comment.
Is this a new requirement, that default values must be specified? This definitely makes sense!
I noticed, that the StatisticalRule defines properties with "null" as the default value... Which means, the property has no value, see https://pmd.github.io/pmd-6.9.0/pmd_rules_java_design.html#excessivemethodlength
There was a problem hiding this comment.
In fact that has been documented for a while, but not enforced properly ^^ Disallowing null values now allows to check easily whether the value was set or not.
StatisticalRule's properties exploit some of the cracks in the current framework. Reworking them is impossible, because the same descriptor is used for many rules, which don't share a sensible default value... so better scrap them directly when the time comes
| * | ||
| * @author Brian Remedios | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
We should make sure that we cover the testcases in a new unit test class.
There was a problem hiding this comment.
I think, this is probably covered by NumericConstraintsTest and PropertyDescriptorTest.
Refs #1432