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
Whitespaces at the head of line before a cast and after an instanceof or an "enhanced for" results to "'(' should be on the previous line"error #10012
Comments
Jacques, thanks for the detailed report! Could you pare down your code example and config to just the target code, check module, and specific properties that you are having an issue with? It will make it easier for maintainers to approve and get started on this. If you think that it adds context, you can still include the entire files using the |
Hi, Sure I'll do that, I guess directly by editing... |
Done in 1st comment, TIA for reviews |
So, just to be clear, you are not expecting the following violations? ➜ src javac Test.java
➜ src cat Test.java
public class Test {
void method1(Object o) {
if (o instanceof String) {
((CharSequence) o).charAt(0);
}
}
void method2(Object... objects) {
for (Object o : objects) {
((CharSequence) o).charAt(0);
}
}
}
➜ src cat config.xml
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="TreeWalker">
<!-- Checks for whitespace -->
<module name="SeparatorWrap">
<property name="tokens"
value="COMMA,LPAREN,RPAREN,RBRACK,ARRAY_DECLARATOR"/>
<property name="option" value="eol"/>
</module>
</module>
</module>
➜ src java -jar checkstyle-8.42-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/nick/Desktop/Tester/src/Test.java:5:13: '(' should be on the previous line. [SeparatorWrap]
[ERROR] /home/nick/Desktop/Tester/src/Test.java:11:13: '(' should be on the previous line. [SeparatorWrap]
Audit done.
Checkstyle ends with 2 errors.
|
No, that's not what I see at https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html where [SeparatorWrap] does not show. I see only '(' should be on the previous line. Maybe I miss something related to SeparatorWrap? |
Your description is a bit confusing, because you refer to two different checks |
OK, I'll, thanks |
Hi, You are right, MethodParamPad was confusing and misleading. It came from the allowLineBreaks solution I hastily found on StackOverflow. It's unrelated to the issue at hand that is indeed only related to SeparatorWrap. You are right with your example above too. And yes, I'm not expecting the violations for a cast after and instanceof or an "enhanced for". Now, if I remove LPAREN from
Then the remaining issues related to casting after instanceof or "enhanced for" disappear. I mean no <<'(' should be on the previous line>> errors remain. I did not create the checkstyle config we use. The person who did is no longer available. I'm not sure removing LPAREN like above is the right solution. I'll revert back to 1 year ago to see what's happening when does so and if it's applicable... |
If it helps at all, our documentation always shows our own usage of checks at the bottom of each section. Here is our config for checkstyle/config/checkstyle_checks.xml Lines 844 to 857 in 258a4cf
|
Thanks for the suggestion. It gives the same result (number of errors) than removing LPAREN. I'll check both options on an one year old situation. Currently remain 116 errors, was 35k+ last year. |
OK, there is a difference when using your proposition with a checkout from 1 year ago, 26953 errors with the original and 26761 w/ your changes applied. So we would escape 192 issues. When only removing LPARENT we get 26866 errors, so only 87 issues less. I'm not sure what to do yet. I'll discuss that w/ the team... Thanks again for your help! |
This is not a bug |
Hi,
I'm a committer of the Apache OFBiz project. We use a checkstyle configuration corresponding to the OFBiz coding conventions roughly simplified “Sun Coding Standards” 150 characters line length and NewlineAtEndOfFile value="lf_cr_crlf"
Below, first, only the relevant parts are shown (checkstyle and 2 Java classes).
Then, for each, the complete source using the "details" tag.
C:\projectsASF\Git\ofbiz-framework>type config\checkstyle\checkstyle.xml
C:\projectsASF\Git\ofbiz-framework>type framework\base\src\main\java\org\apache\ofbiz\base\conversion\NumberConverters.java
import java.math.BigDecimal;
import java.math.BigInteger;
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.Locale;
import java.util.TimeZone;
import org.apache.ofbiz.base.util.StringUtil;
/** Number Converter classes. */
public class NumberConverters implements ConverterLoader {
}
framework/base/src/main/java/org/apache/ofbiz/base/util/SSLUtil.java
import java.io.IOException;
import java.math.BigInteger;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.Principal;
import java.security.SecureRandom;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509KeyManager;
import javax.net.ssl.X509TrustManager;
import org.apache.ofbiz.base.component.ComponentConfig;
import org.apache.ofbiz.base.config.GenericConfigException;
/**
*/
public final class SSLUtil {
}
I have been working on MethodParamPad[0]. I could fix few issues but despite my efforts[1][2], as you can see here we have still 22 issues of "'(' should be on the previous line." in Java source code.
Among them there are some cases that can't be handled by changing code, notably when a casting is necessary. Using instanceof or "enhanced for" are typical cases. Of course we could add an initial line to assign the cast to another var before the real expression is used. But it's a tedious work and I'd prefer if we could anticipate (we still recommend JDK 8 but already support JDK 11) something like what JDK 14 provides (I know you also rely on JDK 8 for now). I mean :
Looking at MethodParamPad[0], how could we allow white-spaces at head of the line after instanceof or "enhanced for", do I miss something or is this a real bug?
Note: we don't want to use
Because we want to check the other cases, as the ones I fixed.
TIA
Jacques
[0] https://checkstyle.sourceforge.io/config_whitespace.html#MethodParamPad
[1] https://github.com/apache/ofbiz-framework/commit/45f50c910d64c12a2601164fbc64a4a02ef1d833/
[2] https://github.com/apache/ofbiz-plugins/commit/dbfa4f0ca0620896a2313d5b784d197bbc25d4d3/
The text was updated successfully, but these errors were encountered: