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.

Sign up for GitHub

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

EmptyNewlineAtEndOfFile should retain non-whitespace characters #4346

Merged
merged 6 commits into from
Jul 22, 2024
Merged

EmptyNewlineAtEndOfFile should retain non-whitespace characters #4346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89c8360
EmptyNewlineAtEndOfFile should retain non-whitespace characters
timtebeek Jul 22, 2024
74e22e1
Enable EmptyNewlineAtEndOfFile on PR checks
timtebeek Jul 22, 2024
974547b
Annotate the failing disabled test with associated PR
timtebeek Jul 22, 2024
a26cda4
Apply best practices to `noStaticImportOfMethodMatchingInstanceMethod`
timtebeek Jul 22, 2024
e7208a5
Clear out unnecessary whitespace in text blocks
timtebeek Jul 22, 2024
c13620f
Quick bump to clear out test failures
timtebeek Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/receive-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions ...src/test/java/org/openrewrite/gradle/plugins/MigrateGradleEnterpriseToDevelocityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/'
Expand Down Expand Up @@ -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/'
Expand Down Expand Up @@ -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/'
Expand Down Expand Up @@ -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/'
Expand Down
27 changes: 23 additions & 4 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ClassDeclarationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,7 +39,7 @@ void multipleClassDeclarationsInOneCompilationUnit() {
"""
public class A {
int n

def sum(int m) {
n+m
}
Expand Down Expand Up @@ -128,9 +130,9 @@ void hasPackage() {
groovy(
"""
package org.openrewrite

public class A{}
"""
"""
)
);
}
Expand Down Expand Up @@ -168,7 +170,7 @@ void packagePrivate() {
groovy(
"""
import groovy.transform.PackageScope

@PackageScope
class A {}
""",
Expand Down Expand Up @@ -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 {
}
"""
)
);
}
}
72 changes: 37 additions & 35 deletions rewrite-java-test/src/test/java/org/openrewrite/java/UseStaticImportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void replaceWithStaticImports() {
java(
"""
package asserts;

public class Assert {
public static void assertTrue(boolean b) {}
public static void assertFalse(boolean b) {}
Expand All @@ -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);
Expand All @@ -55,9 +55,9 @@ void test() {
""",
"""
package test;

import static asserts.Assert.*;

class Test {
void test() {
assertTrue(true);
Expand All @@ -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();
Expand All @@ -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) {
}
}
Expand All @@ -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");
Expand All @@ -146,7 +146,7 @@ void sameMethodsNoStaticImport() {
java(
"""
package com.helloworld;

public class SameMethodNames {
public void avoidCollision() {
String a = String.valueOf("1");
Expand All @@ -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");
Expand All @@ -178,7 +178,7 @@ void doReplaceWhenWildcard() {
"""
import java.util.Collections;
import java.util.List;

class SameMethodNameLocally {
void avoidCollision() {
List<Object> list = Collections.emptyList();
Expand All @@ -187,9 +187,9 @@ void avoidCollision() {
""",
"""
import java.util.List;

import static java.util.Collections.emptyList;

class SameMethodNameLocally {
void avoidCollision() {
List<Object> list = emptyList();
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -247,7 +247,7 @@ void javadocLinkUnchanged() {
"""
import java.util.Collections;
import java.util.List;

public class WithJavadoc {
/**
* This method uses {@link Collections#emptyList()}.
Expand All @@ -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()}.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
"""
)
);
}
}
1 change: 0 additions & 1 deletion rewrite-java-test/src/test/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ private static Consumer<RecipeSpec> generalFormat(@Nullable Boolean useCRLF) {
};
}


@Issue("https://github.com/openrewrite/rewrite/issues/1045")
@Test
void usesCRLF() {
Expand Down
6 changes: 4 additions & 2 deletions rewrite-java/src/main/java/org/openrewrite/java/format/EmptyNewlineAtEndOfFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Loading