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

new check: UnnecessarySemicolonInTryWithResources #6774

Closed
strkkk opened this issue May 23, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@strkkk
Copy link
Contributor

commented May 23, 2019

From #6752
For last resource in resources declaration semicolon is unnecessary.
It should be detected and reported as violation.
If closing paren is not on same line, no violation in default configuration.

$ cat Test.java
import java.io.*;

public class TestClass {
  void method(){
       try( OutputStream s = new PipedOutputStream();)  // violation
       {}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream();)  // violation
       {}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream(); // no violation, closing paren on next line
       ){}
   }
}; 

$ cat conf.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
       <module name="UnnecessarySemicolonInTryWithResources" />
    </module>
</module>

$ java -jar checkstyle-8.20-all.jar -c conf.xml Test.java
Starting audit...
here is violations at lines commented with // violation

Additional boolean property allowWhenNoBraceAfterSemicolon (default values is true) to make an exception for lines, where closing parentheses is on the next line:

$ cat Test.java
import java.io.*;

public class TestClass {
  void method(){
       try( 
            OutputStream s = new PipedOutputStream();)  // violation
       {}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream();)  // violation
       {}
       try(
           OutputStream s = new PipedOutputStream(); // violation
           ) {}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream(); // violation 
       ){}
   }
}; 

$ cat conf.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
       <module name="UnnecessarySemicolonInTryWithResources">
          <property name="allowWhenNoBraceAfterSemicolon" value="false"/>
       </module>
    </module>
</module>

$ java -jar checkstyle-8.20-all.jar -c conf.xml Test.java
Starting audit...
here is violations at lines commented with // violation
@romani

This comment has been minimized.

Copy link
Member

commented May 27, 2019

yes, lets reconcile conflict of interests by special property allowWhenNoBraceAfterSemicolon=(true|false) with default value to be true.

In your personal style you will use allowWhenNoBraceAfterSemicolon=false.

for allowWhenNoBraceAfterSemicolon=true:

       try(
           OutputStream s = new PipedOutputStream(); // no violation
       ){}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream(); // no violation
       ){}
       try(
           OutputStream s = new PipedOutputStream();) // violation
       {}
       try(
           OutputStream s1 = new PipedOutputStream();
           OutputStream s2 = new PipedOutputStream();) // violation
       {}
       try(OutputStream s = new PipedOutputStream();) { // violation
       }
       try(Stream s1 = new Stream();Stream s2 = new Stream();){ // violation
       }

you can read more on why trailing comma is good at https://checkstyle.org/config_coding.html#ArrayTrailingComma , the same is true for ; (minimization of code diff).


jdk spec - https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.20.3

@checkstyle checkstyle deleted a comment from strkkk Jun 1, 2019

@checkstyle checkstyle deleted a comment from strkkk Jun 1, 2019

@romani

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

I am ok with this proposal.
@rnveach and @pbludov , please your thoughts on this

@rnveach

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

I am ok with proposal.

@romani

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

@pbludov , even we already have two approval ,please review.

@pbludov

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

I am ok with this proposal.

@rnveach

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

Module was merged

@rnveach rnveach closed this Jun 15, 2019

@RohanNagar RohanNagar referenced this issue Jun 24, 2019

Open

Cleanup Tasks #232

3 of 12 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.