Skip to content

Conversation

@gaul
Copy link
Member

@gaul gaul commented Aug 31, 2019

Closing an InputStream in a finally block can mask exceptions thrown
in the try or catch blocks. Found via error-prone. Reference:

https://errorprone.info/bugpattern/Finally

@rfscholte
Copy link
Contributor

Maybe this is a good moment to update the Java requirements and use try-with-resources

@Tibor17
Copy link

Tibor17 commented Aug 31, 2019

@gaul Try to rewrite this title and try to find where we are missing Java 7 features. We ASF developers are not so fast to update all 100 Maven projects, so it means we need some help from you contributors.
Then pls run the build mvn -P run-its verify to make sure all changes are alright.

catch ( IOException e )
{
throw new MojoExecutionException( e.getMessage(), e );
getLog().warn( "Could not close " + PLUGIN_HELP_PATH + ": " + e.getMessage() );

Choose a reason for hiding this comment

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

We should use a try-with-resources block, the exception will be eventually added as 'suppressed'

Copy link

Choose a reason for hiding this comment

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

@eolivelli
If @gaul would fix all Java 7 features then we should create a new Jira ticket as we did it in other projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to let maven plugins use error-prone with the fewest suppressions. Moving the entire project to Java 7 is one way to do this although I don't know if that aligns with the project's goals.

Copy link

Choose a reason for hiding this comment

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

It perfectly fits to our goals. ;-)
We set the Javac version 1.7 and we were updating the code to J7.
Later we will do the same for J8 - inner classes/Lambda, I/O, loops with method chaining, use of the class Optional, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Closing an InputStream in a finally block can mask exceptions thrown
in the try or catch blocks.  Found via error-prone.  Reference:

https://errorprone.info/bugpattern/Finally
@gaul gaul changed the title Do not throw exception in finally block Prefer try-with-resources for file error handling Aug 31, 2019
@gaul
Copy link
Member Author

gaul commented Sep 1, 2019

Superseded by #18.

@gaul gaul closed this Sep 1, 2019
@gaul gaul deleted the finally branch September 1, 2019 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants