EmptyNewlineAtEndOfFile should retain non-whitespace characters (#4346

) * EmptyNewlineAtEndOfFile should retain non-whitespace characters As the Groovy parser has a known issue that would be more effort to resolve. * Enable EmptyNewlineAtEndOfFile on PR checks * Annotate the failing disabled test with associated PR * Apply best practices to `noStaticImportOfMethodMatchingInstanceMethod` * Clear out unnecessary whitespace in text blocks * Quick bump to clear out test failures
openrewrite · Jul 22, 2024 · 4568577 · 4568577
1 parent 3de7723
commit 4568577
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 47 deletions.
diff --git a/.github/workflows/receive-pr.yml b/.github/workflows/receive-pr.yml
@@ -28,7 +28,7 @@ jobs:
           - org.openrewrite.recipes.RecipeTestingBestPracticesSubset
           - org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset
           #- org.openrewrite.java.OrderImports
-          #- org.openrewrite.java.format.EmptyNewlineAtEndOfFile
+          - org.openrewrite.java.format.EmptyNewlineAtEndOfFile
           - org.openrewrite.staticanalysis.InlineVariable
           - org.openrewrite.staticanalysis.MissingOverrideAnnotation
           - org.openrewrite.staticanalysis.UseDiamondOperator

diff --git a/...src/test/java/org/openrewrite/gradle/plugins/MigrateGradleEnterpriseToDevelocityTest.java b/...src/test/java/org/openrewrite/gradle/plugins/MigrateGradleEnterpriseToDevelocityTest.java
@@ -61,7 +61,7 @@ void migrate() {
               """,
             """
               plugins {
-                  id 'com.gradle.develocity' version '3.17.5'
+                  id 'com.gradle.develocity' version '3.17.6'
               }
               develocity {
                   server = 'https://ge.sam.com/'
@@ -101,7 +101,7 @@ void publishAlwaysIf() {
               """,
             """
               plugins {
-                  id 'com.gradle.develocity' version '3.17.5'
+                  id 'com.gradle.develocity' version '3.17.6'
               }
               develocity {
                   server = 'https://ge.sam.com/'
@@ -131,7 +131,7 @@ void publishOnFailure() {
               """,
             """
               plugins {
-                  id 'com.gradle.develocity' version '3.17.5'
+                  id 'com.gradle.develocity' version '3.17.6'
               }
               develocity {
                   server = 'https://ge.sam.com/'
@@ -161,7 +161,7 @@ void publishOnFailureIf() {
               """,
             """
               plugins {
-                  id 'com.gradle.develocity' version '3.17.5'
+                  id 'com.gradle.develocity' version '3.17.6'
               }
               develocity {
                   server = 'https://ge.sam.com/'

diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ClassDeclarationTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ClassDeclarationTest.java
@@ -16,12 +16,14 @@
 
 package org.openrewrite.groovy.tree;
 
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.openrewrite.Issue;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaType;
 import org.openrewrite.java.tree.TypeUtils;
 import org.openrewrite.test.RewriteTest;
+import org.openrewrite.test.TypeValidation;
 
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -37,7 +39,7 @@ void multipleClassDeclarationsInOneCompilationUnit() {
             """
               public class A {
                   int n
-                  
+
                   def sum(int m) {
                       n+m
                   }
@@ -128,9 +130,9 @@ void hasPackage() {
           groovy(
             """ 
               package org.openrewrite
-               
+
               public class A{}
-               """
+              """
           )
         );
     }
@@ -168,7 +170,7 @@ void packagePrivate() {
           groovy(
             """
               import groovy.transform.PackageScope
-                            
+
               @PackageScope
               class A {}
               """,
@@ -288,4 +290,21 @@ public final class A {}
           )
         );
     }
+
+    @Issue("https://github.com/openrewrite/rewrite/pull/4346")
+    @Test
+    @Disabled("Known issue; still need to explore a fix")
+    void classExtendsGroovyLangScript() {
+        rewriteRun(
+          spec -> spec.typeValidationOptions(TypeValidation.none()),
+          // Reduced from https://github.com/openrewrite/rewrite/blob/3de7723ba43d8c44d7b524273ad7548d4fcd04eb/rewrite-gradle/src/main/groovy/RewriteSettings.groovy#L32
+          // "Source file was parsed into an LST that contains non-whitespace characters in its whitespace. This is indicative of a bug in the parser."
+          groovy(
+            """
+              class RewriteSettings extends groovy.lang.Script {
+              }
+              """
+          )
+        );
+    }
 }
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/UseStaticImportTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/UseStaticImportTest.java
@@ -31,7 +31,7 @@ void replaceWithStaticImports() {
           java(
             """
               package asserts;
-              
+
               public class Assert {
                   public static void assertTrue(boolean b) {}
                   public static void assertFalse(boolean b) {}
@@ -42,9 +42,9 @@ public static void assertEquals(int m, int n) {}
           java(
             """
               package test;
-             
+
               import asserts.Assert;
-              
+
               class Test {
                   void test() {
                       Assert.assertTrue(true);
@@ -55,9 +55,9 @@ void test() {
               """,
             """
               package test;
-              
+
               import static asserts.Assert.*;
-              
+
               class Test {
                   void test() {
                       assertTrue(true);
@@ -79,7 +79,7 @@ void ignoreMethodsWithTypeParameter() {
             """
               import java.util.Collections;
               import java.util.List;
-              
+
               public class Reproducer {
                   public void methodWithTypeParameter() {
                       List<Object> list = Collections.<Object>emptyList();
@@ -97,15 +97,15 @@ void sameMethodLocallyNoStaticImport() {
           java(
             """
               package com.helloworld;
-              
+
               import java.util.Collections;
               import java.util.List;
-              
+
               public class SameMethodNameLocally {
                   public void avoidCollision() {
                       List<Object> list = Collections.emptyList();
                   }
-              
+
                   private int emptyList(String canHaveDifferentArguments) {
                   }
               }
@@ -122,9 +122,9 @@ void sameMethodImportedNoStaticImport() {
           java(
             """
               package com.helloworld;
-              
+
               import static java.lang.Integer.valueOf;
-              
+
               public class SameMethodNameImported {
                   public void avoidCollision() {
                       String a = String.valueOf("1");
@@ -146,7 +146,7 @@ void sameMethodsNoStaticImport() {
           java(
             """
               package com.helloworld;
-              
+
               public class SameMethodNames {
                   public void avoidCollision() {
                       String a = String.valueOf("1");
@@ -156,9 +156,9 @@ public void avoidCollision() {
               """,
             """
               package com.helloworld;
-              
+
               import static java.lang.String.valueOf;
-              
+
               public class SameMethodNames {
                   public void avoidCollision() {
                       String a = valueOf("1");
@@ -178,7 +178,7 @@ void doReplaceWhenWildcard() {
             """
               import java.util.Collections;
               import java.util.List;
-              
+
               class SameMethodNameLocally {
                   void avoidCollision() {
                       List<Object> list = Collections.emptyList();
@@ -187,9 +187,9 @@ void avoidCollision() {
               """,
             """
               import java.util.List;
-              
+
               import static java.util.Collections.emptyList;
-              
+
               class SameMethodNameLocally {
                   void avoidCollision() {
                       List<Object> list = emptyList();
@@ -209,10 +209,10 @@ void junit5Assertions() {
           java(
             """
               package org.openrewrite;
-              
+
               import org.junit.jupiter.api.Test;
               import org.junit.jupiter.api.Assertions;
-              
+
               class SampleTest {
                   @Test
                   void sample() {
@@ -222,11 +222,11 @@ void sample() {
               """,
             """
               package org.openrewrite;
-              
+
               import org.junit.jupiter.api.Test;
 
               import static org.junit.jupiter.api.Assertions.assertEquals;
-              
+
               class SampleTest {
                   @Test
                   void sample() {
@@ -247,7 +247,7 @@ void javadocLinkUnchanged() {
             """
               import java.util.Collections;
               import java.util.List;
-              
+
               public class WithJavadoc {
                   /**
                    * This method uses {@link Collections#emptyList()}.
@@ -260,9 +260,9 @@ public void mustNotChangeTheJavadocAbove() {
             """
               import java.util.Collections;
               import java.util.List;
-              
+
               import static java.util.Collections.emptyList;
-              
+
               public class WithJavadoc {
                   /**
                    * This method uses {@link Collections#emptyList()}.
@@ -294,9 +294,9 @@ void reproduce() {
               """,
             """
               import java.util.function.Predicate;
-              
+
               import static java.util.function.Predicate.not;
-              
+
               public class Reproducer {
                   void reproduce() {
                       Predicate<Object> predicate = x -> false;
@@ -333,15 +333,17 @@ void noStaticImportOfMethodMatchingInstanceMethod() {
         // Cannot do a static import of Arrays.toString() because it is ambiguous with Object.toString()
         rewriteRun(
           spec -> spec.recipe(new UseStaticImport("java..* *(..)")),
-          java("""
-            import java.util.Arrays;
-            
-            class A {
-                String s(String[] strings) {
-                    return Arrays.toString(strings);
-                }
-            }
+          java(
             """
-          ));
+              import java.util.Arrays;
+
+              class A {
+                  String s(String[] strings) {
+                      return Arrays.toString(strings);
+                  }
+              }
+              """
+          )
+        );
     }
 }
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFileTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFileTest.java
@@ -49,7 +49,6 @@ private static Consumer<RecipeSpec> generalFormat(@Nullable Boolean useCRLF) {
         };
     }
 
-
     @Issue("https://github.com/openrewrite/rewrite/issues/1045")
     @Test
     void usesCRLF() {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFile.java b/rewrite-java/src/main/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFile.java
@@ -17,6 +17,7 @@
 
 import org.openrewrite.*;
 import org.openrewrite.internal.ListUtils;
+import org.openrewrite.internal.StringUtils;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.tree.Comment;
@@ -61,14 +62,15 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             public J visit(@Nullable Tree tree, ExecutionContext ctx) {
                 if (tree instanceof JavaSourceFile) {
                     JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
-                    GeneralFormatStyle generalFormatStyle = ((SourceFile) cu).getStyle(GeneralFormatStyle.class);
+                    GeneralFormatStyle generalFormatStyle = cu.getStyle(GeneralFormatStyle.class);
                     if (generalFormatStyle == null) {
                         generalFormatStyle = autodetectGeneralFormatStyle(cu);
                     }
                     String lineEnding = generalFormatStyle.isUseCRLFNewLines() ? "\r\n" : "\n";
 
                     Space eof = cu.getEof();
-                    if (eof.getLastWhitespace().chars().filter(c -> c == '\n').count() != 1) {
+                    if (StringUtils.isBlank(eof.getLastWhitespace()) &&
+                        eof.getLastWhitespace().chars().filter(c -> c == '\n').count() != 1) {
                         if (eof.getComments().isEmpty()) {
                             return cu.withEof(Space.format(lineEnding));
                         } else {
-Original file line number
+Diff line change
@@ Expand Up @@
             };
         }
         @Issue("https://github.com/openrewrite/rewrite/issues/1045")
         @Test
         void usesCRLF() {
@@ Expand Down @@