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

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

Closed
asteele0 opened this issue Mar 6, 2024 · 3 comments
Closed

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

asteele0 opened this issue Mar 6, 2024 · 3 comments

Comments

@asteele0
Copy link

asteele0 commented Mar 6, 2024

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
Copy link
Owner

jshiell commented Mar 10, 2024

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
Copy link
Owner

jshiell commented Mar 27, 2024

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
Copy link
Owner

jshiell commented Mar 27, 2024

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 🤞

@jshiell jshiell closed this as completed Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants