CheckStyle - IDEA Code Style configuration#16349
Conversation
|
Please create issue and list Checks that we have , list of checkboxes, and checkout all that you covered, so we might improve step by step to cover all other . Please reference this issue in this PR and commit. |
bdae172 to
e6d2816
Compare
|
Please keep ci green. |
|
Did you reformat code by idea? |
d34b872 to
b9c7c72
Compare
|
@romani Yes, I did reformat the code using IntelliJ IDEA's formatter. Before reformatting, Checkstyle reported 28 errors, and after reformatting, it's down to 15 errors. The remaining issues require manual fixes :
Here's the test code for reference: import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class test {
private static final int MAX_LENGTH = 100;
private final List<String> items;
public Map<String, Integer> data = new HashMap<>();
public test() {this.items = new ArrayList<>();}
public test(List<String> items) {
this.items = items;
}
/**
* This method violates several formatting rules.
*/
public List<String> processItems(String input, int count) throws IllegalArgumentException {
if (input == null) { return null; }
String veryLongString = "This string intentionally goes beyond the 100 character limit to demonstrate a line length violation in the configuration";
boolean isValid = input.length() > 0 &&
count > 0;
String[] parts = new String[]{"a", "b", "c"};
List<String> results = new ArrayList<>();
if(isValid) {
for (int i = 0; i < count; i++)
{
results.add(input + i);
}
} else return Collections.emptyList();
try {
validateResults(results);
} catch (Exception e) {
results.clear();
}finally {
System.out.println("Done");
}
String result;
if (results.isEmpty()) {
result = "empty" +
"list";
}
else {
result = "populated list";
}
return results;
}
private void validateResults(List<String> results) {
assert results != null:"Results cannot be null";
String status;
if (results.isEmpty()) {
status = "Empty";
}
else {
status = "Not empty";
}
}
public enum ProcessingType { SIMPLE, NORMAL, COMPLEX, ADVANCED, SPECIAL, CUSTOM }
} |
please share in commit all IDEA formatting.
lets postpone this for later, IDEA formatting does not tell user what is best practice on naming. It is reponsibility of tools checkstyle or IDEA inspections. |
@romani i have commited the idea config that's i used i guess there will be violations by checkstyle here like pls clarify, I guess there is a bit confusion from my side . I intended to tell that the idea config is formatting code in line with checkstyle. |
|
I am confused, there is no |
|
I imported your config, select CheckstyleAntTask.java , right click, choose "Reformat code" menu, in popup window select only "Optimize imports". |
|
Fixed it! Turns out the issue was the missing version in the config—we had removed it earlier. The updated config now covers all the checks and everything is working perfectly :) |
|
I tried again, same result: please investigate IDEA CLI. example of how we use it checkstyle/.circleci/config.yml Line 25 in d4025b2 checkstyle/.ci/idea-inspection.sh Line 46 in d4025b2 please run it on our local from terminal. |
|
PS C:\Users\anmol\IdeaProjects\checkstyle3> git diff
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java b/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
index c7b8f09890..034e3ec34d 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
@@ -124,7 +124,7 @@ public class CheckstyleAntTask extends Task {
* is a violation.
*
* @param propertyName the name of the property to set
- * in the event of a failure.
+ * in the event of a failure.
*/
public void setFailureProperty(String propertyName) {
failureProperty = propertyName;
@@ -198,10 +198,10 @@ public class CheckstyleAntTask extends Task {
* Creates classpath.
*
* @return a created path for locating classes
- * @deprecated left in implementation until #12556 only to allow users to migrate to new gradle
- * plugins. This method will be removed in Checkstyle 11.x.x .
* @noinspection DeprecatedIsStillUsed
* @noinspectionreason DeprecatedIsStillUsed - until #12556
+ * @deprecated left in implementation until #12556 only to allow users to migrate to new gradle
+ * plugins. This method will be removed in Checkstyle 11.x.x .
*/Looks like it mainly adjusted indentation and spacing—I don't see any significant changes in the diff report. |
|
not working as expected on my side: |
|
plase try to use same version of as me. |
|
@Anmol202005 , can we finish this PR ? |
|
@romani |
|
while I waiting for laptop time to validate it... @Anmol202005 , can you make it as CI job ? same as we have idea inspection execution |
|
@romani is the config working now ! |
9c41ffc to
612e57b
Compare
| echo "Running Checkstyle..." | ||
| ./mvnw -e --no-transfer-progress clean compile antrun:run@ant-phase-verify | ||
| working_directory: ~/project | ||
| - store_artifacts: |
There was a problem hiding this comment.
lets run git diff as "run" and fail is any diff is present. Now it is hard for me to see what is a diff
We have this, that is used in other CIs.
Line 1066 in 3e4e237
There was a problem hiding this comment.
then, it is always going to fail.
There was a problem hiding this comment.
am i missing something ?
we can't rely on git-diff IMO
There was a problem hiding this comment.
if you have some diff, please share in gist of github, to let me investigate.
There was a problem hiding this comment.
in final result we need CI to fail if any diff is noticed :) .
right now we investigating if diff that we have is valid to be merged or we need to change configs
There was a problem hiding this comment.
we need same as at:
checkstyle/.github/workflows/google-java-format.yml
Lines 48 to 49 in 3e4e237
if diff exists we failing a CI .
for IDEA formatter we also need to run checkstye to see its output and understand we can follow IDEA formatting.
so checkstyle vaildation failing CI, git-diff runs on failures also(always).
https://discuss.circleci.com/t/on-fail-for-commands-in-steps/42496
when: on_fail
There was a problem hiding this comment.
please do diff after checkstyle execution.
To let me see what is ok by checkstyle and what is not.
Diff likely always exists, but it is ok, user will just apply patch from formatter (CI output, same as we do with spelling) or run IDEA formatter.
But what will be show stopper is conflict with checkstyle after formatting.
So I and all users should see diff and checkstyle execution at the same time in CI.
I suggesting to run checkstyle first, let it fail a CI, and git diff execution is done when: always to let it run on_fail and passed.
| # Remove closing tag and add new suppression with closing tag | ||
| sed -i '/<\/suppressions>/d' "$file" | ||
| cat >> "$file" << 'EOF' | ||
| <suppress-xpath | ||
| files=".*" | ||
| checks="WhitespaceAfterCheck" | ||
| query="//ARRAY_INIT/COMMA[following-sibling::*[1][self::RCURLY]]"/> | ||
| <suppress-xpath | ||
| files=".*" | ||
| checks="IndentationCheck" |
There was a problem hiding this comment.
this is concerning, we wil discuss this
There was a problem hiding this comment.
just to make ci green
There was a problem hiding this comment.
idea does not autoformat indentation and whitespaceAfter properly for array declaration.
if we are going to rely on git diff, it will be reverted.
There was a problem hiding this comment.
idea does not autoformat indentation and whitespaceAfter properly for array declaration.
I need to see it, we can open issue on Idea to fix, if we see a problem. But we might need to change settings of checkstyle to not conflict with it, if they is not critical
There was a problem hiding this comment.
@Anmol202005 , as this suppression is hack for now, please print git diff right after formatter run, but it should not fail a build by "exit 1", just infromational print of diff.
later on we run checkstyle
later on we do diff one more time but will "exit 1" if diff is found.
|
@romani just need a little clarification on how to proceed :) |
|
Please do git diff to let me see a problem. |
Done. vs checkstyle conflict after suppressions: first: bad formatting by IDEA. second violation is same as first: third is same problem: all other are same problem: it is known issue https://stackoverflow.com/a/53224774 |
done. |
|
https://stackoverflow.com/a/73545205 is this going to help ? BUT this https://youtrack.jetbrains.com/issue/IJPL-42157/An-option-to-apply-continuation-indent-to-method-name is hard blocker to use IDEA formatter. |
|
I need to get access to violations on WhitespaceAfterCheck and IndentationCheck . |
| <option name="CONTINUATION_INDENT_SIZE" value="4" /> | ||
| </indentOptions> | ||
| </codeStyleSettings> | ||
| </code_scheme> |
There was a problem hiding this comment.
| fi | ||
| mkdir -p "$RESULTS_DIR" | ||
|
|
||
| # Compile the project with Maven |
There was a problem hiding this comment.
| # Compile the project with Maven | |
| :: Compile the project with Maven |
whats difference between # and :: ? assuming its both comments?
| call "%IDEA_LOCATION%" format -r -s "%FORMAT_PATH%" -m "*.java" ^ | ||
| "%PROJECT_DIR%\src\xdocs-examples\java" | ||
|
|
||
| echo Formatting completed. |
There was a problem hiding this comment.
| echo Formatting completed. | |
| echo Formatting complete. |
try present not past. as all logs before have present time form as well
| echo Running Maven compile... | ||
| call mvn -e compile | ||
|
|
||
| ::Launch inspections |
There was a problem hiding this comment.
| ::Launch inspections | |
| # Launch inspections |
whats difference between # and :: ? assuming its both comments?
| ::---------------------------------------------------------------------- | ||
| :: IntelliJ IDEA inspections for checkstyle. | ||
| :: | ||
| :: Example: | ||
| :: SET IDEA_PATH=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2017.2.1\bin\idea.bat | ||
| :: .ci\idea-inspection.bat | ||
| ::---------------------------------------------------------------------- |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
done. |
|
its again very related. If we have eclipse config we can simply automate the checkstyle issues. |
|
spotless has recently gained idea support, might apply this via spot instead of having the custom version. Assuming you also just call the CLI as the spot plugin does. |
|
|
||
| run-formatter: | ||
| docker: | ||
| - image: checkstyle/idea-docker:jdk11-idea2023.3.4 |
There was a problem hiding this comment.
we migrated to qodana (official way to run inspections)
may it has formatting also
Issue #16427: IntelliJ IDEA Code Style configuration that matches our checkstyle_check.xml settings