Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: compilation error with providers and throws #533

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

mohamed-abdelnour
Copy link
Contributor

@mohamed-abdelnour mohamed-abdelnour commented Apr 6, 2024

Hello, everyone! Thank you all for your work.

Avaje-Inject currently generates invalid code when a "provider" method (i.e. @Prototype, @Lazy, etc.) has a throws clause.

There are two problems:

  1. a syntax error: try { ... } } catch.
  2. the lambdas passed to Builder::register(Lazy|Provider) do not handle the declared Throwables.

This fixes those problems by wrapping the bodies of the lambdas passed to Builder::register(Lazy|Provider) in try blocks.

To demonstrate, here are the (relevant) differences in the generated code before and after applying the fix:

--- before-fix-fallible-providers/blackbox-test-inject/target/generated-sources/annotations/org/example/myapp/config/AppConfig$DI.java
+++ fix-fallible-providers/blackbox-test-inject/target/generated-sources/annotations/org/example/myapp/config/AppConfig$DI.java
@@ -71,15 +71,15 @@ public final class AppConfig$DI  {
   public static void build_newBuilderThrows(Builder builder) {
     if (builder.isAddBeanFor(AppConfig.BuilderThrows.class)) {
       var factory = builder.get(AppConfig.class);
+      builder.asPrototype()
+.registerProvider(() -> {
       try {
-        builder.asPrototype()
-  .registerProvider(() -> {
           return factory.newBuilderThrows();
-        });
-      }
       } catch (Throwable e) {
         throw new RuntimeException("Error during wiring", e);
       }
+      });
+    }
   }

   /**
@@ -112,15 +112,15 @@ public final class AppConfig$DI  {
   public static void build_generalSecondaryThrows(Builder builder) {
     if (builder.isAddBeanFor(AppConfig.MySecTypeThrows.class)) {
       var factory = builder.get(AppConfig.class);
+      builder.asSecondary()
+.registerProvider(() -> {
       try {
-        builder.asSecondary()
-  .registerProvider(() -> {
           return factory.generalSecondaryThrows();
-        });
-      }
       } catch (Throwable e) {
         throw new RuntimeException("Error during wiring", e);
       }
+      });
+    }
   }

   /**
--- before-fix-fallible-providers/blackbox-test-inject/target/generated-sources/annotations/org/example/myapp/lazy/LazyFactory$DI.java
+++ fix-fallible-providers/blackbox-test-inject/target/generated-sources/annotations/org/example/myapp/lazy/LazyFactory$DI.java
@@ -41,14 +41,14 @@ public final class LazyFactory$DI  {
   public static void build_lazyIntThrows(Builder builder) {
     if (builder.isAddBeanFor("factorythrows", LazyBean.class)) {
       var factory = builder.get(LazyFactory.class);
+      builder.registerLazy(() -> {
       try {
-        builder  .registerLazy(() -> {
           return factory.lazyIntThrows(builder.getNullable(AtomicBoolean.class,"!initialized"));
-        });
-      }
       } catch (Throwable e) {
         throw new RuntimeException("Error during wiring", e);
       }
+      });
+    }
   }

 }

Copy link
Collaborator

@SentryMan SentryMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, everything seems in order

@SentryMan SentryMan added the bug Something isn't working label Apr 6, 2024
@SentryMan SentryMan added this to the 9.12 milestone Apr 6, 2024
@SentryMan SentryMan requested a review from rbygrave April 6, 2024 04:32
@rbygrave rbygrave merged commit 2b7eaa8 into avaje:master Apr 9, 2024
4 checks passed
@rbygrave
Copy link
Contributor

rbygrave commented Apr 9, 2024

Great work, thanks !!

@mohamed-abdelnour
Copy link
Contributor Author

Thank you!

@mohamed-abdelnour mohamed-abdelnour deleted the fix-fallible-providers branch April 10, 2024 15:02
@SentryMan
Copy link
Collaborator

Consider yourself welcomed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants