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

Plugin + Intellij optimize Import for static imports out of order #200

Open
thespags opened this issue Feb 3, 2020 · 14 comments
Open

Plugin + Intellij optimize Import for static imports out of order #200

thespags opened this issue Feb 3, 2020 · 14 comments

Comments

@thespags
Copy link

thespags commented Feb 3, 2020

If the formating is different than expected:

  • make sure you are comparing with the same Eclipse version
  • provide a code example (both from Eclipse and from IntelliJ) and settings files

Define this class,

package somePackage;

import static somePackage.SomeZClass;
import static somePackage.SomeEnum.Bar;

import java.util.function.Supplier;

// Use as a type argument to be a static import.
public class Foo implements Supplier<SomeZClass> {

    public void someMethod() {
        SomeEnum e = Bar;
    }

    @Override
    public SomeZClass get() {
        return null;
    }

    public enum SomeEnum {
        Bar
    }

    // Add Z this should be sorted after SomeEnum
    public static class SomeZClass {

    }
}

What steps will reproduce the issue?

Run optimize import in the IDE.

What is the expected result?

import static somePackage.SomeEnum.Bar;
import static somePackage.SomeZClass;

What happens instead?

import static somePackage.SomeZClass;
import static somePackage.SomeEnum.Bar;

Paste information about IDE and OS (it can be copied from Help | About dialog).

Intellij Version: 193.6015.39
Using Plugin with Eclipse Format: 4.5.1+

Note: mvn spotless:apply uses the expected behavior, and turning off the eclipse formatter plugin then Intellij will optimize the import with the expected behavior.

@krasa
Copy link
Owner

krasa commented Feb 4, 2020

Hmm, spotless uses my old implementation for import sorting. Are you sure you configured it properly? What is your config for import order?

@thespags
Copy link
Author

thespags commented Feb 4, 2020

My settings are.
4=com.xxx
3=java
2=javax
1=
0=#

My spotless configuration points to my resource/myTeam.importOrder
and the plugin points to the same file for import order.

@krasa
Copy link
Owner

krasa commented Feb 4, 2020

I am starting to remember that there is some kind of hack in sorting static imports....

I think this is not supported:

import static somePackage.SomeZClass;

I don't even know what that is, is this some new language feature? Could you post the whole compilable code?

This would work fine:

import static somePackage.SomeZClass.*;

@thespags
Copy link
Author

thespags commented Feb 4, 2020

Here's what I wrote locally

package somePackage;

import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;

import java.util.function.Supplier;

// Use as a type argument to be a static import.
public class Foo implements Supplier<SomeZClass> {

    public void someMethod() {
        SomeEnum e = Bar;
    }

    @Override
    public SomeZClass get() {
        return null;
    }

    public enum SomeEnum {
        Bar
    }

    // Add Z this should be sorted after SomeEnum
    public static class SomeZClass {

    }
}

Sorry I missed the imports including the top level class. It's suppose to be somePackage.Foo.SomeZClass in the example. So I'm doing static imports of SomeZClass for the type argument and Bar.

@krasa
Copy link
Owner

krasa commented Feb 4, 2020

It is probably simplified too much, Eclipse converts it to this:

import static somePackage.Foo.SomeEnum.Bar;

import java.util.function.Supplier;

import somePackage.Foo.SomeZClass;

@krasa
Copy link
Owner

krasa commented Feb 4, 2020

If I do this:

package somePackage;


import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;


public class X {
	{
		SomeZClass();	
	}
	
	somePackage.Foo.SomeEnum bar=Bar;
}

then Eclipse 2019-03 and 2019-12 sorts SomeZClass as first.

@thespags
Copy link
Author

thespags commented Feb 4, 2020

Just to confirm the expected behavior is that we have

import static somePackage.Foo.SomeEnum.Bar;
import static somePackage.Foo.SomeZClass;

?

@krasa
Copy link
Owner

krasa commented Feb 4, 2020

Have you actually tried it in Eclipse?
because Eclipse produces:

package somePackage;

import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;

public class Bar {
	{
		
		SomeZClass.name();
	}
	
	somePackage.Foo.SomeEnum bar=Bar;
}

package somePackage;

public enum Foo  {
	SomeZClass ;
	
    public enum SomeEnum {
        Bar
    }

}

@thespags
Copy link
Author

thespags commented Feb 5, 2020

Yes I just ran it in eclipse and see the behavior you mention. But the plugin for Intellij has a different behavior than what is in eclipse. And spotless has a third behavior.

So is your goal to align with the behavior as much in eclipse?

Personally I feel the ordering that spotless does is what I'd expect, but I can understand if you want 1-1 parity with eclipse and I can reach out to spotless' organization.

@krasa
Copy link
Owner

krasa commented Feb 5, 2020

But the plugin for Intellij has a different behavior than what is in eclipse.

where?

So is your goal to align with the behavior as much in eclipse?

yes

@thespags
Copy link
Author

thespags commented Feb 5, 2020

For clarity the three behaviors I observe from my given import above

import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;

import java.util.function.Supplier;

Eclipse running "Organize Import" produces:

import static somePackage.Foo.SomeEnum.Bar;

import java.util.function.Supplier;

import somePackage.Foo.SomeZClass;

Eclipse Plugin for Intellij running "Optimize Import" produces:

import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;

import java.util.function.Supplier;

Spotless plugin running "mvn spotless:apply" produces:

import static somePackage.Foo.SomeEnum.Bar;
import static somePackage.Foo.SomeZClass;

import java.util.function.Supplier;

To further show that Eclipse Plugin behavior is not expected we can add


   public void callM() {
       m(); // call m with a static import
   }
    public static class SomeZClass {
        // static method to be imported by Foo
        static void m() {

        }
    }

Eclipse Plugin will order imports:

import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeEnum.Bar;
import static somePackage.Foo.SomeZClass.m;

Eclipse will order imports:

import static somePackage.Foo.SomeEnum.Bar;
import static somePackage.Foo.SomeZClass.m;

import java.util.function.Supplier;

import somePackage.Foo.SomeZClass;

and spotless will order imports


import static somePackage.Foo.SomeEnum.Bar;
import static somePackage.Foo.SomeZClass;
import static somePackage.Foo.SomeZClass.m;

import java.util.function.Supplier;

So I expect the behavior of the Eclipse Plugin to respect alphabetical ordering for static imports, SomeZClass should be imported after SomeEnum.

@krasa
Copy link
Owner

krasa commented Feb 5, 2020

Ok, I see.

The problem is that Intellij does not convert import static somePackage.Foo.SomeZClass; into import somePackage.Foo.SomeZClass; like Eclipse.
This plugin only handles the order, not the actual content of imports.

You could workaround it by fixing the import manually or not writing such code in the first place :) Not sure how you ended up with it, my IntelliJ makes public class Foo implements Supplier<somePackage.Foo.SomeZClass> { and has no intention to convert it to an import.

Spotless is missing this fix: #105 so its order is wrong.

@thespags
Copy link
Author

thespags commented Feb 6, 2020

Thank you for the response.

I statically import SomeZcClass to avoid the FQN in the argument. The other option is if I manually format it like Eclipse than spotless and EclipseFormatter won't change it.

I've filed an issue to Spotless as well to get #105. Is it possible to have the plugin change the contents or is there a limitation that makes this difficult?

@krasa
Copy link
Owner

krasa commented Apr 16, 2020

This plugin could change the imports in the same way IntelliJ does it, using PSI, that should be easy.

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