From e882355e8ea85d8bea4376dcca2fd2d6bbe98bd2 Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Thu, 21 Jan 2016 13:09:15 +0100 Subject: [PATCH] Static import refactored. --- .../com/squareup/javapoet/CodeWriter.java | 99 +++++++++++-------- .../java/com/squareup/javapoet/JavaFile.java | 9 +- .../javapoet/JavaFileImportStaticTest.java | 76 ++++++++++++++ .../com/squareup/javapoet/JavaFileTest.java | 27 +---- 4 files changed, 144 insertions(+), 67 deletions(-) create mode 100644 src/test/java/com/squareup/javapoet/JavaFileImportStaticTest.java diff --git a/src/main/java/com/squareup/javapoet/CodeWriter.java b/src/main/java/com/squareup/javapoet/CodeWriter.java index 542f43469..f8bd77f44 100644 --- a/src/main/java/com/squareup/javapoet/CodeWriter.java +++ b/src/main/java/com/squareup/javapoet/CodeWriter.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import javax.lang.model.SourceVersion; import javax.lang.model.element.Modifier; @@ -57,7 +58,10 @@ final class CodeWriter { private final Map importedTypes; private final Map importableTypes = new LinkedHashMap<>(); private final Set referencedNames = new LinkedHashSet<>(); + private final Set referencedStaticImports = new TreeSet<>(); private boolean trailingNewline; + private ClassName deferredClassName = null; + private final StringBuilder deferredLine = new StringBuilder(); /** * When emitting a statement, this is the line of the statement currently being written. The first @@ -213,7 +217,6 @@ public CodeWriter emit(String format, Object... args) throws IOException { public CodeWriter emit(CodeBlock codeBlock) throws IOException { int a = 0; - ClassName deferredTypeName = null; // used by "import static" logic ListIterator partIterator = codeBlock.formatParts.listIterator(); while (partIterator.hasNext()) { String part = partIterator.next(); @@ -236,17 +239,6 @@ public CodeWriter emit(CodeBlock codeBlock) throws IOException { case "$T": TypeName typeName = (TypeName) codeBlock.args.get(a++); - // defer "typeName.emit(this)" if next format part will be handled by the default case - if (typeName instanceof ClassName && partIterator.hasNext()) { - if (!codeBlock.formatParts.get(partIterator.nextIndex()).startsWith("$")) { - ClassName candidate = (ClassName) typeName; - if (staticImportClassNames.contains(candidate.canonicalName)) { - checkState(deferredTypeName == null, "pending type for static import?!"); - deferredTypeName = candidate; - break; - } - } - } typeName.emit(this); break; @@ -284,18 +276,6 @@ public CodeWriter emit(CodeBlock codeBlock) throws IOException { break; default: - // handle deferred type - if (deferredTypeName != null) { - if (part.startsWith(".")) { - if (emitStaticImportMember(deferredTypeName.canonicalName, part)) { - // okay, static import hit and all was emitted, so clean-up and jump to next part - deferredTypeName = null; - break; - } - } - deferredTypeName.emit(this); - deferredTypeName = null; - } emitAndIndent(part); break; } @@ -309,7 +289,9 @@ public CodeWriter emitWrappingSpace() throws IOException { } private static String extractMemberName(String part) { - checkArgument(Character.isJavaIdentifierStart(part.charAt(0)), "not an identifier: %s", part); + if (part.isEmpty() || !Character.isJavaIdentifierStart(part.charAt(0))) { + return null; + } for (int i = 1; i <= part.length(); i++) { if (!SourceVersion.isIdentifier(part.substring(0, i))) { return part.substring(0, i - 1); @@ -318,20 +300,6 @@ private static String extractMemberName(String part) { return part; } - private boolean emitStaticImportMember(String canonical, String part) throws IOException { - String partWithoutLeadingDot = part.substring(1); - if (partWithoutLeadingDot.isEmpty()) return false; - char first = partWithoutLeadingDot.charAt(0); - if (!Character.isJavaIdentifierStart(first)) return false; - String explicit = canonical + "." + extractMemberName(partWithoutLeadingDot); - String wildcard = canonical + ".*"; - if (staticImports.contains(explicit) || staticImports.contains(wildcard)) { - emitAndIndent(partWithoutLeadingDot); - return true; - } - return false; - } - private void emitLiteral(Object o) throws IOException { if (o instanceof TypeSpec) { TypeSpec typeSpec = (TypeSpec) o; @@ -353,6 +321,18 @@ private void emitLiteral(Object o) throws IOException { * names visible due to inheritance. */ String lookupName(ClassName className) { + return lookupName(className, true); + } + + private String lookupName(ClassName className, boolean doStaticImports) { + // Is this a start of a static import match? Then defer the class name rendering... + if (doStaticImports) { + if (staticImportClassNames.contains(className.canonicalName)) { + deferredClassName = className; + deferredLine.setLength(0); + return ""; + } + } // Find the shortest suffix of className that resolves to className. This uses both local type // names (so `Entry` in `Map` refers to `Map.Entry`). Also uses imports. boolean nameResolved = false; @@ -473,6 +453,39 @@ CodeWriter emitAndIndent(String s) throws IOException { } } + // handle "import static" hits... + if (deferredClassName != null) { + trailingNewline = false; + deferredLine.append(line); + String condensed = deferredLine.toString().replaceAll("\\s", ""); + if (condensed.isEmpty() || condensed.equals(".")) { + continue; // grab more chars from next line or from next method call + } + String wildcard = deferredClassName.canonicalName + ".*"; + boolean hit = staticImports.contains(wildcard); + if (hit) { + referencedStaticImports.add(wildcard); + } + if (!hit && condensed.startsWith(".")) { + String memberName = extractMemberName(condensed.substring(1)); + if (memberName != null) { + String exact = deferredClassName.canonicalName + "." + memberName; + hit = staticImports.contains(exact); + if (hit) { + referencedStaticImports.add(exact); + } + } + } + if (hit) { + // hit! preserve original whitespaces by _not_ using condensed here + line = deferredLine.toString().replaceFirst("[.]", ""); + } else { + // no hit? really lookup name now (normal import might kick in) and append deferred line + out.append(lookupName(deferredClassName, false)); + line = deferredLine.toString(); + } + deferredClassName = null; + } out.append(line); trailingNewline = false; } @@ -494,4 +507,12 @@ Map suggestedImports() { result.keySet().removeAll(referencedNames); return result; } + + Set suggestedStaticImports() { + return new LinkedHashSet(referencedStaticImports); + } + + Set staticImports() { + return staticImports; + } } diff --git a/src/main/java/com/squareup/javapoet/JavaFile.java b/src/main/java/com/squareup/javapoet/JavaFile.java index e7662ddd9..4fb16e110 100644 --- a/src/main/java/com/squareup/javapoet/JavaFile.java +++ b/src/main/java/com/squareup/javapoet/JavaFile.java @@ -31,6 +31,7 @@ import java.util.Set; import java.util.TreeSet; import javax.annotation.processing.Filer; +import javax.lang.model.SourceVersion; import javax.lang.model.element.Element; import javax.tools.JavaFileObject; import javax.tools.JavaFileObject.Kind; @@ -75,9 +76,10 @@ public void writeTo(Appendable out) throws IOException { CodeWriter importsCollector = new CodeWriter(NULL_APPENDABLE, indent, staticImports); emit(importsCollector); Map suggestedImports = importsCollector.suggestedImports(); + Set suggestedStaticImports = importsCollector.suggestedStaticImports(); // Second pass: write the code, taking advantage of the imports. - CodeWriter codeWriter = new CodeWriter(out, indent, suggestedImports, staticImports); + CodeWriter codeWriter = new CodeWriter(out, indent, suggestedImports, suggestedStaticImports); emit(codeWriter); } @@ -135,8 +137,8 @@ private void emit(CodeWriter codeWriter) throws IOException { codeWriter.emit("\n"); } - if (!staticImports.isEmpty()) { - for (String signature : staticImports) { + if (!codeWriter.staticImports().isEmpty()) { + for (String signature : codeWriter.staticImports()) { codeWriter.emit("import static $L;\n", signature); } codeWriter.emit("\n"); @@ -244,6 +246,7 @@ public Builder addStaticImport(ClassName className, String... names) { checkArgument(names.length > 0, "names array is empty"); for (String name : names) { checkArgument(name != null, "null entry in names array: %s", Arrays.toString(names)); + checkArgument(!SourceVersion.isKeyword(name), "keyword %s can not be imported", name); staticImports.add(className.canonicalName + "." + name); } return this; diff --git a/src/test/java/com/squareup/javapoet/JavaFileImportStaticTest.java b/src/test/java/com/squareup/javapoet/JavaFileImportStaticTest.java new file mode 100644 index 000000000..0abe3f240 --- /dev/null +++ b/src/test/java/com/squareup/javapoet/JavaFileImportStaticTest.java @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2016 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.javapoet; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeNotNull; + +import java.util.Arrays; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) public class JavaFileImportStaticTest { + + private static Object[] args(Object... args) { + return args; + } + + @Parameters(name = "{index}: {0}") public static Iterable data() { + MethodSpec tan = MethodSpec.methodBuilder("tan").build(); + return Arrays.asList(new Object[][] { + { "abs(-5)", "$T.abs(-5)", args(Math.class) }, + { "E", "$T.E", args(Math.class) }, + { "E", "$T\n.E", args(Math.class) }, + { "E", "$T \n.E", args(Math.class) }, + { "PI", "$T.$N", args(Math.class, "PI") }, + { "PI", "$T\n.$N", args(Math.class, "PI") }, + { "PI=PI", "$T . $N = PI", args(Math.class, "PI") }, + { "PI=PI", "PI = $T . $N", args(Math.class, "PI") }, + { "PI=PI", "PI = \n $T \t . \n\t \n\t $N", args(Math.class, "PI") }, + { "tan()", "$T.$N()", args(Math.class, tan) }, + { "sin(42.1)", "$T.$L($L)", args(Math.class, "sin", 42.1) }, + { "cos(E+9.9)", "$1T$2L$3L($1T$2NE+$4L$2L$4L)", args(Math.class, ".", "cos", 011) } + }); + } + + private final String expected; + private final String format; + private final Object[] args; + + public JavaFileImportStaticTest(String expected, String format, Object... args) { + this.expected = expected; + this.format = format; + this.args = args; + } + + @Test public void statementMatchesExpectation() { + String source = JavaFile.builder("com.squareup.tacos", + TypeSpec.classBuilder("Taco") + .addStaticBlock(CodeBlock.of(format + ";", args)) + .build()) + .addStaticImport(Math.class, "*") + .build() + .toString(); + assertTrue(source.contains("import static java.lang.Math.*;")); + int indexOfStaticInit = source.indexOf("static {\n") + "static {\n".length(); + String actual = source.substring(indexOfStaticInit, source.indexOf(";", indexOfStaticInit)); + assumeNotNull(expected); + assertEquals(expected, actual.replaceAll("\\s", "")); + } +} diff --git a/src/test/java/com/squareup/javapoet/JavaFileTest.java b/src/test/java/com/squareup/javapoet/JavaFileTest.java index e056116a0..a1ba09dcd 100644 --- a/src/test/java/com/squareup/javapoet/JavaFileTest.java +++ b/src/test/java/com/squareup/javapoet/JavaFileTest.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.concurrent.TimeUnit; import javax.lang.model.element.Modifier; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -49,7 +48,9 @@ public final class JavaFileTest { .build(); JavaFile example = JavaFile.builder("com.example.helloworld", hello) .addStaticImport(hoverboard, "createNimbus") + .addStaticImport(Math.class, "*") .addStaticImport(namedBoards, "*") + .addStaticImport(Thread.class, "currentThread") .addStaticImport(Collections.class, "*") .build(); assertThat(example.toString()).isEqualTo("" @@ -74,29 +75,6 @@ public final class JavaFileTest { + " }\n" + "}\n"); } - @Test public void importStaticForCrazyFormatsWorks() { - MethodSpec method = MethodSpec.methodBuilder("method").build(); - JavaFile.builder("com.squareup.tacos", - TypeSpec.classBuilder("Taco") - .addStaticBlock(CodeBlock.builder() - .addStatement("$T", Runtime.class) - .addStatement("$T.a()", Runtime.class) - .addStatement("$T.X", Runtime.class) - .addStatement("$T$T", Runtime.class, Runtime.class) - .addStatement("$T.$T", Runtime.class, Runtime.class) - .addStatement("$1T$1T", Runtime.class) - .addStatement("$1T$2L$1T", Runtime.class, "?") - .addStatement("$1T$2L$2S$1T", Runtime.class, "?") - .addStatement("$1T$2L$2S$1T$3N$1T", Runtime.class, "?", method) - .addStatement("$T$L", Runtime.class, "?") - .addStatement("$T$S", Runtime.class, "?") - .addStatement("$T$N", Runtime.class, method) - .build()) - .build()) - .addStaticImport(Runtime.class, "*") - .build() - .toString(); // don't look at the generated code... - } @Test public void importStaticMixed() { JavaFile source = JavaFile.builder("com.squareup.tacos", @@ -136,7 +114,6 @@ public final class JavaFileTest { + "}\n"); } - @Ignore("addStaticImport doesn't support members with $L") @Test public void importStaticDynamic() { JavaFile source = JavaFile.builder("com.squareup.tacos", TypeSpec.classBuilder("Taco")