adding the ensure compiler plugin #657

Merged
justliliandev merged 7 commits from feature/ensureplugin into dev/1.20.6 2024-06-10 15:17:20 +00:00
justliliandev commented 2024-05-11 11:51:26 +00:00 (Migrated from github.com)

Description

This PR adds the Ensure Compiler Plugin, it allows the usage of Ensure[XYZ] Annotations to add bytecode to check certain things. Currently added are null checks, range checks and Side Checks.

How to add checks

New checks can be added by creating a new Transformer, adding it to the Service, let it Target a new Ensure Annotation using the Target Annotation and then add the code checks in Transformer#transform.

TODO

  • Add more EnsureNotNull checks to places where we may obtain a null, but don't use it directly, causing delayed npe which are hard to track down
  • Test and potentially fix Ensure Annotations on constructors as the checks are inserted as the first statement which may not work when having to call a super constructor

Checklist

  • My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
  • I have made corresponding changes to the documentation.
  • My changes are ready for review from a contributor.
# Description This PR adds the Ensure Compiler Plugin, it allows the usage of Ensure[XYZ] Annotations to add bytecode to check certain things. Currently added are null checks, range checks and Side Checks. ## How to add checks New checks can be added by creating a new Transformer, adding it to the Service, let it Target a new Ensure Annotation using the Target Annotation and then add the code checks in Transformer#transform. # TODO - [ ] Add more EnsureNotNull checks to places where we may obtain a null, but don't use it directly, causing delayed npe which are hard to track down - [x] Test and potentially fix Ensure Annotations on constructors as the checks are inserted as the first statement which may not work when having to call a super constructor # Checklist - [x] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [x] I have made corresponding changes to the documentation. - [x] My changes are ready for review from a contributor.
dphaldes (Migrated from github.com) reviewed 2024-05-11 11:51:26 +00:00
github-actions[bot] (Migrated from github.com) reviewed 2024-05-11 12:10:18 +00:00
@ -0,0 +1,57 @@
package me.liliandev.ensure;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> reported by reviewdog 🐶
Missing package-info.java file.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Missing package-info.java file.
@ -0,0 +1,52 @@
package me.liliandev.ensure.ensures;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>'if' construct must use '{}'s.
@ -0,0 +1,46 @@
package me.liliandev.ensure.ensures;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> reported by reviewdog 🐶
Missing package-info.java file.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Missing package-info.java file.
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>'if' construct must use '{}'s.
@ -0,0 +1,54 @@
package me.liliandev.ensure.setup;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> reported by reviewdog 🐶
Name 'expectedPackages' must match pattern '^[A-Z][A-Z0-9](_[A-Z0-9]+)$'.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Name 'expectedPackages' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
@ -0,0 +1,9 @@
package me.liliandev.ensure.setup.jdkdummy;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> reported by reviewdog 🐶
Missing package-info.java file.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Missing package-info.java file.
@ -0,0 +1,12 @@
package me.liliandev.ensure.setup.jdkdummy;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> reported by reviewdog 🐶
Name 'staticObj' must match pattern '^[A-Z][A-Z0-9](_[A-Z0-9]+)$'.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Name 'staticObj' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
@ -0,0 +1,13 @@
package me.liliandev.ensure.transformerutils;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> reported by reviewdog 🐶
Missing package-info.java file.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Missing package-info.java file.
@ -0,0 +1,112 @@
package me.liliandev.ensure.transformerutils;
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> reported by reviewdog 🐶
Name 'transformers' must match pattern '^[A-Z][A-Z0-9](_[A-Z0-9]+)$'.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>Name 'transformers' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
github-actions[bot] (Migrated from github.com) commented 2024-05-11 12:10:18 +00:00

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.

🚫 **[checkstyle]** <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> <sub>reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:</sub><br>'if' construct must use '{}'s.
Rover656 (Migrated from github.com) requested changes 2024-05-11 19:36:23 +00:00
Rover656 (Migrated from github.com) left a comment

Looks good to me - just one EnsureSide that needs double checking! Thank you

Looks good to me - just one EnsureSide that needs double checking! Thank you
@ -106,7 +109,8 @@ public class ConduitBlockEntity extends EnderBlockEntity {
updateConnectionToData(conduitType);
Rover656 (Migrated from github.com) commented 2024-05-11 19:34:35 +00:00

Changed from Server to Client - is that right?

Changed from Server to Client - is that right?
justliliandev (Migrated from github.com) reviewed 2024-05-11 19:43:14 +00:00
@ -106,7 +109,8 @@ public class ConduitBlockEntity extends EnderBlockEntity {
updateConnectionToData(conduitType);
justliliandev (Migrated from github.com) commented 2024-05-11 19:43:14 +00:00

probably not, I'll double check it

probably not, I'll double check it
Rover656 commented 2024-05-18 10:37:11 +00:00 (Migrated from github.com)

Would you like me to re-review this, or were you hoping for review from another team member? Cheers!

Would you like me to re-review this, or were you hoping for review from another team member? Cheers!
justliliandev commented 2024-05-18 12:54:41 +00:00 (Migrated from github.com)

a review from another maintainer would be appreciated

a review from another maintainer would be appreciated
ferriarnus (Migrated from github.com) reviewed 2024-06-06 20:38:34 +00:00
ferriarnus (Migrated from github.com) left a comment

So this looks good to me, just some small style things and a question

So this looks good to me, just some small style things and a question
@ -91,7 +94,7 @@ public class ConduitBlockEntity extends EnderBlockEntity {
/**
ferriarnus (Migrated from github.com) commented 2024-06-06 20:26:42 +00:00

I think it adds an extra whitespace?

I think it adds an extra whitespace?
@ -0,0 +1,57 @@
package me.liliandev.ensure;
ferriarnus (Migrated from github.com) commented 2024-06-06 20:28:14 +00:00

this comment can be removed now?

this comment can be removed now?
@ -0,0 +14,4 @@
* min is inclusive
* max is exclusive
*/
public @interface EnsureInRange {
ferriarnus (Migrated from github.com) commented 2024-06-06 20:35:11 +00:00

This isn't currently used right?

This isn't currently used right?
@ -0,0 +12,4 @@
/**
* Adds a null check to non primitive parameters
*/
public @interface EnsureNotNull {
ferriarnus (Migrated from github.com) commented 2024-06-06 20:35:39 +00:00

This isn't currently used right?

This isn't currently used right?
justliliandev (Migrated from github.com) reviewed 2024-06-06 20:41:07 +00:00
@ -0,0 +14,4 @@
* min is inclusive
* max is exclusive
*/
public @interface EnsureInRange {
justliliandev (Migrated from github.com) commented 2024-06-06 20:41:07 +00:00

yes, I wanted to have it just in case. this can ensure nonnegativity with 0 and long max value and a spot in conduit code

yes, I wanted to have it just in case. this can ensure nonnegativity with 0 and long max value and a spot in conduit code
justliliandev (Migrated from github.com) reviewed 2024-06-06 20:41:48 +00:00
@ -0,0 +12,4 @@
/**
* Adds a null check to non primitive parameters
*/
public @interface EnsureNotNull {
justliliandev (Migrated from github.com) commented 2024-06-06 20:41:47 +00:00

yes, but I had a few ideas where it makes sense so I added it without checking all potential spots and using it atm

yes, but I had a few ideas where it makes sense so I added it without checking all potential spots and using it atm
ferriarnus (Migrated from github.com) reviewed 2024-06-06 20:44:03 +00:00
@ -0,0 +12,4 @@
/**
* Adds a null check to non primitive parameters
*/
public @interface EnsureNotNull {
ferriarnus (Migrated from github.com) commented 2024-06-06 20:44:03 +00:00

Yeah makes sense

Yeah makes sense
justliliandev (Migrated from github.com) reviewed 2024-06-06 20:45:11 +00:00
@ -0,0 +1,57 @@
package me.liliandev.ensure;
justliliandev (Migrated from github.com) commented 2024-06-06 20:45:10 +00:00

reversed was a newer feature and I wanted to make sure the first parameter check is added last because each check is added to the front so the first ensure on a parameter will be the first one to be checked, but it doesn't really matter

reversed was a newer feature and I wanted to make sure the first parameter check is added last because each check is added to the front so the first ensure on a parameter will be the first one to be checked, but it doesn't really matter
ferriarnus (Migrated from github.com) reviewed 2024-06-06 20:57:01 +00:00
@ -91,7 +94,7 @@ public class ConduitBlockEntity extends EnderBlockEntity {
/**
ferriarnus (Migrated from github.com) commented 2024-06-06 20:57:01 +00:00

I think it did it everywhere where the annotation is used? Does our style guide not catch that?

I think it did it everywhere where the annotation is used? Does our style guide not catch that?
justliliandev (Migrated from github.com) reviewed 2024-06-06 21:10:55 +00:00
@ -91,7 +94,7 @@ public class ConduitBlockEntity extends EnderBlockEntity {
/**
justliliandev (Migrated from github.com) commented 2024-06-06 21:10:55 +00:00

yeah, happened when I've pasted the new annotations in, I'm surprised the style checker didn't catch that. I've fixed them now

yeah, happened when I've pasted the new annotations in, I'm surprised the style checker didn't catch that. I've fixed them now
Rover656 (Migrated from github.com) approved these changes 2024-06-10 15:17:19 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Team-EnderIO/EnderIO#657
No description provided.