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

Do not inherit "throws Exception" from Callable to compile on JDK8 #5191

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Jan 4, 2023

In #5118, I have created a generic method that rethrows callable's (checked) exception using sneaky throws approach - but the code is not compilable on JDK8 because the JDK8 javac fails to infer types properly:

 [nb-javac] /space/src/vscode/netbeans/java/java.lsp.server/nbcode/integration/src/org/netbeans/modules/nbcode/integration/commands/ProjectAuditCommand.java:149: error: call() in <anonymous org.netbeans.modules.nbcode.integration.commands.ProjectAuditCommand$> cannot implement call() in OCIOperation
 [nb-javac]         return OCIManager.usingSession(auditWithProfile, () -> v.findKnowledgeBase(knowledgeBase).
 [nb-javac]                                                          ^
 [nb-javac]   overridden method does not throw Exception

One way is to typecast the lambda or augment the mehod call using explicit type parameters <Object, Exception>, but it seems that removing extends Callable from the OCIOperation interface does the same - and leaves caller code clean - and compiles on both JDK8 and JDK11 (JDK17)

Please review asap - the code reached master branch and failed vscode testsuite ... was not discovered, since I didn't put enough labels on #5118 - sorry for the inconvenience.

@sdedic sdedic added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests enterprise [ci] enable enterprise job labels Jan 4, 2023
@sdedic sdedic added this to the NB17 milestone Jan 4, 2023
@sdedic sdedic requested review from jlahoda and jhorvath January 4, 2023 08:58
@sdedic sdedic self-assigned this Jan 4, 2023
@sdedic sdedic requested a review from mbien January 4, 2023 08:59
Copy link
Contributor

@jhorvath jhorvath left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

makes sense.

If you want to run the LSP/graal jobs on more/other labels just tell me and I add it to the list. Currently you have to be fairly specific there and request LSP or VSCode Extension to run any of them.

@mbien
Copy link
Member

mbien commented Jan 4, 2023

merging since this fixes master

@mbien mbien merged commit 316a3da into apache:master Jan 4, 2023
@jlahoda
Copy link
Contributor

jlahoda commented Jan 4, 2023

Looks good to me, ftr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enterprise [ci] enable enterprise job LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants