Fixed Constants should be on the right side of comparison #1237
Conversation
|
@yegor256 , Could you please review this and tell me if this is what meant in task.Thanks |
|
@pnatashap can you please check this one? |
|
@LaithAlebrahim you have reverted comparisons with null, while the task is about to create a new rule (the most closest from my point of view is https://docs.pmd-code.org/latest/pmd_rules_java_errorprone.html#compareobjectswithequals) to raise a validation error if you have such code. |
|
Could you please review it and check if everything is fine .And let me know if you have any concerns. |
There was a problem hiding this comment.
And also test is needed to check the rule, like here https://github.com/yegor256/qulice/blob/master/qulice-pmd/src/test/java/com/qulice/pmd/UseStringIsEmptyRuleTest.java
| @@ -256,4 +256,26 @@ OF THE POSSIBILITY OF SUCH DAMAGE. | |||
| </property> | |||
| </properties> | |||
| </rule> | |||
| <rule name="ConstantsOnRightSideOfComparison" language="java" class="net.sourceforge.pmd.lang.rule.XPathRule" message="Constants should be on the right side of comparisons."> | |||
| <description> | |||
| Enforces the code style guideline that constants (null, string literals, or numbers) should appear on the right side of comparison operators to reduce the risk of accidental assignment and improve readability. | |||
There was a problem hiding this comment.
- It is better to have this rule only about numbers and null values (because for string values there is another rule, that will write that you need to use equals method instead of ==) (So if you have code
"some" == some, you will get the first warning to revert it, and the second - use equals and it can be confusing) - The same rule should be about != operation, as they are similar, so
null != someshould produce the same error
|
@yegor256 @pnatashap I modified the rule to handle the specific requirments just (numeral and null with '!=' and '==' equality) |
|
@LaithAlebrahim would be great to add a test for this new feature, otherwise how do we know that it works? |
|
@LaithAlebrahim I have checked this rule and do not see any violations, so looks like something does not work |
@yegor256 , Please can you review this issue#716 and let me know if this what is required from the task.
I searched for all null comparison in the code and this what I found where null should be on right side of the comparsion.
Please let me know if you have any concerns .Tx