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

KotlinNullPointerException on antlr dependency #300

Closed
martinbonnin opened this issue Oct 9, 2020 · 11 comments · Fixed by #302 or #307
Closed

KotlinNullPointerException on antlr dependency #300

martinbonnin opened this issue Oct 9, 2020 · 11 comments · Fixed by #302 or #307
Labels
bug Something isn't working
Milestone

Comments

@martinbonnin
Copy link
Contributor

martinbonnin commented Oct 9, 2020

Reproducer project there: https://github.com/martinbonnin/test-antlr/

Calling ./gradlew buildHealth --stacktrace fails with:

Caused by: kotlin.KotlinNullPointerException
        at com.autonomousapps.internal.ConsoleReport$Companion.from(models.kt:487)
        at com.autonomousapps.tasks.BuildHealthTask.action(BuildHealthTask.kt:46)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:104)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.doExecute(StandardTaskAction.java:49)

This only happens on current main. The sample projects expects 0.61.1-SNAPSHOT in mavenLocal. 0.61.0 is ok (although it displays "null" for configurations). I guess it has to do with the fact that the antlr plugin adds the dependency automagically.

@autonomousapps
Copy link
Owner

autonomousapps commented Oct 9, 2020

Huh, that's strange. The antlr project is simple added directly to the jar (see here and here). I see I misunderstood you. Your project uses antlr, this is not about this plugin's use of antlr.

The reproducer is appreciated.

@autonomousapps
Copy link
Owner

Well, I can get it to recognize the antlr configuration, but it still reports the dependency as unused (incorrectly, I assume)

> Task :buildHealth
Advice for root project
Unused dependencies which should be removed:
- antlr("org.antlr:antlr4:4.8-1")

Transitively used dependencies that should be declared directly as indicated:
- api("org.antlr:antlr4-runtime:4.8-1")

@autonomousapps
Copy link
Owner

I have used antlr, but it's been a while. Do you have an idea why the plugin wouldn't "see" any code from that jar being used? Btw, once I merge to main, you can add this to ignore the false warning

dependencyAnalysis {
  issues {
    all {
      onUnusedDependencies {
        exclude("org.antlr:antlr4")
      }
    }
  }
}

@autonomousapps autonomousapps added this to the 1.0 milestone Oct 9, 2020
@autonomousapps autonomousapps added the bug Something isn't working label Oct 9, 2020
@martinbonnin
Copy link
Contributor Author

martinbonnin commented Oct 10, 2020

I think it's correct that antlr isn't used. It's the compiler that is used at compile time but at runtime only antlr-runtime is used (transitively)

@autonomousapps
Copy link
Owner

autonomousapps commented Oct 10, 2020 via email

@autonomousapps
Copy link
Owner

While I have merged the PR, this issue is not quite resolved to my satisfaction.

@autonomousapps
Copy link
Owner

autonomousapps commented Oct 12, 2020

@autonomousapps
Copy link
Owner

I'm starting to think what I actually need is special-handling for non-standard configurations. This is now a design question.

@martinbonnin
Copy link
Contributor Author

Here it is, the antlr plugin sets the compile configuration as a descendant of the antlr configuration

That's interesting... Is there a reason to do this instead of adding antlr-runtime to the compileClasspath configuration, like SQLDelight is doing

I'm starting to think what I actually need is special-handling for non-standard configurations. This is now a design question.

I guess part of why that's hard here because it's hard to tell whether a given dependency was added by the user or by another plugin?...

@autonomousapps
Copy link
Owner

For me, I think I just need to draw the line somewhere. I can't support every plugin out of the box. But to avoid giving bad information to users, I think I might have the plugin ignore dependencies that are on a custom, not-well-known, configuration.

I can imagine supporting additional plugins (like antlr), but that won't solve the general case, which is what I'm talking about now.

@autonomousapps
Copy link
Owner

The latest PR resolves this in a more general and satisfying way. The plugin should now ignore all "unusual" configurations. We can add support for them one at a time as it may seem useful.

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
2 participants