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

clang-format: SpacesInSquareBrackets does not work properly for Java #77740

Closed
mikir opened this issue Jan 11, 2024 · 7 comments
Closed

clang-format: SpacesInSquareBrackets does not work properly for Java #77740

mikir opened this issue Jan 11, 2024 · 7 comments
Labels
bug Indicates an unexpected problem or unintended behavior clang-format

Comments

@mikir
Copy link

mikir commented Jan 11, 2024

The following Java snippet

public class Sample
{
    public static void createObject(Object[] arguments)
    {
        final int numElements = arguments.length;
        final Class<?>[] types = new Class<?>[numElements];
        for (int i = 0; i < numElements; ++i)
            types[i] = arguments[i].getClass();
    }
}

using the following clang-format configuration

---
Language: Java
BasedOnStyle: Microsoft
SpacesInSquareBrackets: false
...

is formatted as

public class Sample
{
    public static void createObject(Object[] arguments)
    {
        final int numElements = arguments.length;
        final Class<?>[] types = new Class<?>[ numElements ]; // wrong, spaces even if SpacesInSquareBrackets is false
        for (int i = 0; i < numElements; ++i)
            types[i] = arguments[i].getClass(); // ok, no spaces
    }
}

We are using clang-format Ubuntu clang-format version 14.0.0-1ubuntu1.1.

@mydeveloperday
Copy link
Contributor

This was still an issue with 17

@mydeveloperday mydeveloperday added the bug Indicates an unexpected problem or unintended behavior label Jan 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/issue-subscribers-bug

Author: Miki (mikir)

The following Java snippet
public class Sample
{
    public static void createObject(Object[] arguments)
    {
        final int numElements = arguments.length;
        final Class&lt;?&gt;[] types = new Class&lt;?&gt;[numElements];
        for (int i = 0; i &lt; numElements; ++i)
            types[i] = arguments[i].getClass();
    }
}

using the following clang-format configuration

---
Language: Java
BasedOnStyle: Microsoft
SpacesInSquareBrackets: false
...

is formatted as

public class Sample
{
    public static void createObject(Object[] arguments)
    {
        final int numElements = arguments.length;
        final Class&lt;?&gt;[] types = new Class&lt;?&gt;[ numElements ]; // wrong, spaces even if SpacesInSquareBrackets is false
        for (int i = 0; i &lt; numElements; ++i)
            types[i] = arguments[i].getClass(); // ok, no spaces
    }
}

We are using clang-format Ubuntu clang-format version 14.0.0-1ubuntu1.1.

@mydeveloperday
Copy link
Contributor

small repo

public static void foo()
{
    auto types = new A[numElements];
    auto types = new Class<T>[ numElements ];
}

@mydeveloperday
Copy link
Contributor

Workaround Add

SpacesInContainerLiterals: false

@mydeveloperday
Copy link
Contributor

So I do think this is missing code (which is handled in C#)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 73840332e22c..86e9ed931f7c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4472,6 +4472,10 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
   } else if (Style.Language == FormatStyle::LK_Java) {
     if (Left.is(tok::r_square) && Right.is(tok::l_brace))
       return true;
+    // spaces inside square brackets.
+    if (Left.is(tok::l_square) || Right.is(tok::r_square))
+      return Style.SpacesInSquareBrackets;
+
     if (Left.is(Keywords.kw_synchronized) && Right.is(tok::l_paren)) {
       return Style.SpaceBeforeParensOptions.AfterControlStatements ||
              spaceRequiredBeforeParens(Right);
diff --git a/clang/unittests/Format/FormatTestJava.cpp b/clang/unittests/Format/FormatTestJava.cpp
index 51afe79859ec..6bb4b0013eeb 100644
--- a/clang/unittests/Format/FormatTestJava.cpp
+++ b/clang/unittests/Format/FormatTestJava.cpp
@@ -625,5 +625,20 @@ TEST_F(FormatTestJava, ShortFunctions) {
                Style);
 }

+TEST_F(FormatTestJava, ConfigurableSpacesInSquareBrackets) {
+  FormatStyle Spaces = getLLVMStyle(FormatStyle::LK_Java);
+
+  verifyFormat("Object[] arguments", Spaces);
+  verifyFormat("final Class<?>[] types = new Class<?>[numElements];", Spaces);
+  verifyFormat("types[i] = arguments[i].getClass();", Spaces);
+
+  Spaces.SpacesInSquareBrackets = true;
+
+  verifyFormat("Object[ ] arguments", Spaces);
+  verifyFormat("final Class<?>[ ] types = new Class<?>[ numElements ];",
+               Spaces);
+  verifyFormat("types[ i ] = arguments[ i ].getClass();", Spaces);
+}
+
 } // namespace format
 } // end namespace clang

@mikir
Copy link
Author

mikir commented Jan 11, 2024

Workaround Add

SpacesInContainerLiterals: false

Thanks a lot for your quick response and especially for this workaround. It helped.

@mydeveloperday
Copy link
Contributor

A fix for this has landed in main and should form part of the 18.X release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior clang-format
Projects
None yet
Development

No branches or pull requests

3 participants