java.lang.ClassCastException SingleMemberAnnotation to NormalAnnotation

[astubbs]

Error adding a annotation value to @generated

java.lang.ClassCastException: class org.eclipse.jdt.core.dom.SingleMemberAnnotation cannot be cast to class
org.eclipse.jdt.core.dom.NormalAnnotation (org.eclipse.jdt.core.dom.SingleMemberAnnotation and
org.eclipse.jdt.core.dom.NormalAnnotation are in unnamed module of loader 'app')
	at org.jboss.forge.roaster.model.impl.AnnotationImpl.setAnnotationValue(AnnotationImpl.java:615)
	at org.jboss.forge.roaster.model.impl.AnnotationImpl.addAnnotationValue(AnnotationImpl.java:731)
	at com.google.common.truth.extension.generator.TruthGenerator.generate(TruthGenerator.java:44)
	at com.google.common.truth.TruthGeneratorTest.poc(TruthGeneratorTest.java:21)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:567)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:221)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)

[astubbs]

FYI here's the example - just fork uncomment it.
https://github.com/google/truth/pull/894/files#diff-ac180ce002e165644a42d41860f2a235efceb7aad733ebf6751c2825cf77d996R48

[astubbs]

Cool project btw

[gastaldi]

@astubbs you need to use generated.setStringValue("truth-generator"); instead. The setAnnotationValue method is intended for cases where you want to add a nested annotation inside an annotation. Closing

[astubbs]

Can the code then not be improved to prevent this mistake from happening, or produce a better error? perhaps if String is assignable from (parameter) throw illegal argument exception? It would be a common mistake IMO.

[gastaldi]

Sure, that would be nice to have. Fancy submitting a PR? 😉

[astubbs]

Maybe. The issues are starting to pile up. Why close the issue then? Perhaps you should reopen this issue if you agree there's a UX problem, instead of closing it.

[astubbs]

Or - if you don't think it's a bug, tag it as not a bug, but a ux improvement or something. Any lib that throws out NPE or Class casts is a bug IMO. What type of bug though...

[gastaldi]

That makes sense. I'll leave it open until we get a better error message in this case

[lincolnthree]

@astubbs Thanks for the issue! Just getting up to speed on this. I agree that re-classifying the issue as a feature request seems appropriate.

In the future, if you would like to report a bug, it would be helpful to also provide a minimal reproduction of the defect so the team can more quickly understand and fix the problem without forking or downloading other codebases. A link to a branch with the comment "just fork uncomment it" means any number of things to me.

If you must link to a fork or branch of a larger project, consider providing the git commands and detailed steps to reproduce the issue in the project.

Regarding libraries throwing NPEs and other exceptions. I agree, particularly since what Roaster is doing is attempting to hide the messy parts of the Eclipse JDT to provide a tasteful and easy to understand way to manipulate the very complex domain of Java Source; however, nobody is perfect and we only try to predict or code a response for every possible misuse of the JDT API. It does provide its own (unfortunately terse) error messages in the end. We can only do our best with the limited time we have.

At some point, it becomes impractical to attempt encapsulation of every underlying language feature or facet, or we'd have re-written the language. Hence, PRs very welcome.

Also, cool project! :) Given that you seem to care about legibility, we would of course welcome your contribution! It's very much in line with what we are trying to achieve.

[astubbs]

@lincolnthree Thanks for the response! I agree it's helpful to provide more detail in bug reports, examples, tests etc. However I also think it's better to report them with some information rather than not at all if time is pressing.

I agree my link to my fork with comment was next to useless :)

I'm not accusing anyone of being perfect; however generally speaking, class casting is an unsafe operation (which is what caused this issue) in what is usually strongly typed language, so if you do it (cast) - you should be sure that the type are going to be compatible. Otherwise - test it first - as is being done in other parts of the code already. This is what I was getting at.

However after looking further and understanding a bit more - I see this class is choco-block with casts - it appears this part is quite a bit more complex than I assumed - first thinking this was simply missing a single type check :) I'll have to leave it there but I see this issue is actually much larger that I thought - which touches on what you were getting at.

BTW FYI - I had already submitted a PR for another issue of a similar type (input verification): https://github.com/forge/roaster/pull/200/files. PRs are nice, but we're all time poor :( I have moved away from my work with Roaster as I'm not working on the project that used it for a while now. But if I pick it back up again - there'll be more PRs to come.

Cheers!