From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#75154: 31.0.50; java-ts-mode. Issues with Indentation Date: Sun, 05 Jan 2025 12:29:16 +0100 Message-ID: <87h66dv6r7.fsf@thornhill.no> References: <87ttapdtre.fsf@void> <86pllcurry.fsf@gnu.org> Reply-To: Theodor Thornhill Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="24059"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 75154@debbugs.gnu.org, Artem To: Stefan Kangas , Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Jan 05 12:30:28 2025 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1tUOpv-00066Q-WF for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 05 Jan 2025 12:30:28 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tUOpb-0001dT-8N; Sun, 05 Jan 2025 06:30:07 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tUOpY-0001d5-AV for bug-gnu-emacs@gnu.org; Sun, 05 Jan 2025 06:30:05 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tUOpX-0004zU-Q5 for bug-gnu-emacs@gnu.org; Sun, 05 Jan 2025 06:30:04 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=mkzc2t0Y9UNkRoIaibihM2D+jADTezR9vNr3w72G1/k=; b=ZdbwHORqnvYOgmvEB8ubyhT72JqA5dGLoGk3D6yKtbrS0yNfAHF7wWQffHX6LhwWG7pftnEDs+KRFBSqO8EDosfPClwSv9ti9Eyrg9MI4V6MmBR3uOU8MOLAI0po0kwY/+GAoEMpROOYn03PDzgOHeupmuNnLOKJSlPTkY34SSF7iGIfe/S9gk+EalUWYlsFpWZq6kB/n8o5CYcAa80mQN8VPhx+Pn192a2GDUVlyIlCcfFkDdbGAY19Voixw4TQ7SqzShXMyElX6qgMX8urXVw7ZO4Tr4uGDM0QxFT2suFMCPvnYjmU9i7RylLGc1BhWGwUMUQ6qfqE6hnOIydRIQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tUOpX-0004zk-IW for bug-gnu-emacs@gnu.org; Sun, 05 Jan 2025 06:30:03 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Theodor Thornhill Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 05 Jan 2025 11:30:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75154 X-GNU-PR-Package: emacs Original-Received: via spool by 75154-submit@debbugs.gnu.org id=B75154.173607657119102 (code B ref 75154); Sun, 05 Jan 2025 11:30:03 +0000 Original-Received: (at 75154) by debbugs.gnu.org; 5 Jan 2025 11:29:31 +0000 Original-Received: from localhost ([127.0.0.1]:60148 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tUOp0-0004y1-Lu for submit@debbugs.gnu.org; Sun, 05 Jan 2025 06:29:31 -0500 Original-Received: from mx.kolabnow.com ([212.103.80.154]:45536) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tUOoy-0004xl-9m for 75154@debbugs.gnu.org; Sun, 05 Jan 2025 06:29:29 -0500 Original-Received: from localhost (unknown [127.0.0.1]) by mx.kolabnow.com (Postfix) with ESMTP id 4A2E4209201D; Sun, 5 Jan 2025 12:29:22 +0100 (CET) Authentication-Results: ext-mx-out011.mykolab.com (amavis); dkim=pass (2048-bit key) reason="pass (just generated, assumed good)" header.d=kolabnow.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kolabnow.com; h= content-transfer-encoding:content-type:content-type:mime-version :message-id:date:date:references:in-reply-to:subject:subject :from:from:received:received:received; s=dkim20240523; t= 1736076560; x=1737890961; bh=mkzc2t0Y9UNkRoIaibihM2D+jADTezR9vNr 3w72G1/k=; b=Z4rqyUwbeRqdR63YxTMT8PjXGVhBgVJRnSzQf1G5DKCVxrLY24R 9fNbXE4acIKmTwrzdz8F0pgUg5x1nkde8uRF9PwwgISTrhNFwazui3hHWlLzXpHD HUuVIvomhMV/on91oKd0VWxHZHJa3WrCm54LCTD4iEoc1BlDNr3oBoUyes3Ip/SR G/Xs13/VGuNKNEIkWYnDKJuDqFsG3lPl89w0Hy8tyY6gpZjxoVokhDGvfBsRzNGW a7t8oWOYAIGvM+1jcOQQQ/SAcBOHD/8AWKP2Rv0TQe5iKcUX/X0zTzSijkrYwIDT RLBdJjQZe+SqgdfnXD7kkdeNr0VgwqRwmJQ== X-Virus-Scanned: amavis at mykolab.com Original-Received: from mx.kolabnow.com ([127.0.0.1]) by localhost (ext-mx-out011.mykolab.com [127.0.0.1]) (amavis, port 10024) with ESMTP id RdIbjTYF4ZaA; Sun, 5 Jan 2025 12:29:20 +0100 (CET) Original-Received: from int-mx011.mykolab.com (unknown [10.9.13.11]) by mx.kolabnow.com (Postfix) with ESMTPS id 76C1E2092013; Sun, 5 Jan 2025 12:29:18 +0100 (CET) Original-Received: from ext-subm010.mykolab.com (unknown [10.9.6.10]) by int-mx011.mykolab.com (Postfix) with ESMTPS id 3CF1D313E1FE; Sun, 5 Jan 2025 12:29:18 +0100 (CET) In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:298542 Archived-At: Hello, Artem! >=20 > I've identified several issues with indentation in java-ts-mode where it > doesn't work as expected. Let me first clarify > what I consider "expected" behavior - there are things that are "GOOD", > "BAD but allowed", and "Opinion-based. > Thanks for taking the time for this. > Here are the specific problems: >=20 > 1) Chaining Methods in the Stream API and Lambda Expressions > Example 1: > class Foo { > void Foo() { > List customers =3D customer > | <-- Actual (BAD) > | <-- Expected (GOOD) (8 spaces) > } > } > When continuing the statement > List customers =3D customer > .stream > and then adding closing parentheses=20=20=20=20=20=20=20=20 > List customers =3D customer > .stream() <-- The method filter will move to 4 spaces automati= cally > .filter <-- without parentheses=20 > .filter() <-- closing bracet and method moving to 4 spaces >=20 > This behavior is problematic. In java-mode, this does not occur.IntelliJ > IDEA uses 8 spaces for method chains,which makes the code more readable. > While some examples use 2 spaces (e.g., for web snippets),in production > environments, 8 spaces are more common. This should ideally be customizab= le for end users. > Yes, while I do agree, this is kind of expected. Let me explain: class Foo { void Foo() { List customers =3D customer | <-- Actual (BAD) | <-- Expected (GOOD) (8 spaces) } } This case boils down to two things. Personally I disagree with intellij and the 8 indent case, but that is a stylistic thing, and not very interesting, compared to the other thing. IIRC when I implemented this there are some ambiguities when we get these unfinished statements. Given your snippet here the parse tree is: ``` (program (class_declaration class name: (identifier) body:=20 (class_body { (method_declaration type: (void_type) name: (identifier) parameters: (formal_parameters ( )) body:=20 (block { (local_variable_declaration type:=20 (generic_type (type_identifier) (type_arguments < (type_identifier) >)) declarator: (variable_declarator name: (identifier) =3D value: (ide= ntifier)) type: ;) })) }))) ``` The interesting thing here is the `declarator` line, that ends in identifier. That identifier is the symol `customer`. To know that we have to indent here isn't really easy, at least not without interfering with other constructs. The reason is that while you know that you want to continue the chain, the parser actually ERRORs: Given this code, class Foo { void Foo() { List customers =3D customer . =20=20=20=20=20=20=20=20=20 } } the parse tree is (program (ERROR class (identifier) { type: (void_type) name: (identifier) parameters: (formal_parameters ( )) { (generic_type (type_identifier) (type_arguments < (type_identifier) >)) name: (identifier) =3D (identifier) . } })) 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 customers =3D customer |; // Point is now at | =20=20=20=20=20=20=20=20=20 } } ``` Now we have a valid parse tree, indented correctly, and can continue chaining. I agree that this very much is a hack, but frankly I just started doing that and forgot about it. We could try to implement some acrobatics to fix this, but I'm too limited for time right now to prioritize this one in particular. Feel free to explore a patch that I can review! > Moreover, the current indentation is hardcoded and doesn=E2=80=99t 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 >=20 > 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: >=20 > The following looks correct - >=20 > public class FloodFill { > public static void main(String[] args) { > List stream =3D students.stream() > .filter(item -> { > return item.getValue() > 100 && > item.isActive(); > }) > .map() > .collect(); > } > } >=20 > But java-ts-mode produces: >=20 > public class FloodFill { > public static void main(String[] args) { > List stream =3D 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(); > } > } >=20 > 2) Inner Classes > Example 1: > public class Outer { > class Inner {| <-- cursor here moves Inner class unexpectedly >=20 > } > } > Example 2: > public class Outer { > class Inner { // ??? > | <-- cursor here.=20=20=20=20=20=20=20=20=20=20=20 > } > } >=20 > 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. Emacs is by convention quite heavy handed in trying to keep a consistent indent style, almost to the order of acting like a formatter, rather than a simple indent offset calculator. I also was confused when I started using Emacs, but now I actually like this, as it feels more deterministic than other editors, for better or worse. > Example 3: > public class Outer { > class Inner{ > void foo(){ > }|<--start position. RET > |<-- expected position > |<-- actual > } > } >=20 > If Inner class has incorrect indentation, subsequent code will also be in= correctly 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(){ } =20=20=20=20=20=20=20=20 } } ``` Indentation at column 8 is expected here, IMO. I'm not sure we should try to work around clearly "incorrectly" indented code. > 3) for, if, else if, while, do-while without braces > public class While { > { > while () > | <-- Expected=20=20=20=20=20=20 > | <-- Actual > } > } > Although this is bad coding style, it=E2=80=99s allowed and compiles > correctly. This looks like several issues to me: 1. The block isn't handled correctly, as in it misses a rule. Line two should be indented 4 spaces, but is indented 0 spaces on my system, right? 2. The parse tree returns an ERROR, so I think this likely is a parser bug rather than emacs bug to begin with. Though I believe the case should be handled when the parser supports this. ``` (program (class_declaration (modifiers public) class name: (identifier) body:=20 (class_body { (block { (ERROR while ( )) }) }))) ``` Could you supply a separate bug report for this one? >=20 > 4) Java 15 text blocks > Text blocks are not properly handled >=20 > 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=20 ``` public class TextBlocks { public void foo() { System.out.println(ctx.fetch(""" SELECT table_schema, count(*) FROM information_schema.tables """)); =20=20=20=20=20=20=20=20 }=20=20=20=20 } ``` >=20 > - Triple quotes handling (should electric-pair-mode be enhanced?) Yes, I agree. > - Text block alignment is opinion-based and should be adjustable with > TAB I usually indent stuff like this marking the region then using C-x i then or > - New SQL expressions should be sticky What do you mean here? Align the newlines to the previous line? > 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 >=20 >=20 > 5) Broken Syntax Highlighting >=20 > public class Outer { > HELLO EMACS <-- Write something here > class Inner{ <-- This class will not be highlighted > void foo(){ > } >=20 > } > } > Tree-sitter should ignore such uncommon cases to maintain syntax highligh= ting This I believe is out of scope, but could be an issue for tree-sitter upstream. We have to deal with the parse tree we get. How does neovim or other editors handle this? >=20 > 6) Multiple Parameters in Methods > Example 1 > public record StudentRecord( > String firstName,=20 > String lastName,=20 > Long studentId,=20 > String email) What is wrong here? > Example 2 > public String filterData(@RequestParam(required =3D false) String name, > @RequestParam(required =3D false) String name, > @RequestParam(required =3D false) Integer age > ) > java-ts-mode fails to handle these cases correctly. > How so? > 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? > - Unused variables, unused classes, etc., highlighted in gray. Not sure i= f this can be achieved > with Tree-sitter. Anyway, with Flymake + Eglot, it currently works in a s= omewhat 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 =3D"HELLO"; >=20 > public static void main(String[] args) { > int i =3D 0; <-- Flymake identifies as unused but looks unpolishe= d.=20 > 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. > Overall: >=20 > 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. 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. 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. > Some information that might be helpful: >=20 > - https://github.com/dakrone/eos/blob/dd8aa3a25b496397dd0162d229de5719896= 68619/eos-java.org?plain=3D1#L30 > .Not sure why this Elasticsearch developer created so many custom rules f= or indentation. > - https://github.com/Michael-Allan/Java_Mode_Tamed A major Java mode > with sensible fontification. This is c mode related, so not easy to relate to the treesit mode. But I wasn't aware of this > - JetBrains Intellij IDEA Community Edition > - File>Settings>Editor>Color Scheme>Java Java for understanding which c= olors are needed and what is missing. > - Editor>Color Scheme>Java>Code Style>Java for indentation settings. > 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. Hope some of these comments are helpful, feel free to either provide patches or disagree :-) Theo