GNU bug report logs - #75154
31.0.50; java-ts-mode. Issues with Indentation

Previous Next

Package: emacs;

Reported by: Artem <snake05865 <at> gmail.com>

Date: Sat, 28 Dec 2024 04:38:02 UTC

Severity: minor

Found in version 31.0.50

Full log


View this message in rfc822 format

From: Artem <snake05865 <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 75154 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: bug#75154: 31.0.50; java-ts-mode. Issues with Indentation
Date: Tue, 28 Jan 2025 17:06:33 +0300
Okay, you may personally disagree. But why not create a separate
setting tailored to your preferences? You seem to agree that IntelliJ IDEA
is currently the canonical editor for Java. Set 8 spaces as the default and
add an option to adjust it.

Do I need to provide examples of major open-source projects that use 8 spaces?
For instance, Apache Tomcat:
https://github.com/apache/tomcat/blob/cd997770105ea960764c3d7844e13c0bbe8fd3c1/java/org/apache/juli/OneLineFormatter.java#L102
Or Red Hat Quarkus:
https://github.com/quarkusio/quarkus/blob/0e013fabfc14557e688b559f6edea43b8934406a/devtools/cli/src/main/java/io/quarkus/cli/CliPlugins.java#L37

I think this is a very important matter. Personally,
it drives me crazy when the spacing is off.

> While this is probably catchable, the "workaround" I started using is to
> end the statement with a semi, then RET:
>
> ```
>  class Foo {
>      void Foo() {
>          List<Customer> customers = customer
>              |; // Point is now at |
>          
>      }
>  }
> ```
>
> Now we have a valid parse tree, indented correctly, and can continue
> chaining. I agree that this very much is a hack.

Yes, it does look unnatural. And you know, there’s a similar issue in bash-ts-mode.
I can’t show the exact code snippet right now, but the behavior is essentially the same.
You press RET, the cursor ends up in the wrong position, then press RET again,
and the code block jumps to where it’s supposed to be.
Where’s the error? Is it a problem with the mode’s implementation
or a bug in Tree-sitter itself? Or did everyone just borrow logic from
each other, and now this indentation behavior has spread to other modes?

>
>> Moreover, the current indentation is hardcoded and doesn’t allow
>> flexibility. For instance, in python-ts-mode, pressing TAB allows you to
>> adjust constructions more freely.
>> This level of flexibility would be beneficial in this context
>> 
>> This inflexibility prevents writing common patterns like:
>>     @Override
>>     protected void configure(HttpSecurity http) throws Exception {
>>         http
>>                 .authorizeRequests()
>>                 .antMatchers("/admin/**").hasAuthority("ADMIN")
>>                 .antMatchers("/user").hasAnyAuthority("USER", "ADMIN")
>>                 .antMatchers("/", "/index").permitAll()
>> Example 2:
>> 
>> The following looks correct -
>> 
>> public class FloodFill {
>>     public static void main(String[] args) {
>>         List<Foo> stream = students.stream()
>>                 .filter(item -> {
>>                     return item.getValue() > 100 &&
>>                            item.isActive();
>>                 })
>>                 .map()
>>                 .collect();
>>     }
>> }
>> 
>> But java-ts-mode produces:
>> 
>> public class FloodFill {
>>     public static void main(String[] args) {
>>         List<Foo> stream = students.stream()
>>             .filter(item -> {
>>             return item.getValue() > 100 &&
>>                    item.isActive();
>>         })
>
> Yes, this really annoys me too, so I should try to fix this. I don't
> remember from the top of my head what the problem was, but it forced me
> either to omit the curlies, or just extract the body into a function and
> pass it. But we should absolutely fix this. I believe there were several
> difficulties here, where some are related to incomplete statements, and
> some is related to treesit.el and the java parser idiosyncrasies. But
> this is a valid bug in an of itself. If not too much of a hassle, could
> you create a separate bugreport just for this case?
>
>>             .map()
>>             .collect();
>>     }
>> }

I’m not sure. Maybe I can create a new bug report? But honestly,
I don’t understand the point. A new report just to track the resolution
of some minor issue that annoys you and that you consider more important than others?

>> 
>> 2) Inner Classes
>> Example 1:
>> public class Outer {
>> class Inner {| <-- cursor here moves Inner class unexpectedly
>> 
>>     }
>> }
>> Example 2:
>> public class Outer {
>>     class Inner { // ???
>>         | <-- cursor here.           
>>     }
>> }
>> 
>> Why does this happen? I did not request this behavior. While Example 1
>> demonstrates bad code style, it is technically valid. Such "magical"
>> formatting should be handled by a Java formatter, not by Emacs or
>> Tree-sitter rules.
>> IntelliJ IDEA does not apply such formatting; it leaves this task to the formatter.
>>
>
> This is due to electric indent. You could disable it yourself in your
> config or just live with it.

Yes, I’ve already figured that out, thank you. But what does it give me? Am I supposed
to just live with it? By the way, I tried disabling electric-indent-mode, but then indentation
stopped working entirely. I briefly went through the documentation to understand what’s going on,
and from what I gathered, this mode uses various backends, including Tree-sitter now.

I could be wrong, but it seems like electric-indent-mode needs a review and a rethinking
of how it should work, considering Tree-sitter is being actively used.

And the extra “magic” performed by electric-indent-mode might have been relevant a long
time ago, or maybe it still is in some cases. But for Java/C/C++, I don’t see why
it’s necessary. It’s just annoying and gets in the way.

>> Example 3:
>> public class Outer {
>> class Inner{
>>     void foo(){
>>     }|<--start position. RET
>>     |<-- expected position
>>        |<-- actual
>>     }
>> }
>> 
>> If Inner class has incorrect indentation, subsequent code will also be incorrectly indented.
>>
>
> I would rather inverse the statement, and say that subsequent code is
> correctly indented, but the preceding code is ignored. As the correct
> code is
> ```
> public class Outer {
>     class Inner{
>         void foo(){
>         }
>         
>     }
> }
> ```
>
> Indentation at column 8 is expected here, IMO. I'm not sure we should
> try to work around clearly "incorrectly" indented code.

In my understanding, and in IntelliJ IDEA’s, it should work so that
it doesn’t matter what syntactic construct, method, or anything else is far
behind—several levels above, to be exact. Tree-sitter should only care
about what’s one step behind you. It shouldn’t matter to you that the Inner
class is misplaced. Your method foo() should focus on where it belongs, even
if the Inner class has incorrect indentation. This isn’t a hack. It feels
natural—just follow what’s next to you.  
Right now, it’s just a tightly coupled construct where breaking one
thing breaks another—or everything altogether.  

>
>> 
>> 4) Java 15 text blocks
>> Text blocks are not properly handled
>> 
>> public class TextBlocks {
>>     System.out.println(ctx.fetch("""
>>         SELECT table_schema, count(*)
>>         FROM information_schema.tables
>>         """));
>> }
>
> This isn't valid java is it? I changed it to 
>

Yes, this is invalid code. Again, this touches on how indentation works.
The machine and the code should think like me:
"You see I’ve placed triple quotes? You see there’s a method earlier?
Guess what I’m about to write? Of course, it’s going to be a TEXT BLOCK! Bingo!"
There’s nothing else I could possibly write there.

> ```
> public class TextBlocks {
>     public void foo() {
>         System.out.println(ctx.fetch("""
>             SELECT table_schema, count(*)
>             FROM information_schema.tables
>         """));
>         
>     }    
> }
> ```

Even if I write this code correctly, I still don't have the proper indentation.
Did you configure anything additional on your end? Out of the box, this doesn't work for me

public class TextBlocks {
    void foo() {
        System.out.println(ctx.fetch(""" <-- 1. press RET 
| <-- and you will be here (Actual) 
            | <-- Expected
            """));
    }
}

java-mode, unlike java-ts-mode, is a bit smarter in this regard - the indentation is at least
somewhat recognized, but it still ends up in the wrong place. Manual adjustment of rules is required.

> I usually indent stuff like this marking the region then using C-x i
> then <left> or <right>
What is C-x i? For me it's insert-file.
>
>> - New SQL expressions should be sticky
> What do you mean here? Align the newlines to the previous line?

I've already forgotten what I meant. But yes, sticky means aligning the new line according
to the previous line. I think I saw this somewhere or maybe
I'm confusing it with Stream API method chains.

>> It seems such multiline strings also do not work well, for example:
>> "'The time has come,' the Walrus said,\n" +
>
> Not sure I understand what you mean here
>

Looks like I said too much here. I apologize. Yes, this works correctly.
For example, such code will make proper indentation
    String sql = "SELECT u.id, u.name, u.email, COUNT(o.id) AS order_count " +
                 "FROM users u " +
                 "LEFT JOIN orders o ON u.id = o.user_id " +

>> 6) Multiple Parameters in Methods
>> Example 1
>> public record StudentRecord(
>>   String firstName, 
>>   String lastName, 
>>   Long studentId, 
>>   String email)
>
> What is wrong here?

There's nothing broken in the code above. But in java-ts-mode it looks like this:

public record StudentRecord(
    String a,
String b, <--Actual (BAD)
    String b <--Expected (Good)     
)

>> Example 2
>> public String filterData(@RequestParam(required = false) String name,
>>                          @RequestParam(required = false) String name,
>>                          @RequestParam(required = false) Integer age
>> )
>> java-ts-mode fails to handle these cases correctly.
>>
>
> How so?
Current behavior in java-ts-mode:
    public String filterData(@RequestParam(required = false) String name,
        @RequestParam(required = false) String name, <--Actual
                             @RequestParam(required = false) <--Expected
 )

You understand, I can't even press TAB to move this over?

>> Desired Fontification (Out of the Box):
>> - Annotations (@Annotations)
>> - Diamond Brackets (<>)
>> - Constants, Static Variables, Enum Variables should be highlighted with
>> distinct colors and optionally italic font
>> - Unused Variables or Classes (Grayed Out)
>
> This feels more like opinions rather than errors, but feature requests
> are always welcome, of course. You could try to add patches for some of
> these that I can review?

These might be my personal wishes. But didn't some of this work out of the box in java-mode?
Am I right about this? Why did this disappear in java-ts-mode?
Actually, I wouldn't worry about this if I could simply press M-x customize and select
the needed colors. But... For some reason it's not that simple. In general,
custom color highlighting setup isn't very oriented towards people without Elisp knowledge.
Doesn't Emacs philosophy assume that many settings should be done this way
and all of this is then saved in custom.el?

>> - Unused variables, unused classes, etc., highlighted in gray. Not sure if this can be achieved
>> with Tree-sitter. Anyway, with Flymake + Eglot, it currently works in a somewhat clunky manner.
>>
>
> This isn't possible with tree sitter. How is eglot clunky here? I'm kind
> of satisfied, at least when the lsp actually marks these.
>
>> Example
>> public class TextBlocks {
>>     enum AnEnum { CONST1, CONST2 } <-- No effect for unused AnEnum.
>>     public static final String HELLO ="HELLO";
>> 
>>     public static void main(String[] args) {
>>         int i = 0; <-- Flymake identifies as unused but looks unpolished. 
>>         System.out.println(HELLO);
>>     }
>> }
>>
>
> This isn't treesit related, but could be a report for flymake
> maintainers. We need to get the information from the server, though.

You know what I thought about? I thought that since treesitter
builds a syntax tree of the file, it should be able to know what
in this tree has no usage and isn't connected to anything.

>> Overall:
>> 
>> I may have missed some aspects, but as it stands, Emacs is not
>> comfortable for Java development with these issues.
>>
>
> Can you provide some sort of priority here? Most feel cosmetic, but
> there are some real bugs here. One issue I also want, but is not quite
> sure how to fix yet is the inline sql. We could try to do some multi
> mode sql syntax highlighting here, possibly.

What priority for each problem? For me, everything is priority.
Maybe for someone who understands Elisp well, some things might be less urgent.
Very very very many people don't write anything in the bug tracker.
Because many things can really look like the user just needs to open
the Emacs Lisp references manual and do it themselves, rather than asking someone to do it for them.

Slightly off-topic....

Actually, I realized long ago that many things in Emacs are made so that users are given
some sandbox and provided opportunities to play in it themselves and customize it to their taste.
But there aren't many things that are really polished and designed to just open and use.
I haven't tried many programming modes. But recently I discovered Racket-mode.
God, how neat, polished, well-thought-out this mode looks and requires almost no additional setup
from the user, and tons of documentation is written.
For Java, which is in the top 3 languages worldwide, there is NOTHING in Emacs.
More precisely, there was something and it's probably gathering dust somewhere deep in Emacs' depths,
something like JDEE or CEDET which 1-2 people use. And nobody talks about this now and nowhere
is it written about. You need to do some research work to understand what was relevant before
and what wasn't.
Do you understand that there isn't even a "run main" button?
This is funny and sad.

> As another data point - I use Emacs for java development as the sole
> emacs user on my team in a sea of Intellij users, and I don't really
> share the view that it isn't comfortable for Java development.

That's cool. I want to be like you and be the only Java developer who works in Emacs,
but reality is quite different for now. There's so much! So much! That needs to be finished.
Well okay, I'm lying, not that much.
I think attaching transient.el + hydra.el + a couple dozen functions and this should
become almost indistinguishable from IDE and maybe even superior in some places.
But this still needs to be done...

>There is
> a quirk here and there, sure, but I'm just as productive as everyone
> else there. For me the biggest issue is the one case you mentioned here
> with nested blocks inside of a chain of methods.
>
>
> I'm not sure we should consider what Intellij does as a factual
> source. Though it is for now the canonical java editor, who knows for
> how long, and it feels time consuming to jump after a moving target.

As soon as there's some interested person who wants to do some refactoring
of everything that was previously done for JAVA in Emacs
and rewrites/writes/adapts it to new realities.
I'll be able to officially declare that Emacs will be the best editor/IDE for Java.
But for now, that person hasn't been born yet. Or maybe it all exists already,
but somewhere in personal repositories on GitHub or somewhere else,
but won't make it into the core of java-ts-mode.

I know you're not the only one who writes Java in Emacs and remains productive.
But... I expressed my thoughts about the situation earlier.
And you understand these are thoughts of an ordinary person, not an Emacs hacker.
If you can do it, it doesn't mean that thousands of others can do exactly the same.




This bug report was last modified 60 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.