Fix for #1391: place implicit import(s) after package or explicit import

groovy · Sep 19, 2022 · df3d0a8 · df3d0a8
1 parent 84b1e79
commit df3d0a8
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 16 deletions.
diff --git a/....groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/ImportsTests.java b/....groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/ImportsTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -775,13 +775,24 @@ public void testExtraImports1() {
 
             "com/foo/Type.groovy",
             "package com.foo\n" +
+            "import java.lang.Object\n" +
             "class Type {\n" +
             "  public static void m() {}\n" +
             "}\n",
         };
         //@formatter:on
 
         runConformTest(sources, "done", options);
+
+        // implicit import placed at end of package statement
+        ImportReference ref = getCUDeclFor("Runner.groovy").imports[0];
+        assertEquals(15, ref.sourceStart);
+        assertEquals(14, ref.sourceEnd);
+
+        // implicit import placed at end of explicit import
+        ref = getCUDeclFor("Type.groovy").imports[1];
+        assertEquals(39, ref.sourceStart);
+        assertEquals(38, ref.sourceEnd);
     }
 
     @Test

diff --git a/...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java b/...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java
@@ -1094,12 +1094,18 @@ private void createImportDeclarations(ModuleNode moduleNode) {
                 }
 
                 if (!importReferences.isEmpty()) {
+                    ImportReference last = unitDeclaration.currentPackage;
                     ImportReference[] refs = importReferences.values().toArray(unitDeclaration.imports);
                     for (ImportReference ref : refs) {
-                        if (ref.declarationSourceStart > 0 && (ref.declarationEnd - ref.declarationSourceStart + 1) < 0) {
-                            throw new IllegalStateException(String.format(
-                                "Import reference alongside class %s will trigger later failure: %s declSourceStart=%d declEnd=%d",
-                                  moduleNode.getClasses().get(0), ref.toString(), ref.declarationSourceStart, ref.declarationEnd));
+                        if (ref.sourceStart >= 0) {
+                            last = ref;
+                        } else if (last != null) { // place after package/import
+                            ref.declarationSourceStart = last.sourceEnd + 1;
+                            ref.sourceStart = ref.declarationSourceStart;
+                            ref.declarationSourceEnd = last.sourceEnd;
+                            ref.trailingStarPosition = last.sourceEnd;
+                            ref.declarationEnd = last.sourceEnd;
+                            ref.sourceEnd = last.sourceEnd;
                         }
                     }
                     unitDeclaration.imports = refs;
@@ -2685,18 +2691,20 @@ private boolean isVargs(Parameter[] parameters) {
          * Ensures lexical ordering and synthetic de-duplication.
          */
         private static String lexicalKey(ImportReference ref) {
-            StringBuilder key = new StringBuilder();
-            key.append(Prefix.format(ref.declarationSourceStart));
+            StringBuilder key = new StringBuilder(64);
+            int pos = ref.sourceStart;
+            if (pos < 0) pos = 999999;
+            key.append(Prefix.format(pos));
             key.append(CharOperation.concatWith(ref.tokens, '.'));
-            if ((ref.bits & ASTNode.OnDemand) == 0) {
+            if (ref instanceof AliasImportReference) {
                 key.append(" as ").append(ref.getSimpleName());
             }
             return key.toString();
         }
         private static final NumberFormat Prefix;
         static {
             Prefix = NumberFormat.getInstance();
-            Prefix.setMinimumIntegerDigits(5);
+            Prefix.setMinimumIntegerDigits(6);
             Prefix.setGroupingUsed(false);
         }
 

diff --git a/...clipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTestSuite.groovy b/...clipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTestSuite.groovy
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2020 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -146,21 +146,19 @@ abstract class OrganizeImportsTestSuite extends GroovyEclipseTestSuite {
     }
 
     protected void doContentsCompareTest(CharSequence originalContents, CharSequence expectedContents = originalContents) {
-        def unit = addGroovySource(originalContents.stripMargin(), nextUnitName(), 'main')
+        def unit = addGroovySource(originalContents.stripMargin(), nextUnitName())
         ReconcilerUtils.reconcile(unit)
 
         OrganizeGroovyImports organize = new OrganizeGroovyImports(unit, { TypeNameMatch[][] matches, ISourceRange[] range ->
             Assert.fail("Should not have a choice, but found $matches[0][0] and $matches[0][1]")
         })
         TextEdit edit = organize.calculateMissingImports()
-        // NOTE: Must match TestProject.createGroovyType()!
-        String prefix = 'package main;\n\n'
 
-        Document doc = new Document(prefix + originalContents.stripMargin())
+        Document doc = new Document(originalContents.stripMargin())
         if (edit != null) edit.apply(doc)
 
         // deal with some variance in JDT Core handling of package only
-        String expect = prefix + expectedContents.stripMargin()
+        String expect = expectedContents.stripMargin()
         String actual = doc.get()
         if (expectedContents.toString().isEmpty()) {
             expect = expect.trim()

diff --git a/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTests.groovy b/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTests.groovy
@@ -1982,7 +1982,7 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
             |}
             |'''
 
-        createGroovyType 'main', 'One', '''\
+        createGroovyType '', 'One', '''\
             |@Canonical
             |class One {
             |  String string
@@ -2042,6 +2042,24 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
         doContentsCompareTest(contents, contents - ~/\|import static foo.Bar.\*\s+/)
     }
 
+    @Test
+    void testOrganizeWithExtraImports9() {
+        addConfigScript '''\
+            |withConfig(configuration) {
+            |  imports {
+            |    star 'java.time'
+            |  }
+            |}
+            |'''
+
+        String contents = '''\
+            |import groovy.transform.*
+            |/*
+            |*/
+            |'''
+        doContentsCompareTest(contents, contents - ~/\|import groovy.transform.\*\s+/)
+    }
+
     @Test @NotYetImplemented
     void testOrganizeWithInterleavedComments() {
         String originalContents = '''\