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

Jump to bottom

Use compact-no-array style for @InlineMe annotations #2366

Closed
wants to merge 1 commit into from
Closed

Use compact-no-array style for @InlineMe annotations #2366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion core/src/main/java/com/google/errorprone/bugpatterns/inlineme/InlineMeData.java
View file
Open in desktop
Expand Up @@ -83,7 +83,11 @@ String buildAnnotation() {
}

private static String quote(Set<String> imports) {
return "{\"" + Joiner.on("\", \"").join(imports) + "\"}";
String quoted = "\"" + Joiner.on("\", \"").join(imports) + "\"";
if (imports.size() == 1) {
return quoted;
}
return "{" + quoted + "}";
}

// TODO(glorioso): be tolerant of trailing semicolon
Expand Down
36 changes: 24 additions & 12 deletions core/src/test/java/com/google/errorprone/bugpatterns/inlineme/SuggesterTest.java
View file
Open in desktop
Expand Up @@ -42,6 +42,18 @@ public void testBuildAnnotation_withImports() {
+ "imports = {\"java.time.Duration\", \"java.time.Instant\"})\n");
}

@Test
public void testBuildAnnotation_withSingleImport() {
assertThat(
InlineMeData.buildAnnotation(
"REPLACEMENT",
ImmutableSet.of("java.time.Duration"),
ImmutableSet.of()))
.isEqualTo(
"@InlineMe(replacement = \"REPLACEMENT\", "
+ "imports = \"java.time.Duration\")\n");
}

@Test
public void testInstanceMethodNewImport() {
refactoringTestHelper
Expand All @@ -67,7 +79,7 @@ public void testInstanceMethodNewImport() {
"public final class Client {",
" private Duration deadline = Duration.ofSeconds(5);",
" @InlineMe(replacement = \"this.setDeadline(Duration.ofMillis(millis))\","
+ " imports = {\"java.time.Duration\"})",
+ " imports = \"java.time.Duration\")",
" @Deprecated",
" public void setDeadline(long millis) {",
" setDeadline(Duration.ofMillis(millis));",
Expand Down Expand Up @@ -100,7 +112,7 @@ public void testStaticMethodInNewClass() {
"public final class Client {",
" @InlineMe(",
" replacement = \"Duration.ofMillis(millis)\", ",
" imports = {\"java.time.Duration\"})",
" imports = \"java.time.Duration\")",
" @Deprecated",
" public Duration fromMillis(long millis) {",
" return Duration.ofMillis(millis);",
Expand Down Expand Up @@ -142,7 +154,7 @@ public void testReturnField() {
"import com.google.errorprone.annotations.InlineMe;",
"import java.time.Duration;",
"public final class Client {",
" @InlineMe(replacement = \"Duration.ZERO\", imports = {\"java.time.Duration\"})",
" @InlineMe(replacement = \"Duration.ZERO\", imports = \"java.time.Duration\")",
" @Deprecated",
" public Duration getZero() {",
" return Duration.ZERO;",
Expand Down Expand Up @@ -230,7 +242,7 @@ public void testMethodReference() {
"import java.util.Optional;",
"public final class Client {",
" @InlineMe(replacement = \"input.map(Duration::ofMillis)\", ",
" imports = {\"java.time.Duration\"})",
" imports = \"java.time.Duration\")",
" @Deprecated",
" public Optional<Duration> silly(Optional<Long> input) {",
" return input.map(Duration::ofMillis);",
Expand Down Expand Up @@ -258,7 +270,7 @@ public void testNewClass() {
"import com.google.errorprone.annotations.InlineMe;",
"import org.joda.time.Instant;",
"public final class Client {",
" @InlineMe(replacement = \"new Instant()\", imports = {\"org.joda.time.Instant\"})",
" @InlineMe(replacement = \"new Instant()\", imports = \"org.joda.time.Instant\")",
" @Deprecated",
" public Instant silly() {",
" return new Instant();",
Expand Down Expand Up @@ -286,7 +298,7 @@ public void testNewArray() {
"import com.google.errorprone.annotations.InlineMe;",
"import org.joda.time.Instant;",
"public final class Client {",
" @InlineMe(replacement = \"new Instant[42]\", imports = {\"org.joda.time.Instant\"})",
" @InlineMe(replacement = \"new Instant[42]\", imports = \"org.joda.time.Instant\")",
" @Deprecated",
" public Instant[] silly() {",
" return new Instant[42];",
Expand Down Expand Up @@ -314,7 +326,7 @@ public void testNewNestedClass() {
"import com.google.errorprone.annotations.InlineMe;",
"public final class Client {",
" @InlineMe(replacement = \"new NestedClass()\", ",
" imports = {\"com.google.frobber.Client.NestedClass\"})",
" imports = \"com.google.frobber.Client.NestedClass\")",
" @Deprecated",
" public NestedClass silly() {",
" return new NestedClass();",
Expand Down Expand Up @@ -499,7 +511,7 @@ public void testWithCast() {
"import java.time.Duration;",
"public final class Client {",
" @InlineMe(replacement = \"this.foo((Duration) duration)\", imports ="
+ " {\"java.time.Duration\"})",
+ " \"java.time.Duration\")",
" @Deprecated",
" public void setDuration(Object duration) {",
" foo((Duration) duration);",
Expand Down Expand Up @@ -626,7 +638,7 @@ public void testTernaryOverMultipleLines() {
"public final class Client {",
" @InlineMe(replacement = \""
+ "deadline.compareTo(Duration.ZERO) > 0 ? deadline : Duration.ZERO\", ",
"imports = {\"java.time.Duration\"})",
"imports = \"java.time.Duration\")",
" @Deprecated",
" public Duration getDeadline(Duration deadline) {",
" return deadline.compareTo(Duration.ZERO) > 0",
Expand Down Expand Up @@ -660,7 +672,7 @@ public void testStaticCallingAnotherQualifiedStatic() {
"import java.time.Duration;",
"public final class Client {",
" @InlineMe(replacement = \"Client.getDeadline2()\", ",
" imports = {\"com.google.frobber.Client\"})",
" imports = \"com.google.frobber.Client\")",
" @Deprecated",
" public static Duration getDeadline() {",
" return Client.getDeadline2();",
Expand Down Expand Up @@ -692,7 +704,7 @@ public void staticReferenceToJavaLang() {
"import com.google.errorprone.annotations.InlineMe;",
"public final class Client {",
" @InlineMe(replacement = \"format(template, arg)\", staticImports ="
+ " {\"java.lang.String.format\"})",
+ " \"java.lang.String.format\")",
" @Deprecated",
" public static String myFormat(String template, String arg) {",
" return format(template, arg);",
Expand Down Expand Up @@ -723,7 +735,7 @@ public void replacementContainsGenericInvocation() {
"import java.util.List;",
"public final class Client {",
" @InlineMe(replacement = \"new ArrayList<Void>()\", imports ="
+ " {\"java.util.ArrayList\"})",
+ " \"java.util.ArrayList\")",
" @Deprecated",
" public static List<Void> newArrayList() {",
" return new ArrayList<Void>();",
Expand Down