8000 [MNG-8727] Prepare for `Java 24` - fix `unnecessary` throws leftover; promoting `ExecutorException` to `Exception` being stringent to ensure `catch` by Pankraz76 · Pull Request #2336 · apache/maven · GitHub
[go: up one dir, main page]
Skip to content

[MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException to Exception being stringent to ensure catch #2336

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException to Exception being stringent to ensure catch #2336

wants to merge 1 commit into from

Conversation

Pankraz76
Copy link
Contributor
@Pankraz76 Pankraz76 commented May 14, 2025 •
edited
Loading

@Pankraz76 Pankraz76 changed the title [MNG-8727] Prepare for Java 24 - Unnecessary throws [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover May 14, 2025
This was referenced May 14, 2025
Merged
Draft
@Pankraz76 Pankraz76 marked this pull request as ready for review May 14, 2025 07:11
@Pankraz76
Copy link
Contributor Author

leftover...

image

thats why CPU out-smart human try&error, not the other way around, right?:
https://docs.openrewrite.org/recipes/staticanalysis/unnecessarythrows

@michael-o
Copy link
Member

Is this an unchecked exception?

@Pankraz76
Copy link
Contributor Author
Pankraz76 commented May 14, 2025 •
edited
Loading

sorry. No its Runtime. Does this make sense then?
This is kind of surprise.

image

@Pankraz76
Copy link
Contributor Author
Pankraz76 commented May 14, 2025 •
edited
Loading

Is this an unchecked exception?

https://stackoverflow.com/questions/824217/should-methods-that-throw-runtimeexception-indicate-it-in-method-signature

It is no longer obvious whether the exception has to be explicitly handled.
Having to add runtime exceptions in every method declaration would reduce a program's clarity.

gnodet
gnodet reviewed May 14, 2025
View reviewed changes
impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java Outdated
@@ -100,8 +100,7 @@ public String artifactPath(ExecutorRequest.Builder executorRequest, String gav,
}

@Override
public String metadataPath(ExecutorRequest.Builder executorRequest, String gav, String repositoryId)
throws ExecutorException {
public String metadataPath(ExecutorRequest.Builder executorRequest, String gav, String repositoryId) {
Copy link
Contributor
@gnodet gnodet May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The interface actually throws the exception. Given this is a public method, I'd rather keep it, as it may forbid derived classes to actually throw the exception in an overridden method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it's an unchecked exception, so the argument is moot.

@michael-o
Copy link
Member

Unchecked ones should be in Javadoc only, not on the signature.

@Pankraz76 Pankraz76 changed the title [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException to checked May 14, 2025
@Pankraz76
Copy link
Contributor Author

Unchecked

why guess, when able to make clear statement?

@Pankraz76 Pankraz76 requested a review from gnodet May 14, 2025 08:49
@Pankraz76
Copy link
Contributor Author

Unchecked ones should be in Javadoc only, not on the signature.

kindly request critics.

@Pankraz76 Pankraz76 changed the title [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException to checked [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException from Runtime- to Exception being stringent to ensure catch May 14, 2025
Pankraz76
Pankraz76 commented May 14, 2025
View reviewed changes
impl/maven-executor/src/main/java/org/apache/maven/api/cli/ExecutorException.java
@@ -29,7 +29,7 @@
* @since 4.0.0
*/
@Experimental
public class ExecutorException extends RuntimeException {
Copy link
Contributor Author
@Pankraz76 Pankraz76 May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

sounds important enough, being worth to give dedication and segregation.

@Pankraz76 Pankraz76 changed the title [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException from Runtime- to Exception being stringent to ensure catch [MNG-8727] Prepare for Java 24 - fix unnecessary throws leftover; promoting ExecutorException to Exception being stringent to ensure catch May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnodet gnodet Awaiting requested review from gnodet

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Pankraz76 @michael-o @gnodet
0