eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
164 stars 130 forks source link

[formatter] Format License header in module-info.java like in every other java-class #2127

Closed enexusde closed 7 months ago

enexusde commented 7 months ago

The formatter have a bug. The formatter should not format a multiline comment before any LLOC like in every other java-class.

Steps to reproduce:

  1. Create a new workspace
  2. Create a java-project "Test".
  3. Switch to java-development-perspective.
  4. Add a package "test"
  5. Add a class "Test".
  6. Add the JPMS file module-info.java

Add this this signature to both files:

/*******************************************************************************
 * Copyright (c) 2005, 2007 BEA Systems, Inc.
 *
 * This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License 2.0
 * which accompanies this distribution, and is available at
 * https://www.eclipse.org/legal/epl-2.0/
 *
 * SPDX-License-Identifier: EPL-2.0
 *
 *******************************************************************************/
  1. Format both files.

Expectation

The signatures are same.

Problem

The module-info.java is formatted different (3 lines instead of 4)

/*******************************************************************************
 * Copyright (c) 2005, 2007 BEA Systems, Inc.
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License 2.0 which accompanies this distribution,
 * and is available at https://www.eclipse.org/legal/epl-2.0/
 *
 * SPDX-License-Identifier: EPL-2.0
 *
 *******************************************************************************/
module Test {
}
jukzi commented 7 months ago

I understand you do not want your license headers to be modified. However i don't know if there is way to detect those and to treat them differently. So i doubt its a bug but a new feature you want. Can you suggest a code change?

enexusde commented 7 months ago

I think you missed the point. Have you noticed the Test.java retain the format correctly but the module-info.java breaks the format?

So what I need is not to threat them differently, its the opposite way: I need to threat them the same!

jukzi commented 7 months ago

So what I need is not to threat them differently, its the opposite way: I need to threat them the same!

please suggest a patch for that

enexusde commented 7 months ago

Suboptimal, I am not into the code.

enexusde commented 7 months ago

I think it is not an enhancement anyway as it should behave like in typical java-classes.

enexusde commented 7 months ago

Please someone talk to jukzi

jukzi commented 7 months ago

It does not matter much if it is a bug or enhancement. As long as nobody provides a PR it will stay unchanged. Please notice that JDT is open source comes without warranty and the project has not enough resources to fulfill all wishes.

enexusde commented 7 months ago

Bugs werden eher gefixt. Dadurch dass man diesen Bug eine Erweiterung nennt sinkt die Dringlichkeit und dieser Bug wird vermutlich nicht gefixt. Es gab schon viele Verbesserungsforschläge die nie angefasst wurden. Das soll das ziel sein? Bugs nennen wir Erweiterungen damit wir die nicht fixen müssen? Das ist doch keine technische Entscheidung sondern eine politische. Warum sollte ich contributen wenn Entscheidungen politisch motiviert sind?

Wenn ich richtig geonboarded werde dann hätte ich gerne einen PR erstellt.

akurtakov commented 7 months ago

Please keep comments in English.

enexusde commented 7 months ago

Sorry, it was more like a private message to jukzi.

iloveeclipse commented 7 months ago

@mateusz-matela : could you please check if something in formatter missing to work with module-info.java files? Any special rule one has to use?

mateusz-matela commented 7 months ago

Sure, it looks like the comment in Test.java is not touched because the "Enable header comment formatting" setting is not checked. The formatter determines which parts of code is a header in DefaultCodeFormatter.findHeader() method. Apparently this method has been prepared to work with module-info.java with https://github.com/eclipse-jdt/eclipse.jdt.core/commit/4a2995e55bb490ddda70decb3fa4c48aaec59c94

If I read it right, the header is considered to end where the first type declaration begins or if there aren't any (as is the case in module-info), at the package definition. @enexusde where do you put your comment in relation to the package definition? Is the formatting retained if you move it above?

enexusde commented 7 months ago

@mateusz-matela In my case the first character (even the first byte of the first character accordingly) in both files (module-info.java and Test.java) are already part of the signature.

After the signature-lines (my last signature-line ends with a newline accordingly) the package declaration starts. I have a newline but no empty line between the signature and the package-declaration (TWIMC).

This means the signature and the package declaration do not share a same line-number (i.e. if my signature ends in line 15 the package declaration must be in line 16).

By some testing, the signature in Test.java seems not reformated before the class-declaration (including the class-modifiers) start. This means in the import-clause it is not formatted either.

Another suggestion is to use/retain what the code-templates (Preferences -> Java -> Code Style -> Code Templates -> Comments -> Modules) say. I am not sure if this is up for debate.

enexusde commented 7 months ago

Oh by the way, I do not and never worked for BEA Systems. So nothing I ever said or say relates to BEA System.

mateusz-matela commented 7 months ago

OK, turns out the commit I mentioned was about packege-info and I confused it with module-info. I've added a similar fix so it should now work for module-info as well.

enexusde commented 4 months ago

I noticed that the fix works great. Thank!