[apex] Add a rule to prevent use of non-existent annotations#836
[apex] Add a rule to prevent use of non-existent annotations#836jsotuyod merged 13 commits intopmd:masterfrom anand13s:master
Conversation
# Conflicts: # pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
jsotuyod
left a comment
There was a problem hiding this comment.
@anand13s thanks for the PR! The rule looks like a nice addition, and the way to get valid annotations by reflection from Jorje is neat (are we certain it includes all accepted annotations?).
Please, rebase against master and check the issues I here raise. Let me know if you need any assistance.
| public abstract class AbstractApexRule extends AbstractRule | ||
| implements ApexParserVisitor, ImmutableLanguage, CodeClimateRule { | ||
|
|
||
| public static final IntegerProperty CODACY_MINUTES_TO_FIX = new IntegerProperty("codacy_minutes_to_fix", |
There was a problem hiding this comment.
the codacy properties are unrelated to the object of this PR, please revert this changeset
| @@ -0,0 +1,88 @@ | |||
| package net.sourceforge.pmd.lang.apex.rule.style; | |||
There was a problem hiding this comment.
the rule seems like a better fit for the errorprone category (code that is likely to break) rather than a code style preference. Consider moving it there.
| @@ -1,128 +1,180 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <ruleset xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="Default ruleset used by the CodeClimate Engine for Salesforce.com Apex" xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd"> | |||
|
|
|||
There was a problem hiding this comment.
changes to this file are unrelated to the object of this PR, please revert
| </example> | ||
| </rule> | ||
|
|
||
| <rule name="AvoidNonExistentAnnotations" |
There was a problem hiding this comment.
rulesets are deprecated. New rules should be added to categories. In particular, this looks like a fit for the errorprone category
| </rule> | ||
|
|
||
| <rule name="AvoidNonExistentAnnotations" | ||
| since="5.5.5" |
There was a problem hiding this comment.
the next release to include new rules will be 6.1.0
| @@ -0,0 +1,55 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
consider adding test scenarios where valid annotations are not flagged
| end inline_praqma_error; | ||
| / | ||
| create or replace package inline_praqma_error is |
There was a problem hiding this comment.
this changeset is unrelated to the object of the PR, please revert
| loFPOGE_OBJ:=SELECT A FROM persons p WHERE IS OF TYPE (employee_t); | ||
| loFPOGE_OBJ:=SELECT A FROM persons p WHERE IS NOT OF TYPE (ONLY employee_t, other_t); | ||
| PROCEDURE IsOfType ( |
There was a problem hiding this comment.
this changeset is unrelated to the object of the PR, please revert
|
@jsotuyod I'm the product manager for Apex at Salesforce (Anand is on the dev team), and wanted to answer
This will be the case for the foreseeable future. I can't make an official guarantee as this isn't a formally supported API, but we have no current plans to make changes that would stop this from being true. |
| <description> | ||
| Apex supported non existent annotations for legacy reasons. | ||
| In the future, use of such non-existent annotations could result in broken apex code that will not copile. | ||
| This will prevent users of garbage annotations from being able to use legitimate annotations added to apex in the future. |
There was a problem hiding this comment.
consider adding the link to the list of valid annotations here for user reference
|
@capeterson thanks for the input! I'm really glad to know that. I'm looking forward to strengthen the collaboration between PMD and Salesforce. Feel free to contact me to better align objectives / priorities and improve synergies. |
|
@jsotuyod and @capeterson and please always feel free to pull me in as a middle man in case of timing issues. I still feel very responsible ;-) |
|
@jsotuyod can I help to solve this conflict? Is there even still a conflict? |
|
@rsoesemann there are 2 merge-conflicts and still all my unresolved comments. I don't think you can solve this unless you take the branch into your PMD fork, and submit a new PR yourself. PMD maintainers can do something similar on local copies before merging. |
|
@anand13s any reason for not responding to the comments and merge conflicts? |
|
@rsoesemann he's been busy with his day job and all the work I heaped on him for our Summer release. We know we need to fix these before this will be accepted. |
|
I personally fixed the remaining issues and merged it. This will be part of PMD 6.5.0. |
|
Thanks everybody involved especially @jsotuyod for getting this into production. |
This rule seeks to warn against using non-existent annotations in apex source code. A 'non existent' annotation is one not defined in the apex language documentation annotations section.
This is done to prevent apex source code from being broken by newer Apex annotations in the Salesforce language that possess the same signature as a non existent one.