jshiell / checkstyle-idea

CheckStyle plug-in for IntelliJ IDEA
https://plugins.jetbrains.com/plugin/1065-checkstyle-idea
Other
889 stars 161 forks source link

Simple elements in one line always true when importing XML #633

Closed asteele0 closed 7 months ago

asteele0 commented 8 months ago

It appears that no matter what the imported Checkstyle XML config is, these options are always set to true: image

This can cause a mismatch between the resulting auto-formatted Java code and the Checkstyle inspection results.

Version

Android Studio Iguana | 2023.2.1 RC 2
Build #AI-232.10227.8.2321.11429013, built on February 8, 2024
Runtime version: 17.0.9+0--11185874 amd64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
Windows 10.0
GC: G1 Young Generation, G1 Old Generation
Memory: 4096M
Cores: 16
Registry:
    actionSystem.playback.typecommand.delay=10
    debugger.new.tool.window.layout=true
    ide.experimental.ui=true

Non-Bundled Plugins:
    org.jetbrains.idea.project.template.variables (0.5.0)
    CheckStyle-IDEA (5.87.1)
    PythonCore (232.10203.2)
    name.kropp.intellij.makefile (232.8660.88)
    net.seesharpsoft.intellij.plugins.csv (3.3.0-232)
    siosio.kodkod (2.0.1)
    com.github.copilot (1.4.19.4986)
    com.github.chocovon.debug-variable-extractor (2.4.0)
    org.jetbrains.settingsRepository (232.9921.28)

Reproducing

Given checkstyle.xml containing the following

<!-- Allman braces -->
<module name="LeftCurly">
    <property name="severity" value="error" />
    <property name="option" value="nl" />
</module>
<module name="RightCurly">
    <property name="severity" value="error" />
    <property name="option" value="alone_or_singleline" />
</module>
<!-- Allows left and right braces in a single line, for compact statements -->
<module name="NeedBraces">
    <property name="severity" value="error" />
    <property name="allowSingleLineStatement" value="true" />
</module>

When importing the configuration into Android Studio image

With the following Java to format

class ADemoClass 
{
   public ADemoClass() { }
}

This java code is unchanged when performing an auto-format, while Checkstyle presents an error:

'{' at column 24 should be on a new line. (22:24) [LeftCurly]

image

Expected behavior

I would expect that when a configuration is imported as the code style, once auto-formatting has been performed, running Checkstyle against the code would not produce any warnings.


I can make a new issue, but I felt weird about making two back to back.

Spaces inside one line enum braces
With this whitespace configuration

<module name="WhitespaceAround">
    <property name="severity" value="error" />
</module>
<module name="WhitespaceAfter">
    <property name="severity" value="error" />
</module>

This setting isn't enabled: image

Checkstyle will error on

enum FooBar
{Foo, Bar, FooBar}

Expecting it to be

enum FooBar
{ Foo, Bar, FooBar }
jshiell commented 8 months ago

Great issue report!

I'll be honest though - this isn't high on my list to fix. The code formatter support was a contribution, and the contributor then vanished; and it has several notable bugs. Honestly, I've been tempted to remove it entirely.

Nonetheless, if they ever do get high enough for me to look at, this one will be first on the list as I really appreciate the thorough report. Likewise, very open to contributions to fix this if you're keen.

jshiell commented 7 months ago

I had a quick look, and it's odd. com.intellij.psi.codeStyle.CommonCodeStyleSettings sets the simple options to false, and our importer doesn't touch those settings:

public boolean KEEP_SIMPLE_BLOCKS_IN_ONE_LINE = false;
public boolean KEEP_SIMPLE_METHODS_IN_ONE_LINE = false;
public boolean KEEP_SIMPLE_LAMBDAS_IN_ONE_LINE = false;
public boolean KEEP_SIMPLE_CLASSES_IN_ONE_LINE = false;
public boolean KEEP_MULTIPLE_EXPRESSIONS_IN_ONE_LINE = false;

When I tried importing a file locally then they stayed unset.

However, it does seem fair that we turn these off when using nl or nlow, as otherwise - as you say - there will be a conflict after formatting. So I shall give that a go.

jshiell commented 7 months ago

I've had a go at fixing in 5.89.0 - can you please have a butchers and reopen if problems continue? As said, this isn't an area I have a lot of knowledge in, so it's entirely possible I've buggered it up or introduced new problems ... but let's hope not 🤞