checkstyle
Allow multiline UnnecessaryParentheses
using Adding parenthesis around multiline returns (honestly not certain all of the rules) is how prettier treats typescript and thus the prettier-plugin-java treats java. I'd like a way to be able to say allow unnecessary parentheses when multiline.
if not java 17, you should be able to use old instanceof cast pattern to get the same results
public abstract class AbstractEntity<ID extends AbstractIdentity<? extends Serializable>> {
private boolean dirty;
private ID id;
private int version;
protected abstract boolean canEqual(AbstractEntity<?> that);
@Override
public final boolean equals(Object other) {
if (other instanceof AbstractEntity<?> that) {
return (
that.canEqual(this) &&
Objects.equals(this.id, that.id) &&
this.version == that.version &&
this.dirty == that.dirty
);
}
return false;
}
}
<?xml version="1.0" ?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="TreeWalker">
<module name="UnnecessaryParentheses" />
</module>
</module>
suggested:
<?xml version="1.0" ?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="TreeWalker">
<module name="UnnecessaryParentheses">
<property name="allowMultiline" value="true" />
</module>
</module>
</module>
com.puppycrawl.tools:checkstyle:{strictly 9.3} -> 9.3
This style is very unusual. Please try to explain reasons of it.
Meanwhile, you looks like want to suppress violation on some specific use case. Do you think that you can archive it by xpath suppression?
Or https://checkstyle.org/filters/suppresswithplaintextcommentfilter.html
With offCommentFormat="return ("
The style comes from prettier Plug-In Java which tries to be consistent with the prettier formatting of typescript. Prettier is a very common tool in the javascript world for automatic formatting.
Oh I just realized that I already said all of that in the original ticket. Can you please clarify the question? Or did you need a link to the prettier documentation.
IMO, part of the issue even supporting this, even if we agreed to the issue, is how will you identify this case versus real errors.
How is this:
return (
that.canEqual(this) &&
Objects.equals(this.id, that.id) &&
this.version == that.version &&
this.dirty == that.dirty
);
any different than this which I think is pretty realistic as most if statements eventually end up spanning multiple lines:
if ((someLongVariableOrMultipleExpressions
== somethingElse())) {
This is also unnecessary and spans multiple lines like your example.
I believe what @romani was leading to is what are the rules you want us to support implementation to? How do we identify what is a "prettier" versus what is a true error?
Allow multiline
Do you just want to suppress all multilines? I see this as just leading to the above example of false negatives. I am not good with xpath to show an example of suppressing this case.
Maybe you should ask yourself why you keep this check enabled and not just turn it off completely?
you need a link to the prettier documentation.
This may help you case, but I am sorry to say, I don't think we have the time to research this ourselves and would need someone to provide all the details.
any different than this which I think is pretty realistic as most if statements eventually end up spanning multiple lines:
if your if statement spans multiple lines I think you generally need to refactor. I've seen that code and it was very very bad and in order to continue adding to it we need to refactor to a better design pattern, specifically strategy. So I wrote this garbage just to get a long multiline, if, it's not a real implementation of equals, and this is how prettier-plugin-java formats it.
if (
that == this &&
this.canEqual(that) &&
that.canEqual(this) &&
this.hashCode() == that.hashCode() &&
this.toString().equals(that.toString())
) {
return true;
}
I think that these parens only happen on return statements. I think that's the only place I've ever seen them. I could open a ticket in prettier-plugin-java and ask them if there are any other cases that would wrap in parenthesis.
Do you just want to suppress all multilines? I see this as just leading to the above example of false negatives. I am not good with xpath to show an example of suppressing this case. Maybe you should ask yourself why you keep this check enabled and not just turn it off completely?
Check the attitude please. This appears to be a missing piece of the original ticket. Perhaps I thought it was inferred by the request and used a bad property name. This is my current workaround to allowing this syntax.
public final boolean equals(@Nullable Object other) {
if (other instanceof AbstractSurrogateEntity<?> that) {
// CHECKSTYLE.OFF: UnnecessaryParentheses
return (
that.canEqual(this) &&
Objects.equals(this.id, that.id) &&
Objects.equals(this.version, that.version) &&
this.dirty == that.dirty
);
// CHECKSTYLE.ON: UnnecessaryParentheses
}
return false;
}
This style is very unusual. Please try to explain reasons of it. This may help you case, but I am sorry to say, I don't think we have the time to research this ourselves and would need someone to provide all the details.
I was mostly responding to "this style is very unusual" to which I suggest it's not because prettier is very common, formatting java with it isn't.
I don't think prettier has a comprehensive style guide, unfortunately, because it's meant to be run programatically. Its playground afaik, will not run the java plugin, although you could experiment with typescript. If you want I could upload an archive that would set up prettier with the java plugin for you to run it.
https://prettier.io/docs/en/technical-details https://github.com/jhipster/prettier-java
Intellij can also run it on save. Instructions on how to install on node are in prettier-java, once you have that. In intellij language->frameworks must be set up to use the correct node/npm instance, configure prettier as follows in the UI, and of course check the box to use it in actions on save
$ cat Test.java
public abstract class AbstractEntity<ID extends AbstractIdentity<? extends Serializable>> {
private boolean dirty;
private ID id;
private int version;
protected abstract boolean canEqual(AbstractEntity<?> that);
@Override
public final boolean equals(Object other) {
if (other instanceof AbstractEntity<?> that) {
return (
that.canEqual(this) &&
Objects.equals(this.id, that.id) &&
this.version == that.version &&
this.dirty == that.dirty
);
}
return false;
}
}
$ cat config.xml
<?xml version="1.0" ?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="TreeWalker">
<module name="UnnecessaryParentheses" />
</module>
</module>
$ java -jar checkstyle-10.20.0-all.jar -g -c config.xml Test.java
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suppressions PUBLIC
"-//Checkstyle//DTD SuppressionXpathFilter Experimental Configuration 1.2//EN"
"https://checkstyle.org/dtds/suppressions_1_2_xpath_experimental.dtd">
<suppressions>
<suppress-xpath
files="Test.java"
checks="UnnecessaryParenthesesCheck"
query="/COMPILATION_UNIT/CLASS_DEF[./IDENT[@text='AbstractEntity']]/OBJBLOCK/METHOD_DEF[./IDENT[@text='equals']]/SLIST/LITERAL_IF/SLIST/LITERAL_RETURN/EXPR"/>
</suppressions>
Checkstyle ends with 1 errors.
reuse xpath from above and just generalize it abit:
$ cat Test.java
public abstract class AbstractEntity<ID extends AbstractIdentity<? extends Serializable>> {
private boolean dirty;
private ID id;
private int version;
protected abstract boolean canEqual(AbstractEntity<?> that);
@Override
public final boolean equals(Object other) {
if (other instanceof AbstractEntity<?> that) {
return (
that.canEqual(this) &&
Objects.equals(this.id, that.id) &&
this.version == that.version &&
this.dirty == that.dirty
);
}
return false;
}
}
$ cat config.xml
<?xml version="1.0" ?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="TreeWalker">
<module name="UnnecessaryParentheses" />
<module name="SuppressionXpathSingleFilter">
<property name="checks" value="UnnecessaryParentheses"/>
<property name="query" value="//LITERAL_RETURN/EXPR"/>
</module>
</module>
</module>
$ java -jar checkstyle-10.20.0-all.jar -c config.xml Test.java
Starting audit...
Audit done.
We will wait for other users support for this problem UnnecessaryParentheses.
If no feedback, we will consider xpath suppression/filter as good compromise for now and users should use it if they want to tune Check to address their needs.